1542263 - 2.61 - 5.65% Heap Unclassified / Images (linux64-shippable, linux64-shippable-qr, windows10-64-shippable, windows7-32-shippable) regression on push 60c20a0f320cc769a8da229cdf6edfbbf38d3ed3 (Wed Apr 3 2019)

Status: RESOLVED WORKSFORME

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

Description

[Florin Strugariu [:Bebe]]

We have detected an awsy regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=60c20a0f320cc769a8da229cdf6edfbbf38d3ed3

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

Regressions:

6% Heap Unclassified windows7-32-shippable opt 35,823,294.67 -> 37,848,650.37
5% Images linux64-shippable-qr opt stylo tp6 7,758,852.51 -> 8,117,218.04
5% Heap Unclassified windows10-64-shippable opt stylo tp6 62,115,137.20 -> 64,980,197.55
3% Heap Unclassified linux64-shippable opt stylo tp6 96,206,383.49 -> 98,716,540.38

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

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://wiki.mozilla.org/AWSY/Tests

Comment 1

[Emilio Cobos Álvarez (:emilio)]

I'll land the trivial fix for the unclassified memory now. If there's a memory regression from that I can look into it as well.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b88166b3b314
Measure the styleset memory usage from the document, not the pres shell.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Florin, mind letting me know if after that patch there's still a memory usage regression compared to before bug 1535788 landed?

If so, I can look into that as well. Otherwise, that should take care of it.

Thanks!

Comment 4

[Daniel Varga [:dvarga]]

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

Comment 5

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/40456af7da1c
Backed out changeset b88166b3b314 for 1401692.html perma failures a=backout

Comment 6

[Andrei Ciure[:aciure]]

push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=windows%2C10%2Cx64%2Cdebug%2Creftests%2Cwith%2Ce10s%2Ctest-windows10-64%2Fdebug-crashtest-e10s%2Cr-e10s%28c%29&selectedJob=238823012&revision=b88166b3b314cda6ea94a0444d116c6b0321b6bd

failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=471b2063c817d3eb1dddf334963200e293953cdb&searchStr=windows%2C10%2Cx64%2Cdebug%2Creftests%2Cwith%2Ce10s%2Ctest-windows10-64%2Fdebug-crashtest-e10s%2Cr-e10s%28c%29&tochange=921fabb1962728d357018a3863762e8d1d70e354&selectedJob=238869000

backout: https://hg.mozilla.org/mozilla-central/rev/40456af7da1c

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/913749a193b6
Measure the styleset memory usage from the document, not the pres shell.

Comment 8

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/913749a193b6

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Maybe you can also answer the question in comment 3 now that the fix for the unclassified memory has been in for 3 days?

Comment 10

[Florin Strugariu [:Bebe]]

Emilio

Looks like that https://hg.mozilla.org/integration/autoland/rev/913749a193b6 only partially fixed the regressions:

• 6% Heap Unclassified windows7-32-shippable opt 35,823,294.67 -> 37,848,650.37
  • about 50% fixed: https://treeherder.mozilla.org/perf.html#/graphs?series=autoland,1959377,1,4&highlightedRevisions=913749a193b6
• 5% Images linux64-shippable-qr opt stylo tp6 7,758,852.51 -> 8,117,218.04
  • Not fixed: https://treeherder.mozilla.org/perf.html#/graphs?series=autoland,1959042,1,4&highlightedRevisions=913749a193b6
• 5% Heap Unclassified windows10-64-shippable opt stylo tp6 62,115,137.20 -> 64,980,197.55
  • about 50% fixed: https://treeherder.mozilla.org/perf.html#/graphs?series=autoland,1958603,1,4&highlightedRevisions=913749a193b6
• 3% Heap Unclassified linux64-shippable opt stylo tp6 96,206,383.49 -> 98,716,540.38
  • about 50% fixed: https://treeherder.mozilla.org/perf.html#/graphs?series=autoland,1958972,1,4&highlightedRevisions=913749a193b6

Comment 11

[Ionuț Goldan [:igoldan]]

Removing ni?, as Florin provided the answer.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Ok, I'll work more on it.

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1542263 - Don't keep two list of stylesheets in ServoStyleSet. r=#style

Just one set of stylesheets is enough. While at it, unify SheetType and Origin.

Comment 14

[Phabricator Automation]

Comment on attachment 9058367 [details]
Bug 1542263 - Don't keep two list of stylesheets in ServoStyleSet. r=#style

Revision D27564 was moved to bug 1544535. Setting attachment 9058367 [details] to obsolete.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Does it look better now that the dependent bugs have landed? Or do I need to go dig more?

Comment 16

[Ionuț Goldan [:igoldan]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #15)

Does it look better now that the dependent bugs have landed? Or do I need to go dig more?

I noticed another round of improvements for the Heap regressions. I'm happy considering them fixed.
However, the Images regression remained unchanged.

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Mkay, let me try to dig what can I do here to improve a bit more...

Comment 18

[Emilio Cobos Álvarez (:emilio)]

Not actively looking into this.

Comment 19

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

The leave-open keyword is there and there is no activity for 6 months.
:emilio, maybe it's time to close this bug?

Comment 20

[Emilio Cobos Álvarez (:emilio)]

WFM per comment 16. we should look into the images regressions, but there are other bugs open for that like bug 1633410.