Closed
Bug 1421374
Opened 7 years ago
Closed 7 years ago
stylo: Regression on AWSY when enabling stylo-chrome
Categories
(Core :: CSS Parsing and Computation, defect, P1)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox57 | --- | unaffected |
firefox58 | --- | unaffected |
firefox59 | --- | disabled |
firefox60 | --- | fixed |
People
(Reporter: xidorn, Unassigned)
References
(Depends on 2 open bugs)
Details
(Whiteboard: [MemShrink:P2])
Attachments
(1 file)
6.11 MB,
text/plain
|
Details |
See this comparison: https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=af1d0a3e3a6a&newProject=try&newRevision=11876e34b9c4&framework=4
It seems there are some 2%~3% regression on heap unclassified, which probably means there are more things we need to track in memory report, but may not be a serious blocker.
There are also some "uncertain" ones (which looks really like actual regressions) on Images, and moderate "uncertain" regression on Explicit memory. Not sure whether we need to care about them.
Comment 1•7 years ago
|
||
Bobby says a 2-3% regression might be a blocker. We should investigate further.
status-firefox57:
--- → unaffected
status-firefox58:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Priority: -- → P1
Summary: stylo: Regression on awsy when enabling stylo-chrome → stylo: Regression on AWSY when enabling stylo-chrome
Reporter | ||
Comment 2•7 years ago
|
||
So in comparison in some later try pushes, it seems that the Images is actually a main regression. It has a significant 10% regression among platforms.
I had a look at the regression, and from the comparison of subtests, it seems that main regressions are from fresh start, fresh start [+30s], and tabs closed [+30s, forced gc], and the regression is about 200KB~300KB. From direct comparison of their memory report, it seems that the regression is from SVG images used in the chrome document.
This is from one of the memory report comparison:
─212,368 B (52.37%) -- images/chrome/vector/used
├───23,440 B (05.78%) ── image(16x16, chrome://browser/skin/tracking-protection-16.svg#enabled)/source
├───11,936 B (02.94%) ── image(20x20, chrome://global/skin/icons/close.svg)/source
├───11,632 B (02.87%) ── image(12x12, chrome://browser/skin/window-controls/restore.svg)/source
├───11,632 B (02.87%) ── image(16x16, chrome://browser/skin/sidebars.svg)/source
├───11,120 B (02.74%) ── image(12x12, chrome://browser/skin/window-controls/maximize.svg)/source
├───10,976 B (02.71%) ── image(12x12, chrome://browser/skin/window-controls/close.svg)/source
├───10,976 B (02.71%) ── image(12x12, chrome://browser/skin/window-controls/minimize.svg)/source
├───10,976 B (02.71%) ── image(16x16, chrome://browser/skin/arrow-left.svg)/source
├───10,976 B (02.71%) ── image(16x16, chrome://browser/skin/back.svg)/source
├───10,976 B (02.71%) ── image(16x16, chrome://browser/skin/chevron.svg)/source
├───10,976 B (02.71%) ── image(16x16, chrome://browser/skin/forward.svg)/source
├───10,976 B (02.71%) ── image(16x16, chrome://browser/skin/home.svg)/source
├───10,976 B (02.71%) ── image(16x16, chrome://browser/skin/library.svg)/source
├───10,976 B (02.71%) ── image(16x16, chrome://browser/skin/menu.svg)/source
├───10,976 B (02.71%) ── image(16x16, chrome://browser/skin/reload.svg)/source
├───10,976 B (02.71%) ── image(16x16, chrome://browser/skin/search-glass.svg)/source
├───10,976 B (02.71%) ── image(16x16, chrome://global/skin/icons/arrow-dropdown-16.svg)/source
└───10,896 B (02.69%) ── image(16x16, chrome://browser/skin/tabbrowser/newtab.svg)/source
It matches the number shown in perfherder.
(I was confused that the numbers in perfherder show a much larger regression, but today I realized that it was because the number shown in the overview table is geometric mean of numbers on subtests, and thus it can only shows an average regression percentage, and its accurate number is meaningless.)
Reporter | ||
Comment 3•7 years ago
|
||
So basically the 10% regressions on Images are from multiple 20~30% regressions on the subtests, because of that we use more memory for each document.
We would also need DMD to understand where are the additional unclassified heap are from.
Reporter | ||
Comment 4•7 years ago
|
||
From the DMD, it seems that most unreported memory usage are from XBL, including the stylesheets, stylist, and style structs. (I have no idea, though, why style structs are not counted.)
I'm not sure why stylo takes more memory here. Maybe stylist is larger than rule processors?
Reporter | ||
Comment 5•7 years ago
|
||
Most of the focus were on talos regression on stylo-chrome, but we do still have regression on awsy.
We still regress ~4% on unclassified heap, and ~10% on images, which is kinda sad.
Reporter | ||
Comment 6•7 years ago
|
||
To make things a bit clearer, the ~10% images regression is roughly a fixed 100KB~200KB regression on the chrome process (for SVG-as-images used in the chrome), and the ~4% unclassified heap regression is roughly a fixed 1MB~2MB regression on the chrome process (maybe mostly XBL stuff), so obviously the unclassified heap regression is actually worse than the images regression despite that its percentage is smaller due to its larger base number.
Updated•7 years ago
|
Whiteboard: [MemShrink]
Reporter | ||
Comment 7•7 years ago
|
||
Eric, based on comment 6, do you think this is a blocker for stylo-chrome? If yes, how much should we try to shrink it before we can ship stylo-chrome?
Flags: needinfo?(erahm)
Comment 8•7 years ago
|
||
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #7)
> Eric, based on comment 6, do you think this is a blocker for stylo-chrome?
> If yes, how much should we try to shrink it before we can ship stylo-chrome?
I'd say landing measurements for the "XBL stuff" should be a blocker.
Once we have measurements, it might be easier to diff the results and determine *why* we're regressing and see if there's something we can do about it. All said, 1-2MB is bad but if it's limited to the chrome process I'm less concerned.
Flags: needinfo?(erahm)
Reporter | ||
Comment 9•7 years ago
|
||
Hmmmm... even after bug 1438497 and bug 1439507, there still seems to be some much more unreported memory usage for stylo-chrome.
There are several weird things from the DMD report:
I can see some ComputedValues as well as style structs which are generated during traversal. That is responsible for at least ~350KB unreported memory usage. Gecko has similar problem as well, I can see unreported memory from nsIPresShell::AllocateByObjectID called by nsRuleNode::Compute*Data. Given that presshell arena is directly taken accounted from the document level (via nsIPresShell::AddSizeOfIncludingThis), it seems that there are some unreported document?
I can also see some binding stuff, specificall AuthorStyles shows up in the report, and also some stylesheets data parsed from file (via Servo_StyleSheet_FromUTF8Bytes). They are probably not a lot, but they may provide some clue.
I cannot really see how stylo-chrome is regressing a lot on unclassified heap, actually... There doesn't seem to be anything big unreported...
Reporter | ||
Comment 10•7 years ago
|
||
This is a DMD diff report I got for a debug build after patches in bug 1438497 and bug 1439507 applied.
Reporter | ||
Comment 11•7 years ago
|
||
So, checking with the report in comment 10 again, it seems I was partially wrong in comment 9 that, Gecko doesn't have unreported memory for computed values. The blocks show up in the report are from a hash table for debugging, not the arena allocation itself. There is no nsPresArena::Allocate I can find in an unreported stack with nsRuleNode::Compute*Data.
So it looks like Stylo's computed value reporting indeed has some problem itself. It is not a document level missing.
Updated•7 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P2]
Comment 12•7 years ago
|
||
Xidorn says the heap-unclassified regression is now mostly fixed or classified. The primary issue is SVG image memory in the parent process (bug 1426295), so we can close this meta bug and track bug 1426295 on its own.
Comment 13•7 years ago
|
||
stylo-chrome is off in 59.
You need to log in
before you can comment on or make changes to this bug.
Description
•