1957263 - Explosive OOM on a Codepen Demo (https://codepen.io/Mikhail-Bespalov/pen/LYwoyWG) with a 7MB SVG file. Chrome handles it easily.

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Mayank Bansal]

Attachment: Whisky_distilleries_and_regions_in_Scotland.svg

Go to https://codepen.io/Mikhail-Bespalov/pen/LYwoyWG
Inside the demo, browse to the attached SVG file.

AR: 4GB memory use within 2 seconds. Then OOM crash / system getting unstable. The rapid OOM is specific to this SVG only. With a 100MB+ SVG, the explosive OOM does not occur. It merely takes a loong time.
ER: Not so, like Chrome.

Partial profile: https://share.firefox.dev/3Y7eIZO (partial)

Profile shows NS_URI and reflow. Not sure which component this should go to.
cc : longsonr and dholbert.

Comment 1

[Mayank Bansal]

Attachment: about:support

Comment 2

[Mayank Bansal]

Bisection: Changelog
Bug 1944365: apply code formatting via Lando

ignore-this-changeset

2025-03-30T07:44:42.012000: DEBUG : Did not find a branch, checking all integration branches
2025-03-30T07:44:42.015000: INFO : The bisection is done.
2025-03-30T07:44:42.016000: INFO : Stopped

---

Maybe some sort of oom check or upper bound on memory is needed?

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Running this locally it appears that we're ending up allocating ~10Gb of heap-unclassified with roughly the following lines:

document.getElementById('dispImageRed').setAttribute('href', dataURL);
document.getElementById('dispImageGreen').setAttribute('href', dataURL);
document.getElementById('dispImageBlue').setAttribute('href', dataURL);

Somewhat notably, doing this for even a single line appears to consume the full ~10Gb, and the additional copies of the image for additional channels do not appear to consume the same memory, which suggests this probably isn't a per-filter leak?

---

The data URI being used here appears to have been built by a FileReader. This string is being built and returned as utf-16 (https://searchfox.org/mozilla-central/rev/5fb48bf50516ed2529d533e5dfe49b4752efb8b8/dom/file/FileReader.cpp#486-497), and the JS engine should hold onto the underlying string buffer. This should also avoid a copy when we're passing the string in to setAttribute, though due to the buffer is not being precisely sized, we may end up needing to make a second copy for each call to setAttribute here (https://searchfox.org/mozilla-central/rev/5fb48bf50516ed2529d533e5dfe49b4752efb8b8/dom/base/nsAttrValue.cpp#2100-2106).

The actual load process in SVGFEImageElement (also SVGImageElement) does a few of wasteful string conversions, though not clearly enough to balloon to 10Gb, and most of these copies should be temporary and not sticking around long-term:

1. 2 more copies will be created by the call to NS_MakeAbsoluteURI which will convert the URI string to utf-8 to call Resolve then immediately back to utf-16 (https://searchfox.org/mozilla-central/rev/5fb48bf50516ed2529d533e5dfe49b4752efb8b8/netwerk/base/nsNetUtil.cpp#662,664), though these should be temporary.
2. The string is copied back to utf-8 to resolve into a nsIURI to do a URI equality check with baseURI (https://searchfox.org/mozilla-central/rev/5fb48bf50516ed2529d533e5dfe49b4752efb8b8/dom/svg/SVGFEImageElement.cpp#76).
3. Immediately afterwards, the URI is once again copied back to utf-8 in LoadImage to actually start the load (https://searchfox.org/mozilla-central/rev/5fb48bf50516ed2529d533e5dfe49b4752efb8b8/dom/base/nsImageLoadingContent.cpp#998-1002)
4. When the image is actually being loaded, we are unfortunately forced to perform an additional copy as we unescape the base64-encoded data (https://searchfox.org/mozilla-central/rev/5fb48bf50516ed2529d533e5dfe49b4752efb8b8/netwerk/protocol/data/nsDataChannel.cpp#93-101). After this point, the data should load like any normal SVG image.

AFAICT none of these should be new overhead from the changes in bug 1944365. The data URIs here do not have query or reference components, and it doesn't seem like the code was calling any of the methods which used to be slightly more memory efficient before the changes in that bug.

Notably, within the SVG attached to this bug there are a number of other data: URIs for PNGs and JPEGs. Perhaps there is some inefficiency in our SVG / CSS filter code which is causing us to make many copies of this and leak them? Leaving a ni? for :emilio to perhaps take a quick look as I'm unfamiliar with that part of the DOM/layout/styling.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

I think the main issue is in the SVG file itself. There's a lot of local URIs like:

  <use id="use12114" xlink:href="#use12050" transform="translate(16.797 -2.3581e-7)" height="100%" width="100%" y="0" x="0"/>
  <use id="use12050" xlink:href="#use9212" transform="translate(66.406 -86.134)" height="100%" width="100%" y="0" x="0"/>

That said, maybe it's not <use>, because afaiui we should deal with local refs here without involving a copy: https://searchfox.org/mozilla-central/rev/5fb48bf50516ed2529d533e5dfe49b4752efb8b8/dom/svg/SVGUseElement.cpp#565-566

But I don't think that's true of this tho? That ends up here which would explain (and probably <svg:use> helps with the explosion.

This is on a massive document, which means that it already has a giant data URI... And that already goes to here which ends up just stripping it and finding by id here...

My understanding is that before your patch, those would share the buffer for the non-ref part, but now each of those end up causing a copy, right? Or am I missing something?

It seems like the right fix would be to teach the SVG rendering observer code to deal with local ref URIs explicitly...

Comment 5

[Nika Layzell [:nika] (ni? for response)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

My understanding is that before your patch, those would share the buffer for the non-ref part, but now each of those end up causing a copy, right? Or am I missing something?

Ah shucks, yes you're correct on that. When I was looking into consumers of the mutating methods on nsIURI I didn't consider this case with resolving a reference from a base URI on a large data URI. This is a pretty unfortunate oversight on my part.

Theoretically this could be causing problems already for cases like large http:// URIs with SVGs, though those rarely get nearly as large as data URIs. I'm not sure it makes sense to put all of the responsibility for this onto the SVG engine given that this is an operation which used to be efficient.

Perhaps there is some optimization we could do where, in the case where the mSpec buffer is both large and shared, the nsSimpleURI code could manage an "overlay" for the query & reference components? I worry a bit about the complexity of such a change, but perhaps it's manageable.

(as a side note, it would be nice if these URIs were reported to the memory reporter, rather than in heap-unclassified, but that's an unrelated project/problem).

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1944365

Comment 7

[Mayank Bansal]

I dont get the memory increase now : https://share.firefox.dev/3RyJQOd
Is there anything more to do here? If no, I am happy to close this as fixed.

Comment 8

[Nika Layzell [:nika] (ni? for response)]

(In reply to Mayank Bansal from comment #7)

I dont get the memory increase now : https://share.firefox.dev/3RyJQOd
Is there anything more to do here? If no, I am happy to close this as fixed.

Thanks for confirming this issue isn't occuring for you anymore!