1551520 - 1.53 - 3.93% displaylist_mutate (linux64-shippable-qr, windows10-64-shippable-qr) regression on push 73254a69497b209b372ed209ce9d004080ea8052 (Fri May 10 2019)

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Ionuț Goldan [:igoldan]]

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b86c8998a2b246e0c5f13b32481ace92f1be0d99&tochange=73254a69497b209b372ed209ce9d004080ea8052

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

Comment 1

[Dzmitry Malyshau [:kvark]]

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?

Comment 2

[Ionuț Goldan [:igoldan]]

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

Comment 3

[Dzmitry Malyshau [:kvark]]

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?

Comment 4

[Ionuț Goldan [:igoldan]]

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

Comment 5

[Dzmitry Malyshau [:kvark]]

Here is a pending try on the base version (without the patch) - https://treeherder.mozilla.org/#/jobs?repo=try&revision=80912921211bb9492335b8505eec04ec7ec4f403

Comment 6

[Dzmitry Malyshau [:kvark]]

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?

Comment 7

[Dzmitry Malyshau [:kvark]]

Actually, I found a piece in the original change that could cause this slowdown ("ref_world_inv_transform"). Working on removing it now.

Comment 8

[Dzmitry Malyshau [:kvark]]

Attachment: Use WR relative transform instead of the world inverse

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.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/065da3e6d4a7
Use WR relative transform instead of the world inverse r=gw

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/065da3e6d4a7

Comment 11

[Dzmitry Malyshau [:kvark]]

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

[Julien Cristau [:jcristau]]

Comment on attachment 9067170 [details]
Use WR relative transform instead of the world inverse

WR perf fix, approved for 68.0b5

Comment 13

[Oana Pop-Rus]

https://hg.mozilla.org/releases/mozilla-beta/rev/33e163907440

Comment 14

[Cosmin Sabou [:CosminS]]

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 <[e-mail]>' 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 <[e-mail]>' 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...

Comment 15

[Dzmitry Malyshau [:kvark]]

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

[Cosmin Sabou [:CosminS]]

Yes, we can uplift the modified patch in another push.

Comment 17

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/releases/mozilla-beta/rev/39c14a33468f