646510 - Perf regression since Firefox 3.6 with SVG Polyline element

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[hfmanson]

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]

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]]

I get a slow script warning for 
Script: http://mansoft.nl/spiro/spiro.js:48

SVG or JS ?

Comment 3

[hfmanson]

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]

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]]

Attachment: 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]]

(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]]

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) (email my personal email if necessary)]

Brian, I hope you can find time to look at this.

Comment 9

[Jonathan Watt [:jwatt]]

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) (email my personal email if necessary)]

Yes, I guess so.

Comment 11

[Jonathan Watt [:jwatt]]

(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)]

(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 [:longsonr]]

Back up to speed again now post bug 629200

Comment 14

[hfmanson]

Confirming fix. My original webpage http://www.mansoft.nl/spiro/spiro.htm now renders in less than a second.