Closed
Bug 1458638
Opened 7 years ago
Closed 7 years ago
It looks like Skia is no longer being used on talos on windows
Categories
(Testing :: Talos, defect, P3)
Tracking
(firefox61 fixed)
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: rhunt, Assigned: jmaher)
References
Details
(Whiteboard: [PI:May])
Attachments
(1 file)
(deleted),
patch
|
rwood
:
review+
|
Details | Diff | Splinter Review |
Previously we had Skia being tested on talos under the `Windows 7` job category. It doesn't look like that's the case any longer.
Looking at this graph for rasterflood_gradient times, I noticed that Windows 7 jumped up from the 400s to the 1300s [1]. The 1300s are about the range that Windows 10 which uses Direct2D typically perform.
The commits at that time point to bug 1431161 which changed the hardware under which jobs are run.
Joel was this an intentional change? I think we definitely should be still testing Skia.
I assumed we forced it to always run by disabling Direct2D with `gfx.direct2d.disabled`, but it looks like we might just use hardware detection, which would mean a hardware change could just flip this.
This is important because the changes in bug 1457007 only affect windows with skia and were supposed to have a good performance benefit, but haven't triggered any changes in talos. (Besides a memory regression because AWSY still seems to test Skia)
[1] https://treeherder.mozilla.org/perf.html#/graphs?timerange=5184000&series=mozilla-central,1650998,1,1&zoom=1522412309378.493,1522481722926.3335,18.656602546350314,1451.4924234418727&selected=mozilla-central,1650998,321999,441082837,1
Reporter | ||
Updated•7 years ago
|
Flags: needinfo?(jmaher)
Assignee | ||
Comment 1•7 years ago
|
||
Hi Ryan, thanks for asking about this. I had consulted with :milan about this back in December and it was determined that we could test 32 bit builds of Firefox on new Windows 10 64 bit hardware/OS. The hardware and OS is the exact same between win7/win10, it is just a 32 bit build vs a 64 bit build.
Is it possible to set a preference for win7 (32 bit builds)?
Flags: needinfo?(jmaher)
Reporter | ||
Comment 2•7 years ago
|
||
Okay that makes sense. It seems like this was just an oversight and we were relying on the hardware detection to pick Skia, which worked until that upgrade.
If we were to set `gfx.direct2d.disabled = true` for the windows 7 runs, that should do the trick. I'm not sure where the best place to make that change is, but I can write the patch if you'd like.
Assignee | ||
Comment 3•7 years ago
|
||
ni myself to work on this
Flags: needinfo?(jmaher)
Whiteboard: [PI:May]
Assignee | ||
Comment 4•7 years ago
|
||
:rhunt, how can I examine the logs to learn if we are testing in skia or not?
Flags: needinfo?(jmaher) → needinfo?(rhunt)
Reporter | ||
Comment 5•7 years ago
|
||
Thanks for looking into this!
Hmm, from looking at a talos log, I'm not sure if we have a good way to do that. Which is probably one of the reasons this wasn't noticed.
I know in reftest logs it will print out what backend (and other information) at the beginning of a test. Maybe we could do that here? Here is where the reftest sandbox get's built and collects that information [1]
[1] https://searchfox.org/mozilla-central/rev/ce9ff94ffed34dc17ec0bfa406156d489eaa8ee1/layout/tools/reftest/manifest.jsm#426
Flags: needinfo?(rhunt)
Assignee | ||
Comment 6•7 years ago
|
||
I have been trying to get this to print out unsuccessfully for the last couple hours. I think the way to validate this is looking at what preferences are set and just assume it is working.
Reporter | ||
Comment 7•7 years ago
|
||
That seems reasonable to me, if that pref is set we should use skia unconditionally in the default case.
Assignee | ||
Comment 8•7 years ago
|
||
Comment 9•7 years ago
|
||
Comment on attachment 8973293 [details] [diff] [review]
add preference for skia on win32 builds for talos
Review of attachment 8973293 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM
Attachment #8973293 -
Flags: review?(rwood) → review+
Comment 10•7 years ago
|
||
Pushed by jmaher@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc1a6c069b71
It looks like Skia is no longer being used on talos on windows. r=rwood
Comment 11•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in
before you can comment on or make changes to this bug.
Description
•