Closed
Bug 1436723
Opened 7 years ago
Closed 7 years ago
Enabling "Circle differences" in the reftest analyzer has gotten a lot slower
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla60
| Tracking | Status | |
|---|---|---|
| firefox-esr52 | --- | unaffected |
| firefox58 | --- | unaffected |
| firefox59 | + | wontfix |
| firefox60 | --- | fixed |
People
(Reporter: kats, Assigned: rhunt)
References
()
Details
(Keywords: perf, regression)
Attachments
(2 files)
STR:
Go to a reftest analyzer link, e.g. https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/ang7VNsPSA2W9TE1fVgu3w/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1
Click on the failing reftest on the left
Press "d" on the keyboard to turn on the show differences view.
Expected: The differences view appears near-instantaneously
Actual: The differences view takes a noticeable second or two to appear.
In 58.0.1 on OS X, I get the "expected" behaviour. In 59.0b5 I get the "actual" behaviour. So this regressed somewhere between those two. I'll run mozregression to bisect.
|
Reporter | |
Comment 1•7 years ago
|
||
Mozregression gave me this:
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f6bd79484f2311acb4ebd7c1ee2071d9d2e29710&tochange=b7d96ef3b33c77e7a8fe367d4b6c463a07aac63a
Ryan, thoughts?
Blocks: 1425056
status-firefox58:
--- → unaffected
status-firefox59:
--- → affected
status-firefox60:
--- → affected
Flags: needinfo?(rhunt)
Keywords: perf,
regression
|
|
Reporter | |
Updated•7 years ago
|
tracking-firefox59:
--- → ?
|
Assignee | |
Comment 2•7 years ago
|
||
This is a result of the FilterNodeSoftware changes. Probably from removing caching. I'm not sure why a filter node is even being used for this page. I'm investigating further.
Assignee: nobody → rhunt
Flags: needinfo?(rhunt)
Tracking for 59 just to keep an eye on possible uplift.
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Comment 6•7 years ago
|
||
|
|
Assignee | |
Comment 7•7 years ago
|
||
With these patches the performance of the circle differences button is greatly increased, and matches chrome.
|
||
Comment 8•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8950395 [details]
Restore FilterNodeSoftware intermediate surface caching (bug 1436723, )
https://reviewboard.mozilla.org/r/219618/#review225396
Attachment #8950395 -
Flags: review?(mstange) → review+
|
|
||
Comment 9•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8950396 [details]
Make FilterNodeSoftware intermediate surface caching thread safe (bug 1436723, )
https://reviewboard.mozilla.org/r/219620/#review225400
Attachment #8950396 -
Flags: review?(mstange) → review+
|
||
Comment 10•7 years ago
|
||
Pushed by rhunt@eqrion.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd4ee2af3673
Restore FilterNodeSoftware intermediate surface caching (bug 1436723, r=mstange)
https://hg.mozilla.org/integration/mozilla-inbound/rev/be009d297bc6
Make FilterNodeSoftware intermediate surface caching thread safe (bug 1436723, r=mstange)
|
||
Comment 11•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/bd4ee2af3673
https://hg.mozilla.org/mozilla-central/rev/be009d297bc6
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
|
||
Comment 12•7 years ago
|
||
Given where we are in the cycle, I'm inclined to let this ride the trains in 60 at this point. However, I could be persuaded to consider this for Beta still if people feel strongly about the severity of the issue on the risk of the patch.
|
|
Assignee | |
Comment 13•7 years ago
|
||
I would agree, the behavior here is thread safety related so the patch risk here is nontrivial.
Flags: needinfo?(rhunt)
You need to log in
before you can comment on or make changes to this bug.
Description
•