Closed Bug 723441 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: SVG, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: go-alex-go, Assigned: bzbarsky)

References

()

Details

(Keywords: hang, perf, Whiteboard: [in-the-wild] [external-report])

Attachments

(1 file)

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.
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
OS: Windows XP → All
Hardware: x86 → All
Version: 13 Branch → Trunk
The crash could be caused by the maximum memory that a 32bit process can allocate.
Severity: blocker → critical
Product: Firefox → Core
QA Contact: general → general
Target Milestone: Firefox 11 → ---
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
Status: UNCONFIRMED → NEW
Component: General → SVG
Ever confirmed: true
QA Contact: general → general
Keywords: hang, perf
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.
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.
Attached patch Proposed fixSplinter Review
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
Attachment #593923 - Flags: review?(dholbert)
Assignee: nobody → bzbarsky
Priority: -- → P2
Summary: SVG to IMG Html Element rendering performance → SVG to IMG Html Element rendering performance (very slow handling of <use> targeting a data: document)
Whiteboard: [need review]
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.
Attachment #593923 - Flags: review?(dholbert) → review+
(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?)
Flags: in-testsuite?
> 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.
http://hg.mozilla.org/integration/mozilla-inbound/rev/e66c1699d0ae
Flags: in-testsuite? → in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/e66c1699d0ae
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [in-the-wild] [external-report]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: