Closed Bug 638452 Opened 14 years ago Closed 1 year ago

Can you please help us make the Orion code editor fast on Firefox?

Categories

(Core :: General, defect)

Product:
Core
Component:
General
Platform:
x86
macOS
Type:
defect

Tracking

()

Status:
RESOLVED INACTIVE

People

(Reporter: bokowski, Unassigned)

References

(
URL
)

Details

(Whiteboard: [orion])

Attachments

(1 file)

Patch I used to measure reflows
1.53 KB, patch
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12) Gecko/20100101 Firefox/4.0b12 Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12) Gecko/20100101 Firefox/4.0b12 The Orion project (http://wiki.eclipse.org/Orion) has a high-performance and scalable code editor. Unfortunately, it is performing significantly better on Safari and Chrome than it is on Firefox, with the end result that Orion developers and users are more likely to use these other browsers and not Firefox. We have a vested interest in Firefox because of our good relationship to the Firebug team - we've started to work on integration features between Orion and Firebug. It would be great if Firefox could become the best browser to use with Orion. Reproducible: Always Steps to Reproduce: 1. Go to http://orion.eclipse.org/editor/samples/demo.html 2. Make sure your browser window is the same size for every browser (e.g. maximized to the full screen). 3. Click on the "JavaScript file" button to load the editor with sample content 4. Click on the "PageDown Scroll" button each time you want to run a timed test. The other buttons test other things, but the PageDown test is a good representative. Actual Results: Here are results from running the "PageDown Scroll" test repeatedly on different browsers, roughly sorted from fastest to slowest. I used my MacBook Pro i5, with a screen size of 1680x1050, and each browser was maximized to fill the entire screen. Chrome 9.0.597.107 time(page down)=6461 time(page down)=7063 time(page down)=4335 time(page down)=2678 time(page down)=3145 time(page down)=3146 time(page down)=2633 time(page down)=2956 time(page down)=3306 time(page down)=2721 Safari 5.0.3 time(page down)=6826 time(page down)=6482 time(page down)=4067 time(page down)=4023 time(page down)=3878 time(page down)=3932 time(page down)=4001 time(page down)=3958 time(page down)=3954 time(page down)=4059 time(page down)=4018 FF 4.0b10 time(page down)=5561 time(page down)=6032 time(page down)=5390 time(page down)=5459 time(page down)=5661 time(page down)=6037 time(page down)=5359 time(page down)=5360 time(page down)=6270 time(page down)=5380 FF4.0b12 time(page down)=5853 time(page down)=5579 time(page down)=5852 time(page down)=5674 time(page down)=5844 time(page down)=5871 time(page down)=5829 time(page down)=5621 time(page down)=5689 time(page down)=5802 Opera 11.01 time(page down)=7334 time(page down)=7278 time(page down)=7240 time(page down)=7253 time(page down)=7198 time(page down)=7226 time(page down)=7308 time(page down)=7304 FF 3.6.13 time(page down)=7631 time(page down)=7437 time(page down)=9253 time(page down)=8722 time(page down)=7507 time(page down)=8840 time(page down)=8590 time(page down)=7742 time(page down)=7430 Expected Results: Please help us get close to the Safari and Chrome numbers! It is of course possible that the Orion editor is doing things that slow it down on Firefox but not on WebKit browsers. Incidentally, the JavaScript example file that you load in step 3 is the source code of the Orion editor. If you spot anything that we could do on our side, please let us know. I do realize that this is not a micro-benchmark that is easy to optimize for. Instead, it is an example of a real-world load and I don't expect any easy wins. But better to file a bug than remain silent about it. The numbers above are from running the test on Snow Leopard. We see similar differences on the other platforms (Linux, Windows).
Thanks for the testcase. I tossed some beginProfiling()/endProfiling() in there and profiled on Mac. Short summary: 20% of the time is painting (half is painting text; the rest is backgrounds, GL stuff, borders). 7% is reflow triggered from WillPaint. 2% is restyles triggered from WillPaint 3% event loop stuff 13% removeChild handling calls. The issue here is not that there's a particular removeChild we're slow to handle, but that there are a _lot_ of these, based on the profile data I'm seeing. 6% layout triggered by getting scrollWidth 8% insertBefore calls (again, a _lot_ of them). 2% innerHTML sets 2% setting inline style 2% createElement calls 13% layout triggered from getBoundingClientRect. There's a pretty big long tail, but those are the major bits. So obvious questions are: 1) What are all these node insertions and removals? 2) Why are there several callsites triggering layout (basically breaking our reflow coalescing by forcing layout at random points instead of when we actually need to update what the user sees).
Product: Firefox → Core
QA Contact: general → general
Oh, and there are things on our end that we can do to make things like the reflow here faster, but based on the above profile it looks like this code could be faster in _all_ browsers.
OK, so I dunno about the insert/remove stuff yet, but for the layout flushes, we have code sorta like this (lots of stuff ripped out): _getScroll: function() { var editorDiv = this._editorDiv; return {x: editorDiv.scrollLeft, y: editorDiv.scrollTop}; }, _handleScroll: function () { var scroll = this._getScroll (); // etc }, _doPageDown: function() { var verticalScrollOffset = this._getScroll().y; this._scrollView(0, something); }, _scrollView: function (pixelX, pixelY) { this._editorDiv.scrollLeft += pixelX; this._editorDiv.scrollTop += pixelY; this._handleScroll(); }, So in _doPageDown we flush out layout, then we call _scrollView which flushes _again_ (getting scroll*) and changes scroll positions and then calls _handleScroll which also flushes layout. You might get some mileage out of passing the x/y to _handleScroll, since the caller in this case actually has them. That said, roc, can we somehow optimize the reflow we end up doing on scroll* changes? :( For at least some of the bounding client rect stuff and DOM-munging, the issue is presumably that _getBoundsAtOffset removes the existing child of the line, creates a text node and a span and sticks them in the DOM and then flushes out layout and then blows them away and puts back the original textnode. This is called from _getOffsetToX in _doPageDown. Also, _getXToOffset causes similar issues. Keep in mind that if you modify the DOM and then ask for the size of the new nodes you added, then _all_ the layout for the page has to be updated, in general. So this "insert into the DOM and then measure" pattern is very expensive....
Thanks for looking at this already! Silenio, I hope you can respond to the comments after the weekend.
(In reply to comment #3) > That said, roc, can we somehow optimize the reflow we end up doing on scroll* > changes? :( That shouldn't be dirtying any frames except possibly for scrollbar thumbs or something. Is that what you're seeing? If the DOM munging and getBoundingClientRect is trying to measure offsets within a textnode, maybe the code should be using Range.getBoundingClientRect instead?
> Is that what you're seeing? I'm not sure yet. For one thing, since I made that comment I've decided that I have no idea which parts of the reflow are caused by the scroll and which by the DOM munging. Disentangling the two will be a good start.
See who's calling mDirtyRoots.AppendElement I guess...
(In reply to comment #1) > 1) What are all these node insertions and removals? > 2) Why are there several callsites triggering layout (basically breaking our > reflow coalescing by forcing layout at random points instead of when we > actually need to update what the user sees). The editor strategy is to create only the visible lines of the content in the DOM. This is done for several reasons, but the main reason is scalibity (been able to show huge content). As the editor scrolls, it removes the lines that were previously visible and add the new visible lines. Is beginProfiling()/endProfiling() some internal tool? Am I able to use it? (In reply to comment #3) > So in _doPageDown we flush out layout, then we call _scrollView which flushes > _again_ (getting scroll*) and changes scroll positions and then calls > _handleScroll which also flushes layout. You might get some mileage out of > passing the x/y to _handleScroll, since the caller in this case actually has > them. > > That said, roc, can we somehow optimize the reflow we end up doing on scroll* > changes? :( Good catch. I released changes to avoid the extra _getScroll() in this specific case. Unfortunately the improvement is not very significant. > > For at least some of the bounding client rect stuff and DOM-munging, the issue > is presumably that _getBoundsAtOffset removes the existing child of the line, > creates a text node and a span and sticks them in the DOM and then flushes out > layout and then blows them away and puts back the original textnode. This is > called from _getOffsetToX in _doPageDown. Also, _getXToOffset causes similar > issues. > > Keep in mind that if you modify the DOM and then ask for the size of the new > nodes you added, then _all_ the layout for the page has to be updated, in > general. So this "insert into the DOM and then measure" pattern is very > expensive.... In these scrolling benchmarks, _getOffsetToX() is called only once and _getXToOffset() is called once per page/line down/up. Only _getXToOffset() matters. I changed _getXToOffset() to return the first offset of the line without doing any work (see below). This would be the perfect situation. The improvement varies between 0% and 5%. From 7950ms to 7450ms for the best case. I believe it is worth investigating a different strategy to do hit testing, but these benchmarks would still be quite slower compared to Chrome (5050ms). One idea we have is to perform all measurements in a serapate IFRAME so that we would not have to modify the DOM and then put back the original text node. The DOM of the separate IFRAME would be smaller as well (probably just one DIV (line)) which in theory would mean faster reflows. We still have to experiment with this. Ideally, it would be great if Firefox provided API similar to IE which makes it possible to measure a text range. This would avoid modifying the DOM at all when performance hit testing. ... _getXToOffset_FF: function (lineIndex, x) { var model = this._model; if (true) return model.getLineStart(lineIndex); ... Improving hit testing is certainly desirable, but I believe it will not improve these benchmarks. The code that needs attention is updatePage() where all the lines are added/removed to/from the DOM. It would be interesting to know how many reflows are performed per page down. Does the profiling tool show this info?
(In reply to comment #5) > If the DOM munging and getBoundingClientRect is trying to measure offsets > within a textnode, maybe the code should be using Range.getBoundingClientRect > instead? Yes, it is exactly doing that. Is Range.getBoundingClientRect available in Firefox 4? This API would certainly avoid a lot of our DOM munging. I will check...
Answering my own question. Yes, Range.getBoundingClientRect is a available in Firefox 4 (latest chrome and opera as well). I released changes to use this API (if available) when performing hit testing instead of munging the DOM. The performance improvement is around 2%. Thanks for pointing it out.
> As the editor scrolls, it removes the lines that > were previously visible and add the new visible lines. OK. Have you considered doing that off a refresh observer instead of synchronously on scroll? That might help things some... Maybe. Maybe not. See below. > Is beginProfiling()/endProfiling() some internal tool? Am I able to use it? Not really, and if you have a Mac then yes. Make sure you have Shark installed, download http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central/firefox-4.0b13pre.en-US.mac-shark.dmg then start shark, put it in programmatic control mode, add startProfiling() and stopProfiling() (not begin and end as I said above) calls where you want to start and stop the profiler, and you get data about where time is spent. Now interpreting that data may take some work.. It's low-level profiling data for the C++ code involved, so you can see _where_ time is spent but usually not _why_ (because that information is in the JS). > Good catch. I released changes to avoid the extra _getScroll() in this > specific case. Unfortunately the improvement is not very significant. Well, nothing in the profile looks "significant" on its own. There's no obvious hotspot. There are a variety of several-percent wins that _could_ add up to something. > It would be interesting to know how many reflows are performed per page down. > Does the profiling tool show this info? No, but it's pretty easy to measure this with some code changes. The number is 47 for the demo as it appears today (on my setup the demo shows 42.3 lines of text in case that matters). We do know that our incremental layout stuff is somewhat slower than Webkit, and we're working on that. But in the meantime, reducing that 47 to, say, 1 might help some. ;)
I applied this patch, then added a dump() call at the top of the |function t| callback in test_pageDownScrolling. I'll do a bit more looking into this on our end too.
Looking at the data a bit more, layout is 26% of the time. So reducing the number of reflows will help by at most 26%. The other big obvious things are removeChild at 12-13% and insertBefore about 9%. For the removeChild, about 2/3 if the time is the invalidations it does. Another 20% is the actual frame destruction. It looks like there's a separate block for each line here or something? For the insertBefore, there's some time spent in AdoptNode (why? Aren't the nodes being created in the right document?), and some time spent doing eager frame construction (because we're in an editor).... No obvious ways to win there on the part of the page other than eliminating the adoptNode bit (!-2% of total time) and possibly not being as eager to create nodes (see comment 11). Again, there are a bunch of things we have in the pipeline on our end that will help with some of that stuff (e.g. the invalidation bit), but that doesn't help you guys.
(In reply to comment #11) > OK. Have you considered doing that off a refresh observer instead of > synchronously on scroll? That might help things some... Maybe. Maybe not. > See below. > I am not sure what the refresh observers are (?). I experimented making scrolling not synchronous in scrollView() by just not calling _doScroll() and letting the _handleScroll() be called from "scroll" event handler only. I did not see any improvements in this specific bench. > Not really, and if you have a Mac then yes. Make sure you have Shark > installed, download > http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central/firefox-4.0b13pre.en-US.mac-shark.dmg > then start shark, put it in programmatic control mode, add startProfiling() and > stopProfiling() (not begin and end as I said above) calls where you want to > start and stop the profiler, and you get data about where time is spent. > > Now interpreting that data may take some work.. It's low-level profiling data > for the C++ code involved, so you can see _where_ time is spent but usually not > _why_ (because that information is in the JS). Thanks for the instructions. Got this setup going. Interpreting to data is quite hard for me given that I am not familiar with the mozilla internals. (In reply to comment #13) > Looking at the data a bit more, layout is 26% of the time. So reducing the > number of reflows will help by at most 26%. > > The other big obvious things are removeChild at 12-13% and insertBefore about > 9%. > > For the removeChild, about 2/3 if the time is the invalidations it does. > Another 20% is the actual frame destruction. It looks like there's a separate > block for each line here or something? That is it. Every line is one DIV with one or more SPAN as children. > > For the insertBefore, there's some time spent in AdoptNode (why? Aren't the > nodes being created in the right document?), and some time spent doing eager > frame construction (because we're in an editor).... > > No obvious ways to win there on the part of the page other than eliminating the > adoptNode bit (!-2% of total time) and possibly not being as eager to create > nodes (see comment 11). I found a couple of places we were using the wrong document. It should be fixed now. The improvement as you said is around 2%. Thanks!
Instead of removing many nodes or inserting many nodes one at a time, you could use a DOM range to select the entire range of elements you want to remove, then remove them in one call to deleteContents. And you could use a document fragment to collect all the nodes you want to insert and insert them all at once: https://developer.mozilla.org/en/DOM/range.deleteContents https://developer.mozilla.org/en/DOM/document.createDocumentFragment This would probably make your editor faster in all browsers, but that's not a bad thing, right? :-)
> I am not sure what the refresh observers are http://weblogs.mozillazine.org/roc/archives/2010/08/mozrequestanima.html As I said, it may well not help. It really looks like the things that will help on our end are possibly painting less, faster invalidation simpler block data structures that cost less to create and tear down. I'm not sure what else you can do on your end at the moment, apart from trying comment 15.
(In reply to comment #15) > Instead of removing many nodes or inserting many nodes one at a time, you > use a DOM range to select the entire range of elements you want to remove, > remove them in one call to deleteContents. And you could use a document > fragment to collect all the nodes you want to insert and insert them all at > once: > https://developer.mozilla.org/en/DOM/range.deleteContents > https://developer.mozilla.org/en/DOM/document.createDocumentFragment > This would probably make your editor faster in all browsers, but that's not a > bad thing, right? :-) We had tried document.createDocumentFragment in the past and did not see much of an improvement. I tried again last week and confirmed that. I still have not tried range.deleteContents. While trying createDocumentFragment, I had to rearrange the code which used to create one line, add the line the DOM and measure it. In order to take advantage of createDocumentFragment, I had to create consecutive lines adding to the fragment and measure these lines (getBoundingClientRect) after adding the fragment to the DOM. This change actually made a difference on my WinXp, Window7 and Linux systems (around 13%). Strange enough, it seems to made less of a difference on my Mac (around 4%). As it stands, the scrolling bench on FF4 is comparable to Chrome/Opera on my WinXp. It still lags quite a bit on my Windows 7, Linux and Mac.
Status: UNCONFIRMED → NEW
Ever confirmed: true
The testcase in comment 0 is now 404.
(In reply to comment #18) > The testcase in comment 0 is now 404. This is the updated URL: http://orion.eclipse.org/examples/textview/demo.html
Attachment #517464 - Attachment is patch: true
Attachment #517464 - Attachment mime type: application/octet-stream → text/plain
Nathan, this is the bug I was telling you about.
Whiteboard: [orion]
Severity: normal → S3
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: