Last Comment Bug 646510 - Perf regression since Firefox 3.6 with SVG Polyline element
: Perf regression since Firefox 3.6 with SVG Polyline element
Status: RESOLVED FIXED
[in-the-wild] [external-report]
: perf, regression, testcase
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: ---
Assigned To: Brian Birtles (:birtles)
:
Mentors:
http://mansoft.nl/spiro/spiro.svg
Depends on: 629200
Blocks: 522308
  Show dependency treegraph
 
Reported: 2011-03-30 09:42 PDT by hfmanson
Modified: 2013-10-29 19:17 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
original testcase (5.37 KB, image/svg+xml)
2011-03-31 11:31 PDT, Daniel Holbert [:dholbert]
no flags Details

Description hfmanson 2011-03-30 09:42:47 PDT
User-Agent:       Mozilla/5.0 (Windows NT 6.0; WOW64; rv:2.0) Gecko/20100101 Firefox/4.0
Build Identifier: Mozilla/5.0 (Windows NT 6.0; WOW64; rv:2.0) Gecko/20100101 Firefox/4.0

This HTML page contains elements like <object data="spiro.svg?figuur=Fig1.xml" type="image/svg+xml" align="baseline" width="400" height="400">
</object> The references themselves don't hang, e.g. http://mansoft.nl/spiro/spiro.svg?figuur=Fig1.xml


Reproducible: Always

Steps to Reproduce:
Visit http://mansoft.nl/spiro/spiro.htm
Actual Results:  
Hangs firefox

Expected Results:  
some spirograph pictures. Try with Google Chrome or Opera
Comment 1 hfmanson 2011-03-30 13:23:05 PDT
Update: When you wait several seconds the pictures appear, thus firefox doesn't really hang but takes a long time for rendering which is a regression
Comment 2 Matthias Versen [:Matti] 2011-03-30 14:19:36 PDT
I get a slow script warning for 
Script: http://mansoft.nl/spiro/spiro.js:48

SVG or JS ?
Comment 3 hfmanson 2011-03-31 04:25:11 PDT
The problem also occurs on a single SVGs, it seems the problem lies in XMLHttpRequest, e.g. http://www.mansoft.nl/spiro/spiro.svg?figuur=Fig4.xml
Comment 4 hfmanson 2011-03-31 07:01:53 PDT
I updated spiro.svg which now shows the problem directly. XMLHttpRequest is not used when no querystring is present. See http://www.mansoft.nl/spiro/spiro.svg
So it is a pure SVG problem, not XMLHttpRequest unlike I stated in Comment 3.
Comment 5 Daniel Holbert [:dholbert] 2011-03-31 11:31:04 PDT
Created attachment 523367 [details]
original testcase

Here's the original testcase (with js file merged into SVG file).

Confirming -- this takes ~7 seconds to load in a mozilla-central nightly, whereas it takes less than 1 second to load in Firefox 3.6.16

Mozilla/5.0 (X11; Linux i686; rv:2.2a1pre) Gecko/20110327 Firefox/4.2a1pre
Comment 6 Daniel Holbert [:dholbert] 2011-03-31 11:32:24 PDT
(In reply to comment #5)
> Here's the original testcase (with js file merged into SVG file).
(By "original testcase", I meant "the file spiro.svg mentioned in comment 4")
Comment 7 Daniel Holbert [:dholbert] 2011-03-31 11:57:23 PDT
Regression range:
Mozilla/5.0 (X11; Linux x86_64; rv:2.0b8pre) Gecko/20101208 Firefox/4.0b8pre
Mozilla/5.0 (X11; Linux x86_64; rv:2.0b8pre) Gecko/20101209 Firefox/4.0b8pre
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=95452499f3d6&tochange=11e328a49e0a

Since the testcase seems to make extensive use of 'polyline', I'd guess that Bug 522308 is the most likely regressor.
Comment 8 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-03-31 14:25:12 PDT
Brian, I hope you can find time to look at this.
Comment 9 Jonathan Watt [:jwatt] 2011-03-31 15:30:55 PDT
Did a quick Shark profile of this.

97.7%  DOMSVGPointList::AppendItem
97.7%    nsSVGElement::DidChangePointList
97.1%      SVGPointList::GetValueAsString
91.4%        nsTextFormatter::snprintf

Our problem here seems to be that we changed from a notification mechanism that required no serialization, to a notification mechanism that does. Previously we would have had something more like this:

nsSVGPointList::AppendElement
  nsSVGValue::DidModify
    nsSVGValue::NotifyObservers
      nsSVGElement::DidModifySVGObservable
        nsAttrValue::nsAttrValue(nsISVGValue*)
        nsGenericElement::SetAttrAndNotify

Creating the nsAttrValue with a nsISVGValue* (the nsSVGPointList) avoided any serialization.

The fix for this bug is probably fixing bug 629200.
Comment 10 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-03-31 15:52:05 PDT
Yes, I guess so.
Comment 11 Jonathan Watt [:jwatt] 2011-03-31 15:55:01 PDT
(It's maybe worth noting that the old world system, despite being quicker, wasn't completely correct. It would have had a similar problem to the one indicated in bug 610990 comment 16, meaning that restyles and mutation events would have been broken back then.)
Comment 12 Brian Birtles (:birtles) 2011-04-04 17:06:19 PDT
(In reply to comment #8)
> Brian, I hope you can find time to look at this.

Sorry, just back from being away. Sure, I'll take this. Presumably I should also take bug 629200 as well since that seems to be the root cause. Please feel free to steal from me though.
Comment 13 Robert Longson 2012-02-16 03:14:24 PST
Back up to speed again now post bug 629200
Comment 14 hfmanson 2012-02-18 10:08:06 PST
Confirming fix. My original webpage http://www.mansoft.nl/spiro/spiro.htm now renders in less than a second.

Note You need to log in before you can comment on or make changes to this bug.