1395576 - 9.6 - 9.76% Images (linux64-stylo, macosx64-stylo) regression on push 6b9d06ba6f769234530ae67d8353377d58a93fd0 (Thu Aug 31 2017)

Status: RESOLVED WONTFIX

(Core :: CSS Parsing and Computation, defect)

Description

[Ionuț Goldan [:igoldan]]

We have detected an awsy regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=6b9d06ba6f769234530ae67d8353377d58a93fd0

As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

 10%  Images summary linux64-stylo opt stylo     5,564,127.22 -> 6,107,248.21
 10%  Images summary macosx64-stylo opt stylo    5,791,987.24 -> 6,347,801.24


You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=9129

On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the jobs in a pushlog format.

To learn more about the regressing test(s), please see: https://developer.mozilla.org/en-US/docs/Mozilla/Performance/AWSY

Comment 1

[Ionuț Goldan [:igoldan]]

As it can be seen from here:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&tochange=04ecbca85f88a6a32d4cb3e8e4648429590841e5&fromchange=71ee70900245cc248028356a674e354a4f41f497&filter-searchStr=linux

Changeset df44116129d4, which did a servo merge, broke the Linux and OSX builds. It affected the builds for bug 1373814, which landed right after. The servo merge then got immediately fixed by bug 1394729.

The bad thing is that we then noticed the serious regressions above. Which of the two bugs is more related to them?
I'm ni?ing the owners, to tackle this faster.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Rather unlikely that bug 1373814 would have affected here.

Comment 3

[Ionuț Goldan [:igoldan]]

I did the Try pushes for bisection.

Stable Baseline before servo merge: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f28090c221d8b7145b6cd6c952496fef3d751546

Bug 1373814, with servo merge push backed out: https://treeherder.mozilla.org/#/jobs?repo=try&revision=44c64bbc7842e7992ef5abe013f330c29cbe513c

Bug 1394729, which resolved the merge, just for extra comparison: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b86bb28b9c2f4cfcc8880b1705a193be1f245ef

Comment 4

[Andrew McCreight [:mccr8]]

If there was no regression in any other measurement (like total memory), then my guess would be that this is caused by bug 1394729 measuring more stuff. If this is the case, then I'd expect to see a corresponding improvement in heap-unclassified.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

:mccr8- you are correct, it is bug 1394729.

leaving the ni? for :njn as he is the patch author

Comment 6

[Nicholas Nethercote [inactive]]

(In reply to Andrew McCreight [:mccr8] from comment #4)
> If there was no regression in any other measurement (like total memory),
> then my guess would be that this is caused by bug 1394729 measuring more
> stuff. If this is the case, then I'd expect to see a corresponding
> improvement in heap-unclassified.

I agree. Bug 1394729 adding new memory reporting code for Stylo. It should not have affected overall memory consumption in any significant way.

https://treeherder.mozilla.org/perf.html#/alerts?id=9129 shows the two regressing measurements. How can I see all the measurements for this push?

Comment 7

[Nicholas Nethercote [inactive]]

> https://treeherder.mozilla.org/perf.html#/alerts?id=9129 shows the two
> regressing measurements. How can I see all the measurements for this push?

With erahm's help I tried poking around here:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=autoland&newRevision=6b9d06ba6f769234530ae67d8353377d58a93fd0&framework=4&showOnlyImportant=0&selectedTimeRange=172800

But I couldn't see anything that corresponded to the alert linked to in comment 0. I can't evaluate this without all the relevant measurements for the new revision.

igoldan, can you please point me in the right direction?

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

My recommendation is pretty much what you got from the perfherder compare view:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=autoland&originalRevision=71d4452dd938d6522e13e7c97bd0298d04852d77&newProject=autoland&newRevision=6b9d06ba6f769234530ae67d8353377d58a93fd0&framework=4&showOnlyImportant=0

that is a list of all the AWSY runs/measurements between the last known good and the regressed build.  Keep in mind we had to bisect on try server.

Are you looking for data from other frameworks like talos?

Comment 9

[Nicholas Nethercote [inactive]]

Thank you, Joel, that's exactly what I was after.

On macosx64-stylo, images went from 5,638,657.83 to 6,306,687.68, but heap-unclassified went from 83,888,924.16 to 83,190,001.96.

On linux64-stylo, images went from 5,717,105.27 to 6,077,797.26, but heap-unclassified went from 69,333,182.22 to 	68,065,868.32.

This confirms the hypothesis in comment 4: the images changes were balanced or more than balanced out by the heap-unclassified changes, in absolute terms. However, in relative terms, the heap-unclassified changes were much smaller than the images changes, and so didn't show up as alerts.

I think this can be considered solved, with no further work required. Thanks!

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

great, glad it all made sense and this balanced out

Comment 11

[Nicholas Nethercote [inactive]]

BTW, it's not obvious how improving style system memory reporting could affect
the result for images in about:memory. So here's an explanation for the curious...

TL;DR: changing the memory reporting for any kind of DOM+layout+style stuff
can change measurements for vector images in about:memory, because vector
images can contain SVG documents, which contain DOM+layout+style stuff.

Here is where we compute the "source" size of an image:
http://searchfox.org/mozilla-central/rev/44c693914255638d74bcf1ec3b0bcd448dfab2f
d/image/Image.cpp#39

Here is the definition of VectorImage::SizeOfSourceWithComputedFallback():
http://searchfox.org/mozilla-central/rev/44c693914255638d74bcf1ec3b0bcd448dfab2f
d/image/VectorImage.cpp#382-407

Two things to note:

- We call DocAddSizeOfIncludingThis() if we have an SVG document, and that's
  the entry point for all of the DOM+layout+style memory reporting.
  
- We call getTotalSize() at the end to combine all that DOM+layout+style into a
  single number.
  
This single number then gets reported in about:memory under these paths:

- "explicit/images/content/vector/used/*/source"
- "explicit/images/chrome/vector/used/*/source"
- "images/content/vector/used/source"
- "images/chrome/vector/used/source"

SVG images are used quite a bit in Firefox's UI, especially in the parent 
process, e.g.:

- chrome://browser/skin/gear.svg
- chrome://browser/skin/sidebars.svg
- chrome://global/skin/icons/close.svg
- chrome://browser/skin/bookmark-hollow.svg
- chrome://browser/skin/home.svg
- chrome://pocket-shared/skin/pocket.svg
- chrome://browser/skin/bookmark.svg