User-Agent: Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322; .NET CLR 2.0.50727) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070314 Minefield/3.0a3pre A javascript function that creates elements in the SVG DOM runs many times more slowly in FF3 than FF2. The performance of pure javascript code (e.g. an empty loop) is of the same order as is javascript code that updates only the HTML DOM, hence this bug is raised against SVG not javascript or DOM. Reproducible: Always Steps to Reproduce: 1. Put the attached test.svg and test.html in a directory. 2. For FF3 you will probably need to increase dom.max_script_run_time in about:config. 3. Open test.html in FF2 and in FF3. 4. Note and compare the timings reported in the alert message. Actual Results: In my tests I get: Empty loop FF2: 1.5s, FF3: 1.6s 5K rectangles FF2: 1.6s, FF3: 8.2s 5K text elements FF2: 1.5s, FF3: 24.2s 5K HTML divs FF2: 1.1s, FF3: 1.4s Changing the test case to do 10K iterations I get: Empty loop FF2: 1.9s, FF3: 1.6s 5K rectangles FF2: 3.3s, FF3: 28.2s 5K text elements FF2: 2.9s, FF3: 89.2s 5K HTML divs FF2: 2.2s, FF3: 3.1s Expected Results: FF 3 should exhibit at least comparable performance, certainly not be dramatically slower otherwise people's javascript applications will be hit.

Would you be willing to establish a regression range? Of course it may be that the regression did not happen in one go.

Yes,I'll have a go at that - leave it with me for now

Reattaching modified HTML file that will work live in the bug.

Duncan, the regression seems to have occurred between the 2007-01-29-04-trunk and 2007-02-16-04-trunk builds. http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/ I'd suggest the best way to track down the regression is to test the build in the middle to half the regression range, rinse and repeat (sorry if that's patronizing). Once you have a one day range we can look at the checkins that occurred in that range and figure out who broke things. :-)

So we're down to a 2.5 week range already ! I'm on the case, as you say a binary chop would be the thing.

I've got the regression range down to a single day, 31/1/2007. My results are Tight 5K SVG 5K SVG 5K HTML Loop rectangle text DIV 30/1/2007 1.80s 2.08s 2.28s 1.38s 31/1/2007 1.99s 8.84s 25.6s 1.53s

Excellent. Can you give the build dates (e.g. 2007-02-16-04-trunk) since they have an hour in there too (and given the shear volume of stuff that's often checked into Mozilla in a 24 hour period that can be important).

Both downloads are ...-04-trunk 30/1/2007 download from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2007-01-30-04-trunk/firefox-3.0a2pre.en-US.win32.installer.exe 31/1/2007 download from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2007-01-31-04-trunk/firefox-3.0a2pre.en-US.win32.installer.exe

Thanks. Okay, so the checkins in this range are: http://bonsai.mozilla.org/cvsquery.cgi?module=SeaMonkeyAll&date=explicit&mindate=2007-01-30&maxdate=2007-01-31+04 I'll look at this more closely.

Add perf key word?

It turns out the regression is caused by the checkin for bug 18333 to implement incremental loading of XML. Since the amount of markup in the testcase is tiny, I'm wondering how changes to the XML parser could possibly have regressed DOM methods so massively.

What's probably happening here is that before bug 18333 we had not started layout when the script executed. Now layout is started so all DOM manipulations cause a bunch of layout stuff to happen. Try surrounding your code with .suspendRedraw and .resumeRedraw and see if that helps.

Or do it all from a setTimeout in an old build, and see if it hurts ;)

Tried <root element>.suspendRedraw() and <root element>.unsuspendRedawAll() and it does have a significant effect but not the panacea I would have wished for. Also tried adding my rectangles and text elements to a <g> element and then adding the <g> to the root at the end. This has a larger effect but is not a usable workaround because getComputedTextLength() doesn't work until the text element is in the document and it is still slower than Firefox 2. My results are Tight 5K SVG 5K SVG 5K HTML Loop rectangle text DIV No suspend/unsuspend 1.66s 8.33s 25.23s 1.45s Suspend/Unsuspend 1.7s 4.92s 9.9s 1.58s Using <g>, adding at end 1.57s 2.55s 2.76s 1.49s FF2.0.0.2, for comparison 1.7s 1.53s 1.43s 1.15s Would it be sensible if when creating the DOM from javascript, no layout/redrawing was performed until the end of the script since nothing will actually get rendered while a script is running anyway ? It just slows down the script for no benefit.

Two things are strange here: 1. That with suspend/unsuspend we're so much slower than when adding inside a <g> 2. That when adding inside a <g> we're so much slower than when 2.0 Are you by any chance doing something inside the loop that are forcing us to reflow? That would explain 1, but seems strange that that would work at all then in 2.0.

Hi Jonas, I don't think I know what you mean by "reflow" (done a quick google and none the wiser) but I'm not doing anything special - see the original test case, all I added was var g=SVGDocument.createElementNS(svgns, "g"); and then change SVGDocument.documentElement.appendChild(shape); to g.appendChild(shape); and then add SVGDocument.documentElement.appendChild(g); at the end.

A quick thought: Since it appears this and also bug 374345 are both a side effect of incremental XML rendering, is a possible resolution to disable incremental rendering whilst a script is running ? Am I right in thinking that incremental rendering of any kind only has value when loading large files from a server but no benefit when the content is produced by javascript since the results of a script are never shown until the script completes anyway ?

Upped the severity of this since the performance hit is very noticeable and as far as I can tell will impact everyone using javascript and the SVG DOM which is a fair proportion of SVG users.

(In reply to comment #15) > Or do it all from a setTimeout in an old build, and see if it hurts ;) > This version should do that. The thing to test with this is: how do the timings compare when you run it in FF2.0.0.2.

Interesting....this runs at much the same speed in FF2 apart from the text elements which are now SLOWER in FF2 than FF3 My results are Tight 5K SVG 5K SVG 5K HTML Loop rectangle text DIV FF2 1.56s 8.37s 44.93s 1.12s FF3 1.56s 8.42s 26.55s 1.53s My installed versions are FF2: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3 FF3: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070316 Minefield/3.0a3pre

Robert: FF2 has the old, very much slower version of cairo. Testing should really be done in pre- and post-async XML trunk builds, but yeah, the setTimeout in a pre-async XML build gives me similar times to the testcase without the setTimeout in a post-async XML build.

Duncan: yeah, it was expected to be slower. Jesse proposed using a setTimeout in an older build as a way to prove that Jonas is right. The setTimeout makes the test run after load time, when "reflow" is active. If reflow is indeed what's caused this bug then the setTimeout test in an old build would be expected to show the same slowness, and it does.

So if I can summarise my understanding the research into this so far there are several effects at work. If we focus just on SVG rectange/text elements and take my results from comment #16 we start with FF3 at 8.33/25.23s. Using suspend/unsuspend redraw removes the effect of "incremental XML rendering" and gets us down to 4.92/9.9s. Adding the text within a <g> removes the effect of "reflow" and gets us down to 2.55/2.76s. We have not yet explained the difference between this and the FF2 timing of 1.53/1.43s. Next question....what can be done about it ? Is my comment #19 ill-conceived and could the same argument apply to "reflow" as well as incremental XML rendering ?

(In reply to comment #25) In mozilla you have content and frames. In simple terms, things which exist have content, if they are also drawn they have frames. In FF2 without setTimeout onLoad occurs before the frame creation phase so all the frames are created at once at the end. In FF3 due to incremental XML, we create the frames as we go along. When you create a <g> element without attaching it to the document, it has no frame. Then you add in all the children and they have no frames either. Finally you attach the <g> to the document and all the frames are created at once. If you add things one at a time you create the frames as you are going along which would seem to be slower. One thing that is slow is that each time we append a frame we need to find the last frame and add the new frame after that. Frames use a singly linked list so we have to traverse the list from beginning to end each time calling nsFrameList::LastChild and we end up O(N2). If we add them all at once then we can link them as we go O(N). One thing to try would be to reverse the loop and use insertBefore rather than appendChild to insert each element at the beginning rather than at the end. (In reply to comment #19) The results of a script can be seen before it completes if the script calls alert for instance. suspendRedraw disables rendering but does not disable frame creation.

It is not possible to disable "incremental rendering" since that is not the problem really. The problem is simply "rendering". We now render the document right away which means that any modifications done to the document are also displayed. While we try to collect changes and only show them once the javascript stops running, even that collecting is bound to be slower then not doing anything (like we did before). But we could certainly try to be better at this. Feel free to file a separate bug on that. The solution is to add things in a <g> that is not yet part of the document, and then insert that in a single operation. The difference between FF2 and FF3 for the <g> test are indeed unsolved and would be very interesting to see what cause that. The best way to test this would be to do a binary search between the point when FF2 branched (which is before FF1.5) and todays FF3 builds. Again, feel free to file a separate bug on this. I do recommend filing separate bugs on these issues since it's always good to have separate bugs on separate things, and this bug is already long enough that it's hard to follow.

(In reply to comment #28) > Created an attachment (id=259109) [details] > insertBefore and suspendRedraw Robert, using insertBefore is much quicker than appendChild, just as you surmised. Also, looking at my original results in comment #1 the timings do appear to be order O(N2), roughly four times as long for 10K as for 5K. Bearing in mind Jonas' comment #29 I was thinking of splitting this bug into three: - One comparing DOM performance of FF2 vs FF3 when using a <g> which is therefore nothing to do with either reflow or rendering. - One comparing DOM performance of FF2 vs FF3 when not using a <g> but when using suspendRedraw and therefore nothing to do with rendering but demonstrating the slowdown due to reflow in FF3. - One comparing DOM performance of FF2 vs FF3 when not using a <g> and not using suspendRedraw which therefore demonstrating the bigger slowdown doe to reflow and rendering in FF3 as compared to the slowdown due to reflow alone in FF3. Unless somebody violently disagrees I'll go ahead and raise two more bugs in the next day or two and find a way of linking them back to this one.

(In reply to comment #30) When you say reflow you really mean frame creation. appendChild performance is bug 40988. This mostly covers the second case. As far as the third case is concerned isn't it obvious that if you redraw things as you make changes it will be slower than batching it all up and redrawing all the changes once at the end. I'm not sure how much traction you will get with a bug like that.

And bug 270264, although the code it talks about has changed a lot and bug 233463 which is also about making appendChild quicker.

(In reply to comment #31) Robert, obvious to you and increasingly to me but definitely not obvious to an "average joe" who doesn't know anything about the changes for incremental XML rendering and just perceives a slowdown. I suppose what I'm saying is that the change for incremental XML rendering needs to come with a health warning about using suspendRedraw to avoid the DOM performance penalty. Unless redraw is disabled by default from a javascript load event handler.

Robert, looking at bug 40988 and bug 233463, neither mention the SVG DOM specifically. Also, neither these two bugs nor bug 270364 are marked as regressions. What I am reporting with this bug is not generic slow performance of element.appendChild(), though that may be an issue others have captured. It is the fact that element.appendChild() is markedly slower on FF3 for the SVG DOM only. The HTML DOM performance is slower on FF3 and FF2 but not by anything like the same margin. It also seems that adding text elements rather than rectangles has a larger effect. On FF2, adding text elements performs about the same as rectangles. There must be some SVG-specific issue in FF3 outside of rendering - maybe it is frame creation but if so it is something specific to SVG. To recap the timings from comment #16 (ignore the top line)... Tight 5K SVG 5K SVG 5K HTML Loop rectangle text DIV No suspend/unsuspend 1.66s 8.33s 25.23s 1.45s Suspend/Unsuspend 1.7s 4.92s 9.9s 1.58s Using <g>, adding at end 1.57s 2.55s 2.76s 1.49s FF2.0.0.2, for comparison 1.7s 1.53s 1.43s 1.15s

(In reply to comment #34) In FF2 we incrementally parse and render html so you see web pages appear a bit at a time as they are transmitted over the network. In FF2 we don't do this with XML, instead we wait till it has all arrived and then render it in one go. In FF3 we do incremental rendering for both XML and html. The slowdown affects SVG because it is XML, you should find that xhtml and mathml are affected in the same way. It also only affects onload, if you try to use the DOM after the page has loaded then you will find FF2 is slow too. The fact is that FF2 cheated and skipped a necessary step in order to get the speed it did. SVG does not work properly with onload partly because of this, you will find that some functions don't work because they require layout information e.g. getBBox. The answer as Jonas said in comment 29 is to use a <g>. Adding text elements is slower than adding rectangles because text is much more complicated, you have to check how it is justified for instance in order to determine its position.

Should I close this bug on the grounds that - Everyone should use suspendRedraw/unsuspendRedraw and add their SVG elements within a <g>, adding the <g> to the document at the end and watching out for things like getComputedTextLength() not working. This does indeed largely get around the performance problem. - Any performance issues not resolved by the above are generic DOM performance issues and are captured already in other bugs. FWIW I'm uneasy about this, got a feeling something is being missed. I can add 5K HTML divs directly to the document body in pretty quick time on FF3 even though it's reported to be doing incremental parsing and rendering of HTML. No need to disable HTML redrawing (if there is such a thing) or add the divs to another DIV which is then added to the document at the end. So if FF3 applies similar incremental processing to XML then why is adding 5K SVG elements so much slower than 5K HTML divs - is the impact of incremental XML expected to be greater than incremental HTML ? Something don't feel right....but if I've lost the plot lets close the bug and all go home.

(In reply to comment #36) > - Everyone should use suspendRedraw/unsuspendRedraw and add their SVG elements > within a <g>, adding the <g> to the document at the end and watching out for > things like getComputedTextLength() not working. This does indeed largely get > around the performance problem. If you add elements within a <g> then suspendRedraw is irrelevant. If you need getComputedTextLength then you could either call it after you parent the <g> or add the elements directly, in which case you should use suspendRedraw. > - Any performance issues not resolved by the above are generic DOM performance > issues and are captured already in other bugs. You have discovered that adding via <g> and/or adding <div> elements in FF3 is slower than FF2. I don't know of a bug which captures that, so that would be worth raising as a separate issue. Since the slowdown affects html this sounds on the face of it like either a DOM or javascript issue. > > FWIW I'm uneasy about this, got a feeling something is being missed. I can add > 5K HTML divs directly to the document body in pretty quick time on FF3 even > though it's reported to be doing incremental parsing and rendering of HTML. No > need to disable HTML redrawing (if there is such a thing) or add the divs to > another DIV which is then added to the document at the end. So if FF3 applies > similar incremental processing to XML then why is adding 5K SVG elements so > much slower than 5K HTML divs - is the impact of incremental XML expected to be > greater than incremental HTML ? Something don't feel right....but if I've lost > the plot lets close the bug and all go home. > You are right that there ought to be scope in SVG for further improvements in object creation speed, it's just that comparisons with FF2 are a bit unfair unless you compare the setTimeout speeds as per comment 22. We could leave this bug open to check how we're getting on with that from time to time.

Created bug 375470 for the slowdown observed after eliminating effects of redrawing and layout. Robert, I think there are now two separate issues: (1) improvements in object creation speed that might be possible but accepting that this is not a regression from FF2 (as proved using setTimeout) and that it will never match the performance before incremental layout/redraw was implemented. And (2) the perception of a "typical" user loading SVG from an onload handler who doesn't know about any of this and now takes this not insignificant hit at load time when previously they didn't. It's (2) that concerns me more than (1). All well and good that it can be worked around using a <g> but what proportion of people will discover this and what proportion will just live with a slower response, thus diminishing Firefox SVG's reputation. Take my situation - migrating from the IE6/ASV platform where this wasn't an issue to FF2 where it also wasn't an issue and then to FF3 where suddenly it was an issue until I went through all of this to find the workaround. And I won't be the only one bearing in mind the withdrawal of ASV.

Robert: please don't think two bugs are the same just because they both involve .appendChild. The problems discussed in this bug are regressions between FF2 and FF3, bugs in the order of 2xxxxx can't possibly be about FF3 as development for that hadn't started yet.

Also, when in doubt, file a new bug. People that really do know the code will dup it against existing bugs if ones exist. If a bug never gets filed that is much worse.

I sense something of an impasse here. I understand the point that incremental XML is bound to have an impact and that it is unfair to compare FF2 and FF3 but the fact is, developers WILL compare them directly. Raising bug 379533 to cover discussion on whether it is acceptable to expect developers to have to apply workarounds to achieve previous levels of performance. This bug should focus only on if and how performance impact could gradually be reduced since there does seem to be some scope for this.

So... I'm looking at the initial numbers here, and here's the problem I see: The execution time is O(N^2) in the number of rectangles or text elements. At least the 10K tests in comment 0 take about 4 times as long as the 5K tests. That's the real problem we should be addressing, imho. I've filed bug 380471 on that, and will put up a profile there if I this testcase doesn't kill X. ;)

The two issues in comment 17 should probably also be filed as separate bugs blocking this one...

I think bug 375470 covers issue 2 in comment 17 already. Though admittedly these bugs are starting to flow into each other as the same issues are raised in all of them :(

> I think bug 375470 covers issue 2 in comment 17 already. OK. I'll post profiles there.

SuspendRedraw/UnsuspendRedraw (those then call QI) seem to be called a lot. Can we do something to the SR/USR calls in nsSVGOuterSVGFrame::InsertFrames? Why are those needed? Code has changed quite a bit since those were add there; InitialUpdate() call isn't there anymore. I tried removing them and at least croczilla examples worked like with them. Flat Profile Total hit count: 702 Count %Total Function Name 128 18.2 _edata 98 14.0 nsSVGPathGeometryFrame::QueryInterface(nsID const&, void**) 94 13.4 nsSVGOuterSVGFrame::SuspendRedraw() 94 13.4 nsSVGOuterSVGFrame::UnsuspendRedraw() 74 10.5 nsFrameList::LastChild() const 36 5.1 nsSVGDisplayContainerFrame::QueryInterface(nsID const&, void**) 32 4.6 nsSVGGlyphFrame::QueryInterface(nsID const&, void**) 28 4.0 nsSVGDisplayContainerFrame::NotifyRedrawSuspended() 17 2.4 nsSVGDisplayContainerFrame::NotifyRedrawUnsuspended() 15 2.1 nsSVGTextContainerFrame::QueryInterface(nsID const&, void**) 6 0.9 nsSVGPathGeometryFrame::NotifyRedrawUnsuspended() 4 0.6 nsCSSFrameConstructor::ContentAppended(nsIContent*, int) 3 0.4 nsSVGTextFrame::NotifyRedrawUnsuspended() 2 0.3 non-virtual thunk to nsSVGPathGeometryFrame::NotifyRedrawUnsuspended() 2 0.3 nsSVGRectElement::QueryInterface(nsID const&, void**) 2 0.3 nsSVGOuterSVGFrame::GetType() const 2 0.3 nsSVGGlyphFrame::NotifyRedrawUnsuspended() 2 0.3 nsSVGTextFrame::NotifyRedrawSuspended() 2 0.3 nsJSContext::DOMBranchCallback(JSContext*, JSScript*) 2 0.3 SelectorMatches(RuleProcessorData&, nsCSSSelector*, int, nsIAtom*, int*) 2 0.3 nsSVGGlyphFrame::AddRef() 2 0.3 non-virtual thunk to nsSVGGlyphFrame::AddRef() 2 0.3 nsSVGTextFrame::UpdateGlyphPositioning() 2 0.3 nsAttrValue::SetTo(nsAString_internal const&) 2 0.3 CSSParserImpl::AddRef() 2 0.3 non-virtual thunk to nsSVGPathGeometryFrame::NotifyRedrawSuspended() ...

When SR/USR are removed from InsertFrames, nsFrameList::LastChild starts to dominate in jprof flat profile Total hit count: 302 Count %Total Function Name 143 47.4 _edata 101 33.4 nsFrameList::LastChild() const 3 1.0 nsJSContext::DOMBranchCallback(JSContext*, JSScript*) 2 0.7 SelectorMatches(RuleProcessorData&, nsCSSSelector*, int, nsIAtom*, int*) 2 0.7 nsSVGOuterSVGFrame::InsertFrames(nsIAtom*, nsIFrame*, nsIFrame*) ...

(In reply to comment #46) > Code has changed quite a bit since those were > add there; InitialUpdate() call isn't there anymore. The InitialUpdate calls on the children have moved to the base class' implementation which is still called. That said I still don't see any point to those SR/USR calls so InitialUpdate could be removed from nsSVGOuterSVGFrame IMO. > I tried removing them and at least croczilla examples worked like with them. SR/USR is only an optimization to prevent things from being rasterized when we know they'll need to be rasterized again soon. Removing the calls would never be expected to change rendering, only potentially make things slower. I don't think that's the case here though.

(In reply to comment #48) > SR/USR is only an optimization to prevent things from being rasterized when we > know they'll need to be rasterized again soon. Removing the calls would never > be expected to change rendering, only potentially make things slower. I don't > think that's the case here though. Exactly. Is it optimizing anything here?

No. When adding a frame list containing frames that overlap on the screen, we might in principle end up drawing the overlapping regions multiple times (when InitialUpdate is called on the children in the list it requests the area the child covers be rerasterized). What we'd have is a stack like this: nsIFrame::Invalidate() nsSVGOuterSVGFrame::InvalidateRect() nsSVGPathGeometryFrame::UpdateGraphic() nsSVGPathGeometryFrame::InitialUpdate() nsSVGDisplayContainerFrame::InsertFrames() nsSVGOuterSVGFrame::InsertFrames() nsSVGContainerFrame::AppendFrames() In practice, when we call nsIFrame::Invalidate we let aImmediate default to PR_FALSE, so these invalidations get buffered there anyway (and the combined dirty area gets rasterized all at once later). Hence, in this case, the SR/USR isn't helping. We really need to sort out our story on SR/USR buffering vs. viewManager buffering, but I don't have the cycles right now.

We should probably just leave buffering issues till we do the compositor post-1.9.

With the latest trunk (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007101605 Minefield/3.0a9pre), loading https://bugzilla.mozilla.org/attachment.cgi?id=258652 I now get Empty loop [FF2: 1.5s], [Old FF3]: 1.6s, [Latest FF3] 1.402s 5K rectangles [FF2: 1.6s], [Old FF3]: 8.2s, [Latest FF3] 2.060s 5K text elements [FF2: 1.5s], [Old FF3]: 24.2s, [Latest FF3] 4.727s 5K HTML divs [FF2: 1.1s], [Old FF3]: 1.4s, [Latest FF3] 1.263s That's more than a little better ! One curiosity is that if I save the html and svg files to disk and re-run (I wanted to try 10K) the performance is noticeably worse, around 3.5s as compared to 2s for 5K rectangles. Why would the same sources run slower just because they're on local disk ? The time for 10K SVG rectangles and text elements is still about 4 times that for 4K so there is still O(N^2) behaviour but the constant is much lower, which is great.

Just re-ran this a few times on local disk vs the links in the bug and the performance difference is very marked - 100% or more for the rectangles and 50% for the text. The HTML divs don't seem to be affected. I hadn't noticed this before - perhaps the effects were masked. Should this be a separate bug ?

Performance has improved to the point where this doesn't seem to be a blocker anymore.

(In reply to comment #53) Filed bug 410314 on the slowdown with file:// as compared to http://. Should one depend on the other ? If so, could someone who knows how update it ?

Performance in Firefox 3 is so much improved that IMHO keeping this issue opened may not make sense anymore. Although an "Unresponsive script" warning is displayed (at least in "insertBefore and suspendRedraw" attachment) which, due to the heavy DOM manipulation seems perfectly acceptable. Version: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008030405 Minefield/3.0b5pre

A lot of work has been done to speed up DOM performance which should have helped here. Duncan: Can you still see any slowdowns with FF3 compared to FF2? If the only remaining issue is that we now start layout sooner I don't think we should keep this bug open.

My results just now (averaging five runs) were Empty loop FF2: 869 Latest FF3: 667 5K rectangles FF2: 1659 Latest FF3: 2997 5K text elements FF2: 1856 Latest FF3: 9304 5K HTML divs FF2: 1072 Latest FF3: 504 Let me just repeat that exercise on a different PC because I do seem to get different behaviour on my work laptop vs home desktop. Versions used are FF2 Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1 and FF3 Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008030706 Minefield/3.0b5pre

OK, different PC, same version of trunk but using FF 2.0.0.11 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11) Empty loop FF2: 1236 Latest FF3: 910 5K rectangles FF2: 1188 Latest FF3: 1231 5K text elements FF2: 1360 Latest FF3: 3687 5K HTML divs FF2: 756 Latest FF3: 292

It's clear that (i) all aspects of this bug have been massively improved and (ii) the javascript-only and html DOM performance is now far superior in FF3 and (iii) Even with the slower SVG DOM manipulations FF3 displays the image just as quickly because it is there as soon as the script finishes whilst FF2 has to spend time rendering it after the script has finished. And ofcourse FF3 is 10 times more responsive once the image is there because of the faster Cairo. The only question is whether there's more that could be done on the SVG DOM side - this bug was originally raised about SVG DOM. It seems harsh when so much great work has been done but I think there are still some things that could be better. For example when I compare the performance of the last two attachments - which differ only in that one uses insertBefore() instead of appendChild() - I see a big speedup for SVG text elements but a slowdown for SVG rectangles and HTML divs. That seems odd - if it's to do with scanning to find the end of the list then wouldn't we expect that to affect performance in the same way for any type of element ? Intuitively, appendChild() and insertBefore() should both work as efficiently as each other if a pointer to the end of the list is maintained. I know there were other bugs about appendChild() performance but I haven't been following their status. It's also odd that the performance difference for SVG DOM, as in the ratio of FF3 to FF2 is so different on my two PCs...unfortunate that my versions of FF2 are nearly a year apart - perhaps I should make them the same and repeat.

Using the last attachment, https://bugzilla.mozilla.org/attachment.cgi?id=259109, the trunk beats FF2 on everything, SVG included. The two differences in that attachment are (i) it uses suspendRedraw and unsuspendRedraw and (ii) it uses insertBefore instead of appendChild. Here's my suggestion. From my naive perspective, I don't see (i) why appendChild couldn't be fixed to be as quick as insertBefore and (ii) why a change couldn't be made so that, in effect, suspendRedraw is automatically called whenever a script begins to run and unsuspendRedraw called when a script finishes (or if it alerts). In other words, stop doing layout while a script is running because nobody will see it. If neither of those things can be done, let's close the bug and accept we've gone as far as we can. If one or both of those things can be done let's close this bug anyway and file new bugs for those ideas (unless they're covered elsewhere).

The reason we can't automatically suspend/unsuspend is that people do animations using javascript, and suspending during the whole animation would sort of defeat the purpose :) Which testcase were you using in comment 59? The numbers in comment 58 look terrible. Did you by any chance run these tests from a local file rather than from a http url? If so we have known issues there that will be fixed before FF3 release. Do you have any extensions installed on that machine?

Jonas Yes the numbers are not great, especially on my home PC. They weren't from local files, they were straight comparisons using the attachments on this bug. It's a real shame when everything but the SVG is far quicker in the trunk. A bit of profiling on insertBefore vs appendChild should reveal what's going on there. Re suspendRedraw - I'm pretty sure when people do animations in javascript they use TIMERS. As in, execute individual scripts at regular intervals that each move something a little bit and then FINISH, displaying the results. As each bit of movement is a separate script, it wouldn't matter at all if layout were deferred until the end of each script. When I've tried to write a single script that does a series of movements, e.g. executes a loop that moves a rectangle across the screen bit by bit, then I don't see smooth movement, only the final result when the script finishes or blocks, e.g. on an alert. I tried for hours to get it to work and eventually raised a new feature request in, see https://bugzilla.mozilla.org/show_bug.cgi?id=373990. Try the test case on that bug (it's not SVG but the same principle applies). I'm as sure as I can be there's absolutely no way to get Firefox to show you the results of a script whilst it is executing (excepting the alert case). Even if it were possible, people would want to use setTimeout() anyway as it's the only way to delay execution and hence control the speed of an animation.

Jonas, I meant to add: Both comment 58 and 59 were using the original test case https://bugzilla.mozilla.org/attachment.cgi?id=258652. Neither PC has extensions or themes (but some plugins)

(In reply to comment #63) > > When I've tried to write a single script that does a series of movements, e.g. > executes a loop that moves a rectangle across the screen bit by bit, then I > don't see smooth movement, only the final result when the script finishes or > blocks, e.g. on an alert. I tried for hours to get it to work and eventually > raised a new feature request in, see > https://bugzilla.mozilla.org/show_bug.cgi?id=373990. Try the test case on that > bug (it's not SVG but the same principle applies). I'm as sure as I can be > there's absolutely no way to get Firefox to show you the results of a script > whilst it is executing (excepting the alert case). Doesn't forceRedraw do what you want http://www.w3.org/TR/SVG11/struct.html#DOMInterfaces

Attaching an example to backup my statements about animation. The attached is a simple and short animation that just moves a square a short distance across the screen. When using setTimeout which I believe would be the normal way of doing it, it works equally well both with and without suspendRedraw. When not using setTimeout it simply does not work.

(In reply to comment #66) > > Doesn't forceRedraw do what you want > http://www.w3.org/TR/SVG11/struct.html#DOMInterfaces > Absolutely not, forceRedraw() doesn't do anything - try the attached.

...and even if it did work in the future (i) people would still use timers for animation in javascript because that's how you control the speed; and (ii) if people used forceRefraw() the DOM could honour it. It's not a reason to dismiss the idea of having redraw suspended by default when a script is running. If this could be done and appendChild were tuned to make it like insertBefore then you'd have FF3 outperforming FF2 even with all the brilliant improvements in it and THAT would be something.

So that testcase still suffers from the fact that layout hasn't started yet in FF2. So that explains why SVG is "slower" in FF3. If that is the only remaining issue I think we should go ahead and close this bug as starting layout earlier is by design and is going to result in slower performance in a few cases. The proper way to get those cases fast is to build the DOM in a disconnected <g> and then insert that as a single operation, that is always the fastest way to build a big DOM. The suspend/unsuspend "workaround" is only going to get you parts of the win. Feel free to file a separate bug on having suspend/unsuspend be called automatically. Not sure if that would violate any specs or break any use cases. The appendChild issue is known and we already have bug 40988 on that. I'm still very interested to figure out why the numbers in comment 58 are so slow. Though the proper way to get fair testing there is to insert into a <g> (to make FF3 behave more like FF2) or to run things off a timer (to make FF2 behave like FF3).

Our current implementation of suspend/unsuspend is slow. It was automatically called in some cases and removing it was one of the reasons that Duncan's testcases work much faster now.

JS animations that don't yield to the event loop and use forceRedraw to paint frames are *bad*. We don't want to support them.

I think forceRedraw should just be a way to escape early from the timeout set by suspendRedraw. It should not be a way to force synchronous redraw.

I don't see the need for suspendRedraw or unsuspendRedraw at all.

Robert, I couldn't agree more. Their only useful effect is to improve performance, although according to comment #71 there is a tradeoff. Since optimal performance is behaviour everybody wants, surely it ought to be part of the internal DOM implementation where optimisations can be made by those who understand the tradeoff instead of expecting the calling script to do it. I've seen a post somewhere from Boris Zbarsky where he comprehensively illustrates that the SVG specification of these methods is so loose as to make it virtually unimplementable. It's also telling that similar methods are conspicuously absent from the HTML DOM which manages to performs superbly well in Firefox without them.

(In reply to comment #70) > I'm still very interested to figure out why the numbers in comment 58 are so > slow. Though the proper way to get fair testing there is to insert into a <g> > (to make FF3 behave more like FF2) or to run things off a timer (to make FF2 > behave like FF3). > Jonas, I was wrong about having no extensions - that PC actually does have three, they are "Firebug 1.04", "Long Titles 1.2.4" and "SVGZoom and Pan 0.3.62". The first 2 are shown as not compatible. The other difference is that I was using FF 2.0.0.1 as my base comparison, not 2.0.0.11. I'll get rid of all three, upgrade my FF2 install on that machine and see if the numbers look any different.

I totally agree that synchronous JS animations are bad, but I was under the impression that people used them enough that we were forced to support them. If that's not the case I'm totally happy. However this discussion sounds like it belongs in bug 373990. Still sounds like the only remaining "issue" is that we now start layout sooner and so mutations to the document are slower before parsing finishes, but this is by design. Yes, we want to make mutations to the DOM when layout has started faster, but that falls under the general category of "make SVG layout faster" rather than a regression. Would like to hear the results from comment 76, waiting with closing this one as WORKSFORME until we hear back on that.

Jonas, much the same results without the extensions and using FF2.0.0.11, i.e. the same test as in comment 59 but just on a different PC. So to summarise, comparing FF3 Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008030706 Minefield/3.0b5pre vs FF 2.0.0.11 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11) my numbers are Work laptop PC Empty loop FF2: 1236 Latest FF3: 910 Factor 0.736 5K rectangles FF2: 1188 Latest FF3: 1231 Factor 1.036 5K text elements FF2: 1360 Latest FF3: 3687 Factor 2.711 5K HTML divs FF2: 756 Latest FF3: 292 Factor 0.386 Home desktop PC Empty loop FF2: 887 Latest FF3: 667 Factor 0.752 5K rectangles FF2: 1650 Latest FF3: 2957 Factor 1.792 5K text elements FF2: 1481 Latest FF3: 9210 Factor 6.219 5K HTML divs FF2: 984 Latest FF3: 458 Factor 0.475 Strange that the tight javascript loop runs quicker on the second machine but everything else slower. And strange that the SVG slowdown in FF3 is more pronounced on the second machine, especially for text, but there it is. Both matchines are windows XP SP2 - can post hardware and memory details if they're of interest. Remember how far we've come, though - when I filed this bug text elements were slower by a factor of 16 and rectangles a factor of 5. On the second machine these factors are now 6 and 2 and on the first machine 3 and 1 respectively.

Bearing in mind today's batch of comments, shall I file a bug along the lines of "have redraw suspended by default when a script is running" ?

Re comment 79: I do think that automatic suspend/unsuspend conversation belongs elsewhere. But comment 71 and comment 74 seem to indicate that we don't want to do that. Maybe posting to the SVG newsgroup would be a better. Re comment 78: Ugh, sorry, should have been more clear in comment 70. We're really doing an apples-to-oranges comparison in attachment 258652 [details] as we're comparing FF2 without layout to FF3 with layout. To make a fair comparison to see if we have any unknown performance regressions you should use attachment 259084 [details], which would compare FF2 with layout to FF3 with layout). Or one where the nodes are added to a disconnected <g> which never is inserted into the DOM, which would compare FF2 without layout to FF3 without layout. Sorry I wasn't clearer in my previous comment, and thanks for all your testing!

Duncan, I think you should file a bug requesting that all the suspend/unsuspend code be ripped out.

Robert et al, Filed bug 422058 for my suggestion on automatically suspending redraw. I don't know that ripping it all out is an option - the methods are in the SVG spec after all ! I just think the calls could be made automatically as described in the new bug.

(In reply to comment #80) Jonas, it's well proven that FF3 outperforms FF2 by a good margin in an "apples to apples" test, i.e. when both are either doing or not doing layout. There were regressions there but they've all been cleared up in other bugs. I stand by my view that the "apples to oranges" case is a perf regression from the point of view of a script author. Those close to the project understand the reasons and can justify them to each other but all a script author will say is "my script that runs in 1 second on FF2 takes 5 seconds on FF3". That's the comparison people will make, unfair or not. Expect many bug reports along these lines.

The "apples to apples" case is bug 375470

So I take it that even on your home PC all the apples-to-apples tests show FF3 as faster? That is excellent news! Unfortunately I don't see a way that the apples-to-oranges test is ever going to be "fixed". There is no way to satisfy the two constraints: 1. Start layout as soon as we have something to display. 2. Start layout as late as possible to allow faster mutations to the page. We will certainly work on making the overhead from layout smaller, but the overhead is always going to be there. I'm going to mark this bug as WORKSFORME as all apples-to-apples tests are now faster. This bug is so big and covers so many topics that I don't think we're going to get anything more useful out of it. Feel free to file separate bugs on the specific topics that are remaining. Either on starting layout later (though I think that would get WONTFIXed as generally people want to see something on screen as early as possible) or on reducing the overhead of mutating pages where layout is enabled.

Jonas, Yes the "apples to apples" case is definitely faster on the trunk with both of my PC's - that's bug 375470. My point about the "apples to oranges" test is that in your constraint 1 you effectively don't have anything to display until a script finishes execution and therefore there's nothing to lose by suspending layout when a script is running. But that debate needs to go in bug 422058.

What do we mean by "start layout" here? Looking at the testcase, we shouldn't be doing any reflow during the test, just frame construction, and AFAIK that would have been the case in FF2. Jonas, what did you mean when you said we "start layout earlier" on trunk?

Roc, we didn't used to have a frameconstructor created by the time onload fired for XML documents. The call to nsContentSink::StartLayout didn't happen until after onload was finished executing. This changed with bug 18333, we now do the same thing for XML as we always have for HTML.

Oh right. Thanks.

Out of interest, what happens with the HTML DOM implementation ? Does that do anything along the lines of "suspending redraw" behind the scenes while a script is running ? A slightly different case in that there is no explicit "suspendRedraw" for the script author to call AFAIK.

*redraw* is always suspended while script is running. *layout* can happen if a script requests it, e.g. by asking for the size of an element after making some changes to its style. What's happening here is something else again, *frame construction* which always happens immediately on DOM changes.

Right, so if I understand then in a test that doesn't trigger layout (e.g. by asking for an element's size) the HTML DOM methods will not experience any overhead due to redraw or layout code, only frame construction. This seems to differ from the SVG case unless suspendRedraw() is used. Just wondering why it has to be that way.

No, both HTML and SVG get frame construction called, but no layout unless needed. Not actually sure what suspendRedraw turns off in order to get SVG faster. I suggest you bring this up in the newsgroups instead as this is turning into a support discussion rather than a bugfix one. And of course we are open source so you can always check things out yourself if you're wondering how things work. Extra great is if you can provide patches to make things faster too :)

Anyone tried this test case in Google Chrome ? Boy does it go fast...need tamarin to compete with that ?

(In reply to comment #68) > ...forceRedraw() doesn't do anything - try the attached. That seems to have changed. This attachment https://bugzilla.mozilla.org/attachment.cgi?id=308438 (with forceRedraw) now does show re-drawing during script execution (press "Continuous") whereas this one https://bugzilla.mozilla.org/attachment.cgi?id=308435 (without forceRedraw) doesn't (as before). Need to disable JIT otherwise it runs too quickly. Did that get intentionally get fixed ?