1.53 - 3.93% displaylist_mutate (linux64-shippable-qr, windows10-64-shippable-qr) regression on push 73254a69497b209b372ed209ce9d004080ea8052 (Fri May 10 2019)
Categories
(Core :: Graphics: WebRender, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox67 | --- | unaffected |
firefox68 | --- | fixed |
firefox69 | --- | fixed |
People
(Reporter: igoldan, Assigned: kvark)
References
(Regression)
Details
(4 keywords)
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
Talos has detected a Firefox performance regression from push:
As author of one of the patches included in that push, we need your help to address this regression.
Regressions:
4% displaylist_mutate windows10-64-shippable-qr opt e10s stylo 3,637.48 -> 3,780.56
2% displaylist_mutate linux64-shippable-qr opt e10s stylo 3,813.24 -> 3,871.62
You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=20908
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/Performance_sheriffing/Talos/Tests
For information on reproducing and debugging the regression, either on try or locally, see: https://wiki.mozilla.org/Performance_sheriffing/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/Performance_sheriffing/Talos/RegressionBugsHandling
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Comment 1•5 years ago
|
||
I think the only change that would affect dl_mutate timing this much is in update_tree
. We used to just write down the transformations of the nodes into TransformPalette
as we process the tree. Now we are having a separate pass called build_transform_palette
that calls get_world_transform
for each node, so it's technically O(N * TreeDepth) complexity.
I'd like this to be addressed in the follow up work that was planned anyway. We'd cache the world transformations of CoordinateSpace
(instead of SpatialNode
), so that any get_world_transform
lookup is O(1) for both scrolled and non-scrolled versions.
This is on my list to address once I'm back from a meeting in MV. That is to say, changes are not trivial, so putting them in for 68 release sounds a bit risky. Perhaps, we can roll back the change for 68 only and keep it in central?
Reporter | ||
Comment 2•5 years ago
|
||
(In reply to Dzmitry Malyshau [:kvark] from comment #1)
Perhaps, we can roll back the change for 68 only and keep it in central?
I'd prefer that. Thanks!
Assignee | ||
Comment 3•5 years ago
|
||
I took a stab at fixing the performance regression as opposed to backing out the original (large) change, and I ended up with a very tiny fix that just landed in https://phabricator.services.mozilla.com/D32195 . The Talos results so far confirm this in https://treeherder.mozilla.org/#/jobs?repo=try&collapsedPushes=480100&selectedJob=247829360&revision=966241befa0bdc998d07a13141f359439b5a2403 , I believe.
:igoldan do I understand correctly that now we need this fix backported to beta?
Reporter | ||
Comment 4•5 years ago
|
||
(In reply to Dzmitry Malyshau [:kvark] from comment #3)
I took a stab at fixing the performance regression as opposed to backing out the original (large) change, and I ended up with a very tiny fix that just landed in https://phabricator.services.mozilla.com/D32195 . The Talos results so far confirm this in https://treeherder.mozilla.org/#/jobs?repo=try&collapsedPushes=480100&selectedJob=247829360&revision=966241befa0bdc998d07a13141f359439b5a2403 , I believe.
Could you also push the baseline on Try (the original patch you applied your fix on) & then share that Treeherder link, so I can properly confirm the fix?
:igoldan do I understand correctly that now we need this fix backported to beta?
I don't demand that, though it would be nice. I believe this depends whether there are any risks on uplifting this to beta.
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Comment 5•5 years ago
|
||
Here is a pending try on the base version (without the patch) - https://treeherder.mozilla.org/#/jobs?repo=try&revision=80912921211bb9492335b8505eec04ec7ec4f403
Assignee | ||
Comment 6•5 years ago
|
||
Looking at these tries now and trying to make sense of them. The "base" try reports the score of 3495.1 on windows, while the "fixed" try reports "3835.58". This is unexpected, since the base try includes the very change that regressed the score from 3,637.48 to 3,780.56 according to the original report in this bug...
:igoldan
May I ask for Gecko Profiled runs of displaylist_mutate on windows10-shippable-qr with/wo the patch?
Assignee | ||
Comment 7•5 years ago
|
||
Actually, I found a piece in the original change that could cause this slowdown ("ref_world_inv_transform"). Working on removing it now.
Assignee | ||
Comment 8•5 years ago
|
||
This is a follow-up to https://phabricator.services.mozilla.com/D30600
Previously, I changed changed the space mapper logic to use the world transformations.
This was seemingly needed because we requrested the relation between primitives and
their clip nodes, which could be in unrelated spatial sub-trees.
However, I believe the change was a mistake, since for clips we should not even try
to get the relative mapping, and clipping is done in world space for these cases.
This change reverts that logic back. Fingers crossed for the try to not show any
asserts firing up inside get_relative_transform.
Updated•5 years ago
|
Comment 10•5 years ago
|
||
bugherder |
Assignee | ||
Comment 11•5 years ago
|
||
Comment on attachment 9067170 [details]
Use WR relative transform instead of the world inverse
Beta/Release Uplift Approval Request
- User impact if declined: 3% slower display list mutation
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Small change, the behavior is matching what we used to do for a while.
- String changes made/needed:
Comment 12•5 years ago
|
||
Comment on attachment 9067170 [details]
Use WR relative transform instead of the world inverse
WR perf fix, approved for 68.0b5
Updated•5 years ago
|
Comment 13•5 years ago
|
||
bugherder uplift |
Comment 14•5 years ago
|
||
Backed out for causing build bustages.
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&resultStatus=testfailed%2Cbusted&revision=80259caee8e3b023f2c2a638c59f20334646679d
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=248212924&repo=mozilla-beta&lineNumber=16171
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=248212973&repo=mozilla-beta&lineNumber=395
Backout link: https://hg.mozilla.org/releases/mozilla-beta/rev/e5a7fef8ed318119d84241732cf4c92c859f0e17
[task 2019-05-24T15:02:12.636Z] 15:02:12 WARNING - [style 0.0.1] Warning: can't set binop_separator = Back
, unstable features are only available in nightly channel.
[task 2019-05-24T15:02:12.636Z] 15:02:12 WARNING - [style 0.0.1] Warning: can't set match_block_trailing_comma = true
, unstable features are only available in nightly channel.
[task 2019-05-24T15:02:12.640Z] 15:02:12 INFO - Running CARGO_PKG_VERSION_MINOR=60 CARGO_PKG_HOMEPAGE= LD_LIBRARY_PATH='/builds/worker/workspace/build/src/obj-firefox/release/deps:/builds/worker/workspace/build/src/rustc/lib:/builds/worker/workspace/build/src/gcc/lib64:/builds/worker/workspace/build/src/gcc/lib32:/builds/worker/workspace/build/src/gcc/lib' CARGO_PKG_VERSION_MAJOR=0 CARGO_MANIFEST_DIR=/builds/worker/workspace/build/src/gfx/wr/webrender CARGO_PKG_NAME=webrender CARGO_PKG_AUTHORS='Glenn Watson <gw@intuitionlibrary.com>' CARGO_PKG_VERSION_PATCH=0 CARGO_PKG_VERSION=0.60.0 CARGO=/builds/worker/workspace/build/src/rustc/bin/cargo CARGO_PKG_VERSION_PRE= CARGO_PKG_DESCRIPTION='A GPU accelerated 2D renderer for web content' CARGO_PKG_REPOSITORY='https://github.com/servo/webrender' OUT_DIR=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/build/webrender-3c12e24a58783078/out /builds/worker/workspace/build/src/rustc/bin/rustc --edition=2018 --crate-name webrender gfx/wr/webrender/src/lib.rs --color never --crate-type lib --emit=dep-info,link -C opt-level=2 -C panic=abort -C codegen-units=1 --cfg 'feature="api"' --cfg 'feature="capture"' --cfg 'feature="ron"' --cfg 'feature="serde"' --cfg 'feature="serialize_program"' --cfg 'feature="smallvec"' --cfg 'feature="webrender_build"' -C metadata=5630a82651988b55 -C extra-filename=-5630a82651988b55 --out-dir /builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -C linker=/builds/worker/workspace/build/src/build/cargo-linker -L dependency=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps -L dependency=/builds/worker/workspace/build/src/obj-firefox/release/deps --extern bincode=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libbincode-98fe8132a7d358f5.rlib --extern bitflags=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libbitflags-f7bbf80aef0d6ac7.rlib --extern byteorder=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libbyteorder-8e6c8d9200f1e34d.rlib --extern cfg_if=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libcfg_if-2bfa6c093b4b19e8.rlib --extern cstr=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libcstr-d908aa290ed6155a.rlib --extern euclid=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libeuclid-1f284c37ba16527b.rlib --extern freetype=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libfreetype-42c77db9374a29c2.rlib --extern fxhash=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libfxhash-851742fa41f33469.rlib --extern gleam=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libgleam-cbaa1af42be6ed25.rlib --extern lazy_static=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/liblazy_static-846765a24a38576b.rlib --extern libc=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/liblibc-56a0813a2d56ca48.rlib --extern log=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/liblog-9c02cd8e9d843574.rlib --extern malloc_size_of_derive=/builds/worker/workspace/build/src/obj-firefox/release/deps/libmalloc_size_of_derive-d301771af436fbd8.so --extern num_traits=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libnum_traits-df755dc730fd6342.rlib --extern plane_split=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libplane_split-4db8bbece07c10ac.rlib --extern rayon=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/librayon-0bcfa7a240ddc0d6.rlib --extern ron=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libron-e3e019b500ba6c2e.rlib --extern serde=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libserde-57860597180c56b1.rlib --extern sha2=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libsha2-2bb75c855a59bf0f.rlib --extern smallvec=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libsmallvec-69431fb304b8f4ac.rlib --extern svg_fmt=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libsvg_fmt-cefe524622703074.rlib --extern thread_profiler=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libthread_profiler-f19d4c13f1f3f33c.rlib --extern time=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libtime-555513f5486d5c25.rlib --extern api=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libwebrender_api-fc9b38ccf0aa586e.rlib --extern webrender_build=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libwebrender_build-aa28714b57fa168f.rlib --extern malloc_size_of=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libwr_malloc_size_of-1ffb3f8f3a9fe3ba.rlib -C opt-level=2 -C debuginfo=2 -Dwarnings
[task 2019-05-24T15:02:12.640Z] 15:02:12 ERROR - error[E0599]: no method named with_destination
found for type clip_scroll_tree::CoordinateSpaceMapping<api::units::LayoutPixel, api::units::LayoutPixel>
in the current scope
[task 2019-05-24T15:02:12.640Z] 15:02:12 INFO - --> gfx/wr/webrender/src/clip_scroll_tree.rs:320:14
[task 2019-05-24T15:02:12.640Z] 15:02:12 INFO - |
[task 2019-05-24T15:02:12.640Z] 15:02:12 INFO - 144 | pub enum CoordinateSpaceMapping<Src, Dst> {
[task 2019-05-24T15:02:12.640Z] 15:02:12 INFO - | ----------------------------------------- method with_destination
not found for this
[task 2019-05-24T15:02:12.640Z] 15:02:12 INFO - ...
[task 2019-05-24T15:02:12.640Z] 15:02:12 INFO - 320 | .with_destination::<WorldPixel>()
[task 2019-05-24T15:02:12.640Z] 15:02:12 INFO - | ^^^^^^^^^^^^^^^^
[task 2019-05-24T15:02:12.640Z] 15:02:12 ERROR - error: aborting due to previous error
[task 2019-05-24T15:02:12.640Z] 15:02:12 INFO - For more information about this error, try rustc --explain E0599
.
[task 2019-05-24T15:02:12.640Z] 15:02:12 ERROR - error: Could not compile webrender
.
[task 2019-05-24T15:02:12.640Z] 15:02:12 INFO - Caused by:
[task 2019-05-24T15:02:12.644Z] 15:02:12 INFO - process didn't exit successfully: CARGO_PKG_VERSION_MINOR=60 CARGO_PKG_HOMEPAGE= LD_LIBRARY_PATH='/builds/worker/workspace/build/src/obj-firefox/release/deps:/builds/worker/workspace/build/src/rustc/lib:/builds/worker/workspace/build/src/gcc/lib64:/builds/worker/workspace/build/src/gcc/lib32:/builds/worker/workspace/build/src/gcc/lib' CARGO_PKG_VERSION_MAJOR=0 CARGO_MANIFEST_DIR=/builds/worker/workspace/build/src/gfx/wr/webrender CARGO_PKG_NAME=webrender CARGO_PKG_AUTHORS='Glenn Watson <gw@intuitionlibrary.com>' CARGO_PKG_VERSION_PATCH=0 CARGO_PKG_VERSION=0.60.0 CARGO=/builds/worker/workspace/build/src/rustc/bin/cargo CARGO_PKG_VERSION_PRE= CARGO_PKG_DESCRIPTION='A GPU accelerated 2D renderer for web content' CARGO_PKG_REPOSITORY='https://github.com/servo/webrender' OUT_DIR=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/build/webrender-3c12e24a58783078/out /builds/worker/workspace/build/src/rustc/bin/rustc --edition=2018 --crate-name webrender gfx/wr/webrender/src/lib.rs --color never --crate-type lib --emit=dep-info,link -C opt-level=2 -C panic=abort -C codegen-units=1 --cfg 'feature="api"' --cfg 'feature="capture"' --cfg 'feature="ron"' --cfg 'feature="serde"' --cfg 'feature="serialize_program"' --cfg 'feature="smallvec"' --cfg 'feature="webrender_build"' -C metadata=5630a82651988b55 -C extra-filename=-5630a82651988b55 --out-dir /builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -C linker=/builds/worker/workspace/build/src/build/cargo-linker -L dependency=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps -L dependency=/builds/worker/workspace/build/src/obj-firefox/release/deps --extern bincode=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libbincode-98fe8132a7d358f5.rlib --extern bitflags=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libbitflags-f7bbf80aef0d6ac7.rlib --extern byteorder=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libbyteorder-8e6c8d9200f1e34d.rlib --extern cfg_if=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libcfg_if-2bfa6c093b4b19e8.rlib --extern cstr=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libcstr-d908aa290ed6155a.rlib --extern euclid=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libeuclid-1f284c37ba16527b.rlib --extern freetype=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libfreetype-42c77db9374a29c2.rlib --extern fxhash=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libfxhash-851742fa41f33469.rlib --extern gleam=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libgleam-cbaa1af42be6ed25.rlib --extern lazy_static=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/liblazy_static-846765a24a38576b.rlib --extern libc=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/liblibc-56a0813a2d56ca48.rlib --extern log=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/liblog-9c02cd8e9d843574.rlib --extern malloc_size_of_derive=/builds/worker/workspace/build/src/obj-firefox/release/deps/libmalloc_size_of_derive-d301771af436fbd8.so --extern num_traits=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libnum_traits-df755dc730fd6342.rlib --extern plane_split=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libplane_split-4db8bbece07c10ac.rlib --extern rayon=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/librayon-0bcfa7a240ddc0d6.rlib --extern ron=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libron-e3e019b500ba6c2e.rlib --extern serde=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libserde-57860597180c56b1.rlib --extern sha2=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libsha2-2bb75c855a59bf0f.rlib --extern smallvec=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libsmallvec-69431fb304b8f4ac.rlib --extern svg_fmt=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libsvg_fmt-cefe524622703074.rlib --extern thread_profiler=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libthread_profiler-f19d4c13f1f3f33c.rlib --extern time=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libtime-555513f5486d5c25.rlib --extern api=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libwebrender_api-fc9b38ccf0aa586e.rlib --extern webrender_build=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libwebrender_build-aa28714b57fa168f.rlib --extern malloc_size_of=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/libwr_malloc_size_of-1ffb3f8f3a9fe3ba.rlib -C opt-level=2 -C debuginfo=2 -Dwarnings
(exit code: 1)
[task 2019-05-24T15:02:12.644Z] 15:02:12 INFO - warning: build failed, waiting for other jobs to finish...
Assignee | ||
Comment 15•5 years ago
|
||
There was one small bit that depended on an earlier change. Here is the try push of the same change rebased (properly) on mozilla-beta:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d0ff93147631e5f1a42b839a9a05ce6e12ad7a62
I verified locally that it builds and passes Wrench tests. Would you be able to uplift this modified patch (this links to the commit in the try push) to beta?
Comment 16•5 years ago
|
||
Yes, we can uplift the modified patch in another push.
Comment 17•5 years ago
|
||
bugherder uplift |
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Updated•5 years ago
|
Updated•3 years ago
|
Description
•