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)

NEW
Assigned to

Status

()

defect
P2
normal
2 months ago
29 days ago

People

(Reporter: Bebe, Assigned: emilio, NeedInfo)

Tracking

(Blocks 1 bug, Regression, {leave-open, perf, regression})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 obsolete attachment)

Reporter

Description

2 months ago

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

Reporter

Updated

2 months ago
Blocks: 1534654
Component: General → CSS Parsing and Computation
Product: Testing → Core
Regressed by: 1535788
Version: Version 3 → unspecified
Assignee

Updated

2 months ago
Assignee: nobody → emilio
Flags: needinfo?(emilio)
Assignee

Comment 1

2 months ago

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.

Assignee

Updated

2 months ago
Flags: needinfo?(emilio)
Keywords: leave-open

Comment 2

2 months ago
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/b88166b3b314
Measure the styleset memory usage from the document, not the pres shell.
Assignee

Comment 3

2 months ago

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!

Flags: needinfo?(fstrugariu)

Comment 5

2 months ago
Backout by aciure@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/40456af7da1c
Backed out changeset b88166b3b314 for 1401692.html perma failures a=backout

Comment 7

2 months ago
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/913749a193b6
Measure the styleset memory usage from the document, not the pres shell.
Assignee

Updated

2 months ago
Flags: needinfo?(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?

Flags: needinfo?(igoldan)
Reporter

Comment 10

a month ago

Emilio

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

Flags: needinfo?(fstrugariu)

Removing ni?, as Florin provided the answer.

Flags: needinfo?(igoldan)

Ok, I'll work more on it.

Flags: needinfo?(emilio)
Assignee

Updated

a month ago
Depends on: 1544535

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

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.

Attachment #9058367 - Attachment is obsolete: true
Assignee

Updated

a month ago
Depends on: 1544546
Assignee

Updated

a month ago
Depends on: 1544548
Assignee

Updated

a month ago
Priority: -- → P2

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

Flags: needinfo?(igoldan)
Flags: needinfo?(fstrugariu)
Flags: needinfo?(emilio)

(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.

Flags: needinfo?(igoldan)

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

Flags: needinfo?(emilio)
Reporter

Updated

29 days ago
Flags: needinfo?(fstrugariu)
You need to log in before you can comment on or make changes to this bug.