Closed Bug 1558598 Opened 1 year ago Closed 1 year ago

Add option to explicitly enable WR in automation and via mach for all test harnesses

Categories

(Core :: Graphics: WebRender, task, P3)

Other Branch
task

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(18 files, 3 obsolete files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

We currently have the --enable-webrender option supported in all mozharness tests that run in automation, to enable WR. And for AWSY we also have a --disable-webrender option to forcibly disable it, because we discovered it was unintentionally getting enabled because of the hardware those tests were running on. And the corresponding mach commands for the tests don't always provide a way to enable/disable webrender, so when running tests locally you either have to explicitly set the MOZ_WEBRENDER env var or the gfx.webrender.all pref to what you want.

It would be better if we just had everything take an optional --enable-webrender option that enabled WR, and not setting that option would forcibly disable WR, which would provide a more consistent experience regardless of what the test is running on.

Downscoping this to just the automation harnesses. Adding --enable-webrender to all the mach commands that don't have it yet (i.e. all except awsy-test) looks like a bunch of work and for dubious benefit. It might also break developer workflows so I'll punt that to later.

Summary: Clean up options for enabling/disabling WR in automation and in local testing → Clean up options for enabling/disabling WR in automation

This drops the --disable-webrender option (which force-disables WR)
and treats the lack of an --enable-webrender as if --disable-webrender
was provided.

Ensure we force-disable webrender unless it is explicitly enabled
via the --enable-webrender flag. Also add missing env variables for
the telemetry_client.py case which appears to be a copy/paste error
that was not caught because we never run that test with WR enabled.

Depends on D34622

This is not used yet but will be eventually so I'm just going to
add it now.

Depends on D34623

Bugbug thinks this bug is a task, but please change it back in case of error.

Type: defect → task
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f24e407e9a6d
Remove --disable-webrender options from a couple of places. r=ahal
https://hg.mozilla.org/integration/autoland/rev/d2f0c18e82de
Force-disable webrender unless it is enabled. r=ahal
https://hg.mozilla.org/integration/autoland/rev/d9c42c5f4850
Add the --enable-webrender option to android HW unittests. r=ahal

Oh hmm I guess we're now trying to pass --setenv MOZ_WEBRENDER=0 to a bunch of suites that don't actually support it. I'll see what I can do.

Flags: needinfo?(kats)

Good to know, thanks.

Flags: needinfo?(kats)

I spent a bit of time poking around and this part of mozharness seems exceptionally opaque. AFAICT when we do the setenv here it actually gets passed to a python command that runs a test-specific script here. Some of those scripts (I guess) inherit the TryToolsMixin which takes --setenv arguments here and somehow(!?) must be turning it into extras that get passed here when launching the application. I don't really understand how that happens. Anyway, the scripts that don't inherit TryToolsMixin are presumably the ones that barf on the --setenv arguments.

Using --setenv at all and going through TryToolsMixin seems wrong; we should probably add a more direct mechanism to pass env vars for the remote process. But that seems like more work than I want to deal with right now, for something that was supposed to be an easy cleanup. I'm just going to close this bug instead.

Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → INCOMPLETE

Looks like we'll need some solution to this in the near future, so that we have a way to disable WR on various suites when we enable it by default on Pixel 2 hardware in bug 1560335. Repurposing this bug to that effect since if we have that then the rest of the patches here can also land.

Blocks: 1560335
Status: RESOLVED → REOPENED
Resolution: INCOMPLETE → ---
Summary: Clean up options for enabling/disabling WR in automation → Add options for disabling WR in automation

I have updated patches here that I think should work, at least enough to unblock bug 1560335 and do all that I set out to do in this bug. Patch queue is currently at https://github.com/staktrace/gecko/commits/disablewr but try is closed so I can't do a try push to test it out at the moment.

Actually this will need updating too: https://searchfox.org/mozilla-central/rev/da14c413ef663eb1ba246799e94a240f81c42488/testing/raptor/raptor/raptor.py#945

I think the raptor webrender option was copy-pasted and won't work for Android at the moment.

There was a whole bunch more stuff that needed upating. Pretty much everywhere the env var was being set directly in testing/mozharness/scripts wouldn't work on Android, and I have to propagate those options down into the individual test harnesses. Working through all that now.

Hi Kats,

Are all the android builds from this try required?
https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&resultStatus=success%2Cpending%2Crunning%2Ctestfailed%2Cbusted%2Cexception&revision=cd0c14a5e592717aad85b033cd1d2c87ed305dd0&searchStr=android%2Craptor

We run raptor android builds on real devices and we have a limited number available and this generates long queues for builds from other branches.

I will stop the current builds. If they are required please re-trigger them.

Flags: needinfo?(kats)

Thanks. Not all of them were required but the fuzzy interface makes it annoying to just select ones that I actually need.

Flags: needinfo?(kats)

Ok, I think I have it working now. I tested all of the harnesses listed below locally via mach commands on both desktop linux and geckoview, and additional checked that manifest parsing was picking up the 'webrender' mozinfo property correctly for mochitests and wpt.

junit
gtest
mochitest
reftest
wpt
raptor (this just hung when running via mach so i wasn't able to locally test)
talos (wouldn't run on android locally)
cppunit
xpcshell (wouldn't run on geckoview locally)
marionette (wouldn't run on android locally)
telemetry-tests-client (ditto as marionette, since it's based on marionette)
awsy (ditto)
firefox-ui (ditto)

jittests doesn't seem to have a mach command, but it doesn't matter as much because WR is not relevant to the JIT. I also did try pushes to verify stuff was good in automation.

It's a ~20-patch patchset to update all the harnesses. I'm going to clear out the old patches and attach the new ones.

Summary: Add options for disabling WR in automation → Add option to explicitly enable WR in automation and via mach for all test harnesses
Attachment #9071435 - Attachment is obsolete: true
Attachment #9071436 - Attachment is obsolete: true
Attachment #9071437 - Attachment is obsolete: true

This covers the local and remote gtests, as well as the mach command.

Depends on D35850

This code covers both remote mochtiests and local mochitests.

Depends on D35852

Also adds it to the mach command, which is a little weird, because the
mach command doesn't expose the option but does parse it via the cpp unit
argument parser. So I just exposed it on the mach command and after that
it Just Works for mach.

Depends on D35858

AWSY is built on marionette, so it inherits the option by default, we mostly
just need to propagate it properly. This also drops the --disable-webrender
option as it is now implied if --enable-webrender is not provided.

Depends on D35862

WebRender being on or off doesn't affect the JIT code, so the option is
not really useful in this harness. However, it is a downstream harness that
we'll be passing this flag to so for consistency we have it accept the
flag and silently eat it.

Depends on D35864

Now that all the downstream test harnesses take the --enable-webrender
option and propagate it correctly, the desktop_unittest.py wrapper can
just pass it along to those harnesses.

Depends on D35865

Instead of using --setenv to control WebRender being on or off, just propagate
the --enable-webrender flag to the downstream "remote" test harness. This
is more reliable than passing --setenv because not all harnesses support
the setenv flag, and it's easier to explicitly add support for --enable-webrender
to the harnesses that need it (i.e. the ones we want to run with WR enabled in
automation).

This also drops the --disable-webrender flag and asserts that lack of
enable-webrender implies WR will be disabled.

Depends on D35866

This is not used yet but will be eventually so I'm just going to
add it now.

As of this patch, all the tests that we currently run on android HW do
accept the --enable-webrender flag and explicitly disable WR if it is
not provided.

Depends on D35867

I don't know if this covers all the things that use mozinfo (probably not)
but it covers all the suites that use mozinfo and have webrender conditions
in the test manifests (i.e. mochitest and wpt variants).

Depends on D35868

Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f1af45cdc798
Force WebRender on or off in the junit harness. r=ahal
https://hg.mozilla.org/integration/autoland/rev/02cb9a216840
Force WebRender on or off in the gtest harness. r=ahal
https://hg.mozilla.org/integration/autoland/rev/499da21448c4
Force WebRender on or off in the mochitest harness. r=ahal
https://hg.mozilla.org/integration/autoland/rev/b06edfe4ff69
Force WebRender on or off in the reftest harness. r=ahal
https://hg.mozilla.org/integration/autoland/rev/5c284c7659c3
Force WebRender on or off in the WPT harness. r=ato
https://hg.mozilla.org/integration/autoland/rev/21ec7388e3e3
Force WebRender on or off in the raptor harness. r=rwood
https://hg.mozilla.org/integration/autoland/rev/99707ff8f401
Force WebRender on or off in the talos harness. r=perftest-reviewers,rwood
https://hg.mozilla.org/integration/autoland/rev/db9b8bf09e16
Force WebRender on or off in the cppunit harness. r=ahal
https://hg.mozilla.org/integration/autoland/rev/4c861c8ffb42
Force WebRender on or off in the xpcshell harness. r=ahal
https://hg.mozilla.org/integration/autoland/rev/f18f992a51b6
Force WebRender on or off in the marionette harness. r=ato
https://hg.mozilla.org/integration/autoland/rev/ee677b549b51
Fix the enable-webrender option for the telemetry Marionette tests. r=ato
https://hg.mozilla.org/integration/autoland/rev/68c9521ac146
Fix up webrender options for the AWSY harness. r=ato
https://hg.mozilla.org/integration/autoland/rev/4fa58fa41842
Fix up webrender options for the firefox-ui harness. r=ato,whimboo
https://hg.mozilla.org/integration/autoland/rev/459c8d5d0048
Add ignored option for enable-webrender in jit_test harness. r=ahal
https://hg.mozilla.org/integration/autoland/rev/e28668bc08bf
Fix up webrender flag in desktop_unittest.py. r=ahal
https://hg.mozilla.org/integration/autoland/rev/64a3dc9c192a
Simplify the webrender flags in android_emulator_unittest.py. r=ahal
https://hg.mozilla.org/integration/autoland/rev/67f20ed19e3e
Add the --enable-webrender option to android HW unittests. r=ahal
https://hg.mozilla.org/integration/autoland/rev/fe0e4450ad1a
Ensure mozinfo correctly detects WebRender. r=ahal
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/17717 for changes under testing/web-platform/tests
Upstream PR merged
You need to log in before you can comment on or make changes to this bug.