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)

defect
Not set
normal

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)

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
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)
Blocks: 1340627
Component: Untriaged → Graphics
Product: Firefox → Core
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.
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
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
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 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?
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)
Attachment #8866955 - Flags: review?(mstange) → review+
Pushed by lsalzman@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/54e6f320ab5d clip drawing of native Cocoa widgets in gfxQuartzNativeDrawing. r=mstange
(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.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: