1440221 - stylo: ComputedValues in undisplayed map is staled and not reported in memory report

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Xidorn Quan [:xidorn] UTC+11]

So eventually I added some debug code to dump what computed values are not reported, and got a list.

emilio looked at the list and suggested this may be related to undisplay map, which we leave staled when we restyle. So I tried to add some code to also update display:none like how we handle display:contents, and the number of unreported ComputedValues significantly dropped.

It feels like we are holding lots of staled ComputedValues in the undisplay map. We should probably either update them during restyle, or just hook the map into the memory report.

If we are doing the latter, probably we want to give it a separate path so that we know how much memory we are wasting for staled styles.

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 1440221 - Track stale computed values and style structs held by undisplayed maps in memory report.

Review commit: https://reviewboard.mozilla.org/r/222320/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222320/

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

It's... interesting... I only see about ~180KB reported on stale computed values, which is far less than what I saw in bug 1421374 comment 9, and that was even collected after I pressed "Minimize memory usage"...

So I tried to press "Minimize memory usage" as well, and the stale computed values suddenly grow to ~300KB... so we are basically adding memory usage on this specific area when we try to minimize memory usage :/

We probably should update the undisplayed maps when we minimize memory usage...

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8953044 [details]
Bug 1440221 - Track stale computed values and style structs held by undisplayed maps in memory report.

https://reviewboard.mozilla.org/r/222320/#review228236

Looks fine to me. I wouldn't worry much about the undisplayed maps, they'll go away once we get rid of the old style system in https://bugzilla.mozilla.org/show_bug.cgi?id=1303605.

Comment 4

[Nicholas Nethercote [inactive]]

Comment on attachment 8953044 [details]
Bug 1440221 - Track stale computed values and style structs held by undisplayed maps in memory report.

https://reviewboard.mozilla.org/r/222320/#review228890

::: layout/base/nsCSSFrameConstructor.cpp:13184
(Diff revision 1)
> +{
> +  if (nsIFrame* rootFrame = GetRootFrame()) {
> +    rootFrame->AddSizeOfExcludingThisForTree(aSizes);
> +  }
> +
> +  // This must be done after collecting from the frame tree, since frame

"Measuring" would be a better word than "collecting". For a second I thought it was talking about garbage collection of some kind.

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8953044 [details]
Bug 1440221 - Track stale computed values and style structs held by undisplayed maps in memory report.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222320/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f7a18f629995
Track stale computed values and style structs held by undisplayed maps in memory report. r=emilio,njn

Comment 7

[Cosmin Sabou [:CosminS]]

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