Add a new reftest column to Windows 7 test machines

RESOLVED FIXED

Status

defect
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: dvander, Assigned: RyanVM)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

We're getting close to enabling the GPU process (one of the Quantum initiatives) on Nightly. I'd like to get a new reftest column on Windows 7 machines, running reftests an additonal time with an additional pref:

  layers.gpu-process.force-enabled = true

The reason for this is that we'll have a hole in our testing configuration otherwise. Currently, reftests will cover the following combinations of GPU process and hardware acceleration:

                     Unaccelerated    Accelerated
-----------------+-----------------+---------------
GPU Process      |   None          |   Win 8
No GPU Process   |   Win 7         |   win 8

Having a Windows 7-specific column should solve this. It should be okay to have this column on either the VM or non-VM machines, or both.
Presumably this would need to go onto a Win7 GPU instance like the other Win7 reftests run on.

To summarize the current state of our automation:
Win7 reftest:         "layersGPUAccelerated":true,  "d3d11":true,  "d2d":false, "dwrite":false
Win7 reftest-noaccel: "layersGPUAccelerated":false, "d3d11":false, "d2d":false, "dwrite":false
Win8 reftest:         "layersGPUAccelerated":true,  "d3d11":true,  "d2d":true,  "dwrite":true
Win8 reftest-noaccel: "layersGPUAccelerated":false, "d3d11":false, "d2d":false, "dwrite":true

And all 4 of the above run with both e10s enabled and disabled. What is this new Win7-gpu job going to look like? I guess what I'm not understanding at the moment is how this pref will affect the Win8 jobs, since I'm assuming we'll want to continue testing with the GPU process both enabled and disabled?
we will need to determine if this works on the win7-vm instances as desired, or if this requires hardware.
(In reply to Ryan VanderMeulen [:RyanVM] from comment #1)
> Presumably this would need to go onto a Win7 GPU instance like the other
> Win7 reftests run on.
> 
> To summarize the current state of our automation:
> Win7 reftest:         "layersGPUAccelerated":true,  "d3d11":true, 
> "d2d":false, "dwrite":false
> Win7 reftest-noaccel: "layersGPUAccelerated":false, "d3d11":false,
> "d2d":false, "dwrite":false
> Win8 reftest:         "layersGPUAccelerated":true,  "d3d11":true, 
> "d2d":true,  "dwrite":true
> Win8 reftest-noaccel: "layersGPUAccelerated":false, "d3d11":false,
> "d2d":false, "dwrite":true
> 
> And all 4 of the above run with both e10s enabled and disabled. What is this
> new Win7-gpu job going to look like? I guess what I'm not understanding at
> the moment is how this pref will affect the Win8 jobs, since I'm assuming
> we'll want to continue testing with the GPU process both enabled and
> disabled?

Unless I'm misunderstanding the question: this additional pref would only be for Windows 7, and only for an additional run of Reftests. It wouldn't affect Windows 8.

It shouldn't matter whether it runs on VMs or not.
OK, I chatted with David on IRC and got the missing bit of info - we detect the DXGI version and enable the GPU process automatically for Win7 w/ the Platform Update installed and Win8+, hence why our Win7 test machines are the only ones in need of anything special here.

So yeah, it sounds like we'll want a nightly-only (not riding the trains yet AFAIK) reftest-gpu job (Symbol Rg). Looks pretty easy to add to win_unittest.py. And then we'll need buildbot-configs and treeherder patches to go with it.
Assignee: nobody → ryanvm
Pretty sure this is all we need for the in-tree configs.
Attachment #8807289 - Flags: review?(jmaher)
I added it to win_taskcluster_unittest.py as well, but I'm not adding it to any other Taskcluster-related configs for the time-being. Win7 tests on TC are still a ways off IIUC.
Attachment #8807289 - Attachment is obsolete: true
Attachment #8807289 - Flags: review?(jmaher)
Attachment #8807293 - Flags: review?(jmaher)
Attachment #8807293 - Attachment is obsolete: true
Attachment #8807293 - Flags: review?(jmaher)
Comment on attachment 8807289 [details] [diff] [review]
add reftest-gpu to win_unittest.py

Thinking it over more, there's really not much reason to add reftest-gpu to the mach commands since the same thing can be accomplished by just invoking the regular reftest command with the pref included on the command line. Let's keep it simple and stick with just worrying about automation support.
Attachment #8807289 - Attachment is obsolete: false
Attachment #8807289 - Flags: review?(jmaher)
Comment on attachment 8807289 [details] [diff] [review]
add reftest-gpu to win_unittest.py

Review of attachment 8807289 [details] [diff] [review]:
-----------------------------------------------------------------

this looks great- now to add buildbot patches.

::: testing/mozharness/configs/unittests/win_taskcluster_unittest.py
@@ +220,5 @@
>          },
> +        "reftest-gpu": {
> +            'options': ['--suite=reftest',
> +                        '--setpref=layers.gpu-process.force-enabled=true'],
> +            'tests': ["tests/reftest/tests/layout/reftests/reftest.list"]

nit; can you make this single quotes?

::: testing/mozharness/configs/unittests/win_unittest.py
@@ +220,5 @@
>          },
> +        "reftest-gpu": {
> +            'options': ['--suite=reftest',
> +                        '--setpref=layers.gpu-process.force-enabled=true'],
> +            'tests': ["tests/reftest/tests/layout/reftests/reftest.list"]

same nit here about single quotes- consistency is what I am looking for
Attachment #8807289 - Flags: review?(jmaher) → review+
Depends on: 1315249
David, is it safe to say that this job will only ever run with e10s enabled? Seems that way offhand, but better to ask and be sure :)
Flags: needinfo?(dvander)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)
> David, is it safe to say that this job will only ever run with e10s enabled?
> Seems that way offhand, but better to ask and be sure :)

Yeah, definitely. GPU process is e10s-only.
Flags: needinfo?(dvander)
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b15e9c4c5b6a
Add support for a reftest-gpu job that forces GPU composition on. r=jmaher
Keywords: leave-open
Running these on Try only to start. Once they look good there, we can easily turn them on across the board.

Builders added:
+ Windows 7 VM-GFX 32-bit try debug test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit try opt test reftest-gpu-e10s
Attachment #8808361 - Flags: review?(rail)
Comment on attachment 8808361 [details] [diff] [review]
schedule reftest-gpu-e10s on win7 gfx vm aws instances on Try

stamp
Attachment #8808361 - Flags: review?(rail) → review+
Comment on attachment 8808361 [details] [diff] [review]
schedule reftest-gpu-e10s on win7 gfx vm aws instances on Try

https://hg.mozilla.org/build/buildbot-configs/rev/6b8d9baf752fedb40abb0bb159e6cf3e544d21de
Attachment #8808361 - Flags: checked-in+
Looks good on Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=da6c9c8d67467c0b4ddf4769f3d78fb8a5af79ec&group_state=expanded

That's with the pref name fixed. And in the debug runs, I see:
INFO - TEST-INFO | leakcheck | gpu process: leak threshold set at 0 bytes

I think we're in good shape here, David! Per our IRC discussion yesterday, enabling across all 52+ trees is easy at this point, pending the pref renaming.
Now that the gpuProcess annotation is available, I've been able to fire off new Try pushes. I can confirm that Win8 reftest-e10s shows gpuProcess:true as expected. However, Win7 reftest-gpu-e10s still shows it as false (with the pref for the test suite set to layers.gpu-process.dev.enabled).
https://archive.mozilla.org/pub/firefox/try-builds/ryanvm@gmail.com-7a95ca98aa4432336513a7799fbf950169cd72c6/try-win32/try_win7_vm_gfx_test-reftest-gpu-e10s-bm138-tests1-windows-build8.txt.gz

Looks like it's still being disabled somewhere at the Gecko level even with the pref set?
Flags: needinfo?(dvander)
I believe the preference got renamed to layers.gpu-process.dev.enabled.
(In reply to Milan Sreckovic [:milan] from comment #19)
> I believe the preference got renamed to layers.gpu-process.dev.enabled.

And the harness is using layers.gpu-process.force-enabled instead?
I noted in comment 18 that I changed the pref for the Try push.
https://hg.mozilla.org/try/rev/7a95ca98aa4432336513a7799fbf950169cd72c6
Bug 1330554 has landed and the GPU process is now riding the 53 train. Need to get this enabled across all 53+ trees now.
Depends on: 1330554
Flags: needinfo?(dvander)
Keywords: leave-open
That proved to be exceedingly easy.

Builders added:
+ Windows 7 VM-GFX 32-bit ash debug test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit ash opt test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit ash pgo test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit autoland debug test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit autoland opt test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit autoland pgo test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit elm debug test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit elm opt test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit larch debug test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit larch opt test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit larch pgo test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit mozilla-central debug test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit mozilla-central opt test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit mozilla-central pgo test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit mozilla-inbound debug test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit mozilla-inbound opt test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit mozilla-inbound pgo test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit oak debug test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit oak opt test reftest-gpu-e10s
+ Windows 7 VM-GFX 32-bit oak pgo test reftest-gpu-e10s
Attachment #8828131 - Flags: review?(rail)
Attachment #8828131 - Flags: review?(rail) → review?(aki)
Attachment #8828131 - Flags: review?(aki) → review+
In production.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
I would like to revisit the reftest-gpu tests again-
* with windows 10 (and not windows 8) are we testing the right things
* with quantum, do we need reftest-gpu (win7) and reftest-no-accel (win7, win10, linux)
* do we need gpu for opt/debug/pgo ?
* if we are keeping all options, do we need all the tests for each option?

My primary goal here is to reduce the load we see on the VM+GPU instances- they are limited and expensive (more so in the last few months), likewise if we could do fewer chunks on windows 7 (8 instead of 32) but that could mean turning off some tests in these scenarios such as the font* tests.
Flags: needinfo?(ryanvm)
That'd be a question for the graphics team.
Flags: needinfo?(ryanvm) → needinfo?(milan)
With WebRender/Quantum, we only need Windows 10 testing, with acceleration.  Debug and PGO would be good, I'm not too worried about Opt.  And, Windows 10, but not Windows 8 is fine.
Flags: needinfo?(milan)
so to confirm:
windows7 - keep everything there (reftest, reftest-gpu, reftest-no-accel x opt/debug/pgo)
Windows10 - we only need reftest, we can drop reftest-no-accel and reftest-gpu?  If we run on debug/pgo, opt is not much extra- helps on try pushes to have opt.

:milan, if you can confirm the above statements, I can change what we have for windows10 and leave windows7 alone.
Flags: needinfo?(milan)
We want reftest-gpu for Windows 10 and quantum/webrender.  Don't need webrender on Windows 7 at all.
Flags: needinfo?(milan)
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.