Enable e10s with accelerated gfx for XP Users

RESOLVED FIXED in Firefox 50

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jimm, Assigned: jimm)

Tracking

48 Branch
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox50 fixed, firefox51 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

3 years ago
revert a3f8a4e83cdd, per comments in bug 1296282.

Updated

3 years ago
Whiteboard: [gfx-noted]
Assignee

Updated

3 years ago
Assignee: nobody → jmathies
Assignee

Updated

3 years ago
tracking-e10s: --- → +
Assignee

Comment 1

3 years ago
Posted patch enabe XP+e10s+accelerated gfx (obsolete) — Splinter Review
Assignee

Comment 2

3 years ago
Comment on attachment 8787335 [details] [diff] [review]
enabe XP+e10s+accelerated gfx

I believe this is all of the related code. 

https://hg.mozilla.org/mozilla-central/rev/a3f8a4e83cdd

There isn't anything remaining in gfxPlatform.cpp, afaict. The rest matches up to the original landing.
Attachment #8787335 - Flags: review?(felipc)
Comment on attachment 8787335 [details] [diff] [review]
enabe XP+e10s+accelerated gfx

If you think there's any chance of us having to re-enable this block, you could hold off on removing the strings because relanding them would require re-translations.
Attachment #8787335 - Flags: review?(felipc) → review+
Assignee

Comment 4

3 years ago
(In reply to :Felipe Gomes (needinfo me!) from comment #3)
> Comment on attachment 8787335 [details] [diff] [review]
> enabe XP+e10s+accelerated gfx
> 
> If you think there's any chance of us having to re-enable this block, you
> could hold off on removing the strings because relanding them would require
> re-translations.

Ok I'll just comment those out to be safe. Thanks!
Assignee

Comment 5

3 years ago
(In reply to Jim Mathies [:jimm] from comment #4)
> (In reply to :Felipe Gomes (needinfo me!) from comment #3)
> > Comment on attachment 8787335 [details] [diff] [review]
> > enabe XP+e10s+accelerated gfx
> > 
> > If you think there's any chance of us having to re-enable this block, you
> > could hold off on removing the strings because relanding them would require
> > re-translations.
> 
> Ok I'll just comment those out to be safe. Thanks!

Actually, I guess I have to leave them in as unused strings.
Assignee

Comment 6

3 years ago
Attachment #8787335 - Attachment is obsolete: true
Assignee

Updated

3 years ago
Attachment #8787342 - Flags: review+
Assignee

Updated

3 years ago
Flags: needinfo?(jmathies)
Keywords: checkin-needed

Comment 7

3 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2407e891aad
Enable e10s with accelerated gfx for XP Users. r=felipe
Keywords: checkin-needed

Comment 8

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f2407e891aad
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Assignee

Comment 9

3 years ago
Comment on attachment 8787342 [details] [diff] [review]
enabe XP+e10s+accelerated gfx v.2

Approval Request Comment
[Feature/regressing bug #]:
no bug, we specifically prevented accelerated gfx + XP users frome getting e10s a while back. This work turns those user back on so we can collect stability numbers from them in beta.
[User impact if declined]:
no data! no data bad.
[Describe test coverage new/current, TreeHerder]:
on mc
[Risks and why]:
none, this won't roll out to release until we're comfortable with the stability data. 
[String/UUID change made/needed]:
none.
Flags: needinfo?(jmathies)
Attachment #8787342 - Flags: approval-mozilla-aurora?
Hi Jim, Milan, what kind of an impact will this have on Aurora50/Beta50? Will the crash rate go up? Will we monitor the XP users with this configuration on Aurora/Beta channel and disable this again if we notice we are losing a lot of users due to this change? Based on https://bugzilla.mozilla.org/show_bug.cgi?id=1237769#c41, this is an incompatible configuration. Are there any code fixes that have improved the compatibility situation here?
Flags: needinfo?(milan)
Flags: needinfo?(jmathies)
Assignee

Comment 11

3 years ago
(In reply to Ritu Kothari (:ritu) from comment #10)
> Hi Jim, Milan, what kind of an impact will this have on Aurora50/Beta50?
> Will the crash rate go up? 

Don't know, this change will allow us to measure stability to find out.

> Will we monitor the XP users with this
> configuration on Aurora/Beta channel and disable this again if we notice we
> are losing a lot of users due to this change?

Yes, I'm going to be looking at the beta numbers as soon as they start showing up. If we se major stability issues we can back the patch out.

> Based on
> https://bugzilla.mozilla.org/show_bug.cgi?id=1237769#c41, this is an
> incompatible configuration. Are there any code fixes that have improved the
> compatibility situation here?

Yes, the gfx team has been working on the accelerated backend code. We need to figure out where we stand.
Flags: needinfo?(jmathies)
(In reply to Jim Mathies [:jimm] from comment #11)
> ...
> 
> Yes, the gfx team has been working on the accelerated backend code. We need
> to figure out where we stand.

Right, we're willing to see how it works in the wild.
Flags: needinfo?(milan)
Comment on attachment 8787342 [details] [diff] [review]
enabe XP+e10s+accelerated gfx v.2

As Jim and Milan said we will be watching the segment of users for any stability/quality issues and revert if the impact is severe, Aurora50+
Attachment #8787342 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee

Updated

3 years ago
Duplicate of this bug: 1237776
You need to log in before you can comment on or make changes to this bug.