Closed
Bug 1364007
Opened 8 years ago
Closed 8 years ago
2.1 - 10.63% cart / tresize / tsvg_static / tsvgx (linux64, osx-10-10, windows7-32) regression on push 0b1371055c7f890ed8cc265726747a35c67ead8f (Wed May 10 2017)
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox53 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | --- | fixed |
People
(Reporter: igoldan, Assigned: lsalzman)
References
()
Details
(Keywords: perf, regression, talos-regression)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
Talos has detected a Firefox performance regression from push 0b1371055c7f890ed8cc265726747a35c67ead8f. As author of one of the patches included in that push, we need your help to address this regression.
Regressions:
11% cart summary osx-10-10 opt 34.39 -> 38.05
5% tresize windows7-32 opt 10.43 -> 10.92
4% tsvg_static summary windows7-32 pgo e10s58.46 -> 60.50
3% tsvgx summary linux64 pgo e10s 317.32 -> 325.55
2% tsvgx summary linux64 pgo 344.42 -> 351.65
Improvements:
16% tcanvasmark summary osx-10-10 opt 6,390.21 -> 7,399.08
4% tsvgx summary windows7-32 pgo 497.53 -> 477.99
3% tsvgx summary windows7-32 pgo e10s 461.03 -> 446.90
3% tsvgx summary linux64 opt 489.15 -> 474.31
2% tsvgx summary linux64 opt e10s 442.98 -> 436.03
You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=6498
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 Talos jobs in a pushlog format.
To learn more about the regressing test(s), please see: https://wiki.mozilla.org/Buildbot/Talos/Tests
For information on reproducing and debugging the regression, either on try or locally, see: https://wiki.mozilla.org/Buildbot/Talos/Running
*** Please let us know your plans within 3 business days, or the offending patch(es) will be backed out! ***
Our wiki page outlines the common responses and expectations: https://wiki.mozilla.org/Buildbot/Talos/RegressionBugsHandling
Reporter | ||
Comment 1•8 years ago
|
||
There is a discrepancy between pgo and opt builds on linux64 platform. One says it's a regression, the other one that is an improvement.
You can ignore it. Further investigations may relate this discrepancy to other issues.
The other stats hold.
:lsalzman I know this got backed out, then backed in with an extra fix. Could you please confirm these regressions, and state whether you have an improvement or whether we should accept this?
Flags: needinfo?(lsalzman)
Reporter | ||
Comment 2•8 years ago
|
||
I'm presenting the stats in chronological order. What you see in the 1st comment, are the initial stats, which are a little out of date.
Reporter | ||
Comment 3•8 years ago
|
||
These are the stats collected after the backout was done:
== Change summary for alert #6510 (as of May 10 2017 17:01 UTC) ==
Regressions:
15% tcanvasmark summary osx-10-10 opt 7,433.41 -> 6,332.12
3% tsvgx summary linux64 opt 474.67 -> 489.77
Improvements:
10% cart summary osx-10-10 opt 38.37 -> 34.41
6% tart summary osx-10-10 opt 11.16 -> 10.53
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=6510
Reporter | ||
Comment 4•8 years ago
|
||
And these are the results collected after the second push:
== Change summary for alert #6515 (as of May 10 2017 20:21 UTC) ==
Regressions:
12% cart summary osx-10-10 opt 34.47 -> 38.57
Improvements:
16% tcanvasmark summary osx-10-10 opt 6,357.54 -> 7,349.08
4% tsvgx summary linux64 opt 490.04 -> 471.58
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=6515
Assignee | ||
Comment 5•8 years ago
|
||
We apparently were not clipping the drawing area for native Cocoa widgets to the dirty rect.
So when we end up asking Skia for a borrowed CG context, there may complex clips (such as multiple rects) lying around that force it to create a layer for the CG context to draw into. This layer will encompass a much larger portion of the window (or the entire window) than the dirty rect requires.
This thus clips to the dirty rect in the hope that either the clip simplifies to a single rect or the area of the complex clip is substantially reduced, thus reducing the size of any allocated layers.
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
Flags: needinfo?(lsalzman)
Attachment #8866948 -
Flags: review?(mstange)
Comment 6•8 years ago
|
||
Comment on attachment 8866948 [details] [diff] [review]
clip drawing of native Cocoa widgets in gfxQuartzNativeDrawing
Review of attachment 8866948 [details] [diff] [review]:
-----------------------------------------------------------------
This will result in unbalanced Push/PopClip calls if allocating mTempDrawTarget fails. Can you push the clip in all cases so that it's more obvious to see that they're balanced?
Assignee | ||
Comment 7•8 years ago
|
||
Cleaned up the failure handling bits to look more sane.
Attachment #8866948 -
Attachment is obsolete: true
Attachment #8866948 -
Flags: review?(mstange)
Attachment #8866955 -
Flags: review?(mstange)
Updated•8 years ago
|
Attachment #8866955 -
Flags: review?(mstange) → review+
Updated•8 years ago
|
status-firefox53:
--- → unaffected
status-firefox54:
--- → unaffected
status-firefox55:
--- → affected
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/54e6f320ab5d
clip drawing of native Cocoa widgets in gfxQuartzNativeDrawing. r=mstange
Assignee | ||
Comment 9•8 years ago
|
||
(In reply to Pulsebot from comment #8)
> Pushed by lsalzman@mozilla.com:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/54e6f320ab5d
> clip drawing of native Cocoa widgets in gfxQuartzNativeDrawing. r=mstange
This patch fixes the cart talos regression on Mac, tested locally and on try.
The other incidental small Windows regressions just look like PGO optimizer differences due to significant code churn in Skia since we last updated. We will just accept those at least since they're within acceptably small ranges/margin of error.
Comment 10•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•8 years ago
|
status-firefox-esr52:
--- → unaffected
You need to log in
before you can comment on or make changes to this bug.
Description
•