1758974 - High cascade_rules heap-unclassified on Gitea diff page

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[eladyn]

Attachment: gitea_diff_firefox.png

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:99.0) Gecko/20100101 Firefox/99.0

Steps to reproduce:

1. open https://codeberg.org/gitnex/GitNex/pulls/1034/files or other Gitea pages with huge diffs
2. wait for it to load entirely

Actual results:

The memory shoots up by several GB multiple times while loading the page and settles a bit after the page is done loading but remains very high in general. (see screenshot)

Expected results:

The page should not require several GB to load, since both GNOME Web (WebkitGTK) and Chromium only require less than 1 GB (GNOME Web is actually fine with something around 100 MB). Apart from that, they also take considerably shorter to load the page in its entirety.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Mayank Bansal]

I got to 4GB memoey use, with 3.6GB as heap-unclassified.

Comment 3

[Mayank Bansal]

Attachment: memory-report.json.gz

Comment 4

[Andrew McCreight [:mccr8]]

Thanks for the bug report. I was able to reproduce this on OSX Nightly.

I ran it with DMD=1, got a DMD log, then then ran dmd.py on the log for the process containing the page, and these are the two top unreported entries:

Unreported {
57,717 blocks in heap block record 1 of 2,189
2,364,088,320 bytes (2,274,511,512 requested / 89,576,808 slop)
Individual block sizes: 40,960 x 57,717
60.18% of the heap (60.18% cumulative)
65.40% of unreported (65.40% cumulative)
Allocated at {
#01: replace_realloc(void*, unsigned long) (/Users/andrewmccreight/mc/memory/replace/dmd/DMD.cpp:0)
#02: alloc::raw_vec::RawVec<T,A>::shrink_to_fit (/rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/alloc/src/raw_vec.rs:397)
#03: style::custom_properties::CustomPropertiesBuilder::build (servo/components/style/custom_properties.rs:732)
#04: style::properties::cascade::cascade_rules (/Users/andrewmccreight/mc/servo/components/style/properties/cascade.rs:218)
#05: style::properties::cascade::cascade (/Users/andrewmccreight/mc/servo/components/style/properties/cascade.rs:96)
#06: style::stylist::Stylist::cascade_style_and_visited (/Users/andrewmccreight/mc/servo/components/style/stylist.rs:1061)
#07: style::style_resolver::StyleResolverForElement<E>::cascade_style_and_visited (/Users/andrewmccreight/mc/servo/components/style/style_resolver.rs:346)
#08: style::style_resolver::StyleResolverForElement<E>::cascade_primary_style (/Users/andrewmccreight/mc/servo/components/style/style_resolver.rs:243)
}
}

Unreported {
57,716 blocks in heap block record 2 of 2,189
1,182,023,680 bytes (1,064,744,768 requested / 117,278,912 slop)
Individual block sizes: 20,480 x 57,716
30.09% of the heap (90.27% cumulative)
32.70% of unreported (98.11% cumulative)
Allocated at {
#01: replace_malloc(unsigned long) (/Users/andrewmccreight/mc/memory/replace/dmd/DMD.cpp:1092)
#02: <hashbrown::raw::inner::RawTable<T> as core::clone::Clone>::clone (/Users/andrewmccreight/mc/third_party/rust/hashbrown/src/raw/mod.rs:1193)
#03: <indexmap::map::core::IndexMapCore<K,V> as core::clone::Clone>::clone (/Users/andrewmccreight/mc/third_party/rust/indexmap/src/map/core.rs:65)
#04: <indexmap::map::IndexMap<K,V,S> as core::clone::Clone>::clone (/Users/andrewmccreight/mc/third_party/rust/indexmap/src/map.rs:90)
#05: style::custom_properties::CustomPropertiesBuilder::cascade (servo/components/style/custom_properties.rs:637)
#06: style::properties::cascade::cascade_rules (/Users/andrewmccreight/mc/servo/components/style/properties/cascade.rs:218)
#07: style::properties::cascade::cascade (/Users/andrewmccreight/mc/servo/components/style/properties/cascade.rs:96)
#08: style::stylist::Stylist::cascade_style_and_visited (/Users/andrewmccreight/mc/servo/components/style/stylist.rs:1061)
}
}

This looks like style stuff.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

So that page has a gazillion custom properties on the root (nothing out of the ordinary) but then also has:

:root * {
  --fonts-regular: var(--fonts-override, var(--fonts-proportional)), "Noto Sans", "Liberation Sans", sans-serif, var(--fonts-emoji);
}

That is causing us to copy-on-write that map for every single element on the page, ouch :(

Comment 6

[Emilio Cobos Álvarez (:emilio)]

This works on Chrome because they store root custom properties separately. And this works on WebKit because of this, which is something similar to what my patch is going to do actually.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1758974 - Make custom-property substitution only traverse variables which could have references. r=#style,#layout-reviewers

If a name is not in self.seen, it means we've inherited it from our
parent. That in turn means that it can't have any variable reference
(because we inherit the computed variables) and we can skip the work of
traversing it, as we'd hit the early-return in traverse() anyways.

This doesn't fix the memory usage issue of this page, but should
significantly reduce the time we spend iterating over custom properties.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1758974 - Reuse inherited custom properties if they didn't change after resolution. r=#style,#layout-reviewers

This should be cheap and gives us a lot of memory savings in this page.
WebKit implements a similar optimization.

Depends on D140825

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7155ccc1c9fe
Make custom-property substitution only traverse variables which could have references. r=jwatt
https://hg.mozilla.org/integration/autoland/rev/da4c45d3e93e
Reuse inherited custom properties if they didn't change after resolution. r=jwatt

Comment 10

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/7155ccc1c9fe
https://hg.mozilla.org/mozilla-central/rev/da4c45d3e93e

Comment 11

[Mayank Bansal]

The memory use is 300mb-400mb on the latest nightly. This is fixed.