Closed Bug 1427681 Opened 3 years ago Closed 3 years ago

stylo: There is unreasonable number of nsStyleDisplay instances are kept for SVG images

Categories

(Core :: CSS Parsing and Computation, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- wontfix
firefox58 --- affected
firefox59 --- affected

People

(Reporter: xidorn, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

See the analysis in bug 1426295 comment 5, the document of tracking-protection-16.svg#enabled contains only 28 elements to style, but stylo generates 42 nsStyleDisplay instances, while Gecko only produces 24.

We should investigate why and try to reduce it at least to match the number of elements.

(I'll try to investigate it tomorrow.)
So, the analysis from bug 1426295 isn't really useful for stylo-chrome memory usage.

There are 15 extra instances of nsStyleDisplay when the SVG file is opened as a document rather than an image, which comes from the scrollbar. Reducing those, an SVG-as-image tracking-protection-16.svg#enabled only has 27 distinct nsStyleDisplay instances, which is much closer to the estimation, although the actual result doesn't match exactly what I described originally.

The style display instances my local log catches are:
<svg>
<svg>::-moz-text
<style>
<defs>
<rect>
<use>
<path>
<use>
<path>
<use>
<path>
<rect>
<use>
<path>
<use>
<path>
<use>
<path>
<line>
<line>
<use>
<path>
<g>
<div>
Viewport
HTMLScroll
Canvas

The element part of the log is listed in pre-order. There are something weird from the log, though.

Firstly, none of the <mask> elements in the file has style display, but their children have. Similarly, <path> elements directly inside <defs> don't have style display, but their clone under <use> elements has. However, the <line> under <defs> seems to have one. I have no idea why this happens.

Also, if Gecko also has the 15 extra instances of nsStyleDisplay when SVG is opened as document, it's going to have only 9 distinct nsStyleDisplay in image, which is extremely low. I'm going to check how many there actually are.
So, yes, in SVG-as-image, Gecko only produces 8 nsStyleDisplay instances in total for the given image... I cannot even tell how can it be achieved...
So... I did some frame tree dumping with additional code to show the display struct each frame gets, it seems that every SVG element in tracking-protection-16.svg#enabled except the root <svg>, but including all the anonymous content inside <use>, share the same display struct, and all the anonymous boxes outside the root <svg> element shares the same display struct, so it has very small number of unique display style.

It sounds like Stylo's rule cache should be able to do this kind of sharing as well. I'll have a look at why it isn't effective.
Servo shares reset structs in an all-or-nothing way, so that could / should explain it.
(That is, either it shares all the structs, or it shares none)
I've figured out the reason, which is that the mapped attributes on the SVG elements form a level of rule node, which stops any sharing from the rule cache. Removing those attributes significantly improves sharing in that file.

Mapped attributes on inherited properties should be quite common in SVG, so I'm trying to skip that kind of rule nodes before querying the rule cache, which should help.

Another problem here is that, we cannot share the same style struct between different anonymous content tree under each <use> element, because we style the new subtree immediately each time we construct them. I'm wondering whether we can defer styling them so that they can also reuse styles from the rule cache.
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #6)
> Another problem here is that, we cannot share the same style struct between
> different anonymous content tree under each <use> element, because we style
> the new subtree immediately each time we construct them. I'm wondering
> whether we can defer styling them so that they can also reuse styles from
> the rule cache.

Yeah, we should move <use> to use Shadow DOM instead of NAC, and then this would be fixed. We need to do that anyway to support display: contents in <use>, but I don't think it's trivial.
P2 because it sounds like we should fix this to reduce memory usage before we ship Stylo-chrome.
Priority: -- → P2
After the PR in comment 9, we should be sharing more on SVG elements, but <use> subtree would still not share styles with anything else, so Stylo would still be using more than Gecko, but that can probably be in a different bug, and maybe switching that to shadow dom from nac is the way to fix it as mentioned by emilio in comment 7.

Given this, I'm going to mark this as FIXED.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.