723441 - SVG to IMG Html Element rendering performance (very slow handling of <use> targeting a data: document)

Status: RESOLVED FIXED

(Core :: SVG, defect, P2)

Description

[Alex]

Name: Firefox
Version: 13.0a1
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:13.0a1) Gecko/20120201 Firefox/13.0a1

Reproducible: Always
Steps to reproduce: 
go to http://a-khromenkov.narod.ru/

Actual Results:
Huge memory (up to 2Gb) and CPU (up to 100%) consumption and then FireFox crashing.

Expected Results:
Download svg file through ajax and render svg to canvas through img html element.

Comment 1

[Thomas Ahlblom]

100% CPU load and high memory usage, but page renders eventually without crash:
Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20100101 Firefox/10.0
Mozilla/5.0 (X11; Linux x86_64; rv:13.0a1) Gecko/20120201 Firefox/13.0a1

Doesn't render (without high CPU and memory usage, no crash)
Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.2.26) Gecko/20120128 Firefox/3.6.26
Opera/9.80 (X11; Linux x86_64; U; en) Presto/2.10.229 Version/11.61


WFM, renders quickly without apparent problems:
Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/535.7 (KHTML, like Gecko) Chrome/16.0.912.77 Safari/535.7

Comment 2

[Matthias Versen [:Matti]]

The crash could be caused by the maximum memory that a 32bit process can allocate.

Comment 3

[Matthias Versen [:Matti]]

Last good nightly: 2011-05-21
First bad nightly: 2011-05-22

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=21c304c5f351&tochan
ge=52f72d71dc59

good=not rendering but without hang

Comment 4

[Boris Zbarsky [:bzbarsky]]

So we have a huge data: URI containing a bunch of <use xlink:href="#glyph7-24" x="146.110555" y="526.517229"/> type things.

Now what <use> will do is clone the thing being targeted and set xml:base on the clone, as a string, to the base URI of the referenced element.  This was apparently done to allow cross-document <use> in bug 433616.  But in this case, what that means is that we have a copy of the whole data URI (about 520KB) for each character in the SVG.  There are 2912 characters, so that's about 1.5GB of RAM just to store those attributes.  And of course there could be other memory allocations that happen too.

I'm going to try something that might help.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Oh, sorry.  I lied.  It's 3GB of RAM to store the attributes, because each char is 2 bytes.  So that probably just runs out of address space on 32-bit Windows.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

We do have tests for this base URI stuff; an initial patch version actually failed them.  This passes the SVG reftests and makes the linked-to performance test nice and fast

Comment 7

[Daniel Holbert [:dholbert]]

Comment on attachment 593923 [details] [diff] [review]
Proposed fix

>diff --git a/content/base/public/nsINode.h b/content/base/public/nsINode.h
>--- a/content/base/public/nsINode.h
>+++ b/content/base/public/nsINode.h
>@@ -989,16 +990,30 @@ public:
>+  /**
>+   * The explicit base URI, if set, otherwise null
>+   */
>+  nsIURI* GetExplicitBaseURI() const {
>+    if (HasExplicitBaseURI()) {
>+      return static_cast<nsIURI*>(GetProperty(nsGkAtoms::baseURIProperty));
>+    }
>+    return nsnull;
>+  }

Perhaps GetExplicitBaseURI should be a protected method?  It's only called from within nsGenericElement::GetBaseURI, and it's probably worth keeping it that way.

That'd also make it less concerning / confusing that this method's return-type differs only slightly from GetBaseURI  (raw nsIURI* instead of an already_AddRefed<nsIURI>)

Other than that possible protected-ness change, r=me.

Comment 8

[Daniel Holbert [:dholbert]]

(Can we add a crashtest of some sort for this, too?  Perhaps something that intentionally tries to run out of address space, per comment 5?)

Comment 9

[Boris Zbarsky [:bzbarsky]]

> Perhaps GetExplicitBaseURI should be a protected method? 

I can do that.

> Can we add a crashtest of some sort for this, too? 

Yes.  In fact, the original SVG from this bug should work.  I'll do that.

Comment 10

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/e66c1699d0ae

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e66c1699d0ae