MainThreadIdlePeriod doesn't work in the GPU process

RESOLVED FIXED in Firefox 52

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: rhunt, Assigned: rhunt)

Tracking

unspecified
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

The changes in bug 1198381 added MainThreadIdlePeriod which crashes the GPU process on startup when it accesses Preferences [1], which is not initialized in the GPU process. It doesn't seem like the functionality in bug 1198381 is needed in the GPU process, so it may be best to just disable this somehow when running in the GPU process.


[1] https://hg.mozilla.org/mozilla-central/annotate/333a899fb5e6/xpcom/threads/MainThreadIdlePeriod.cpp#l42
I hit this today too, worked around it by checking for the GPU process (XRE_GetProcessType() != GeckoProcessType_GPU) when creating the MainThreadIdlePeriod [1]

Will upload a patch tomorrow if nobody beats me to it.

[1] https://hg.mozilla.org/mozilla-central/annotate/333a899fb5e6/xpcom/threads/nsThreadManager.cpp#l104
Blocks: e10s-gpu-win
Posted patch idle-prefs-gpu.patch (obsolete) — Splinter Review
This patch prevents MainThreadIdleCallback from using Preferences::AddFloatVarCache on the GPU process. The end result should be that GetLongIdlePeriod should use the default idle period. Idle tasks should not be used in the GPU process so there shouldn't be a problem with this. It's also possible to fix this by disabling idle tasks completely like here [1], but I'm not sure of the benefits of that.

[1] (http://searchfox.org/mozilla-central/source/xpcom/threads/nsThread.cpp#1092)
Attachment #8805966 - Flags: review?(afarre)
Assignee: nobody → rhunt
This might be the same as bug 1314060. I like Preferences::IsServiceAvailable(), whould that work here, say

@@ -37,7 +37,7 @@ MainThreadIdlePeriod::GetLongIdlePeriod()
   static float sLongIdlePeriod = DEFAULT_LONG_IDLE_PERIOD;
   static bool sInitialized = false;
 
-  if (!sInitialized) {
+  if (!sInitialized && Preferences::IsServiceAvailable()) {
     sInitialized = true;
     Preferences::AddFloatVarCache(&sLongIdlePeriod, "idle_queue.long_period",
                                   DEFAULT_LONG_IDLE_PERIOD);

or can't I do Preferences::IsServiceAvailable() on the GPU process?
But yes, I guess that I like Matt's solution even better. For some main threads we might need different nsIIdlePeriods. Should we do both perhaps?
Don't use Preferences in MainThreadIdle until the preferences is available.
Attachment #8805966 - Attachment is obsolete: true
Attachment #8805966 - Flags: review?(afarre)
Attachment #8806097 - Flags: review?(bugs)
Blocks: 1314060
Attachment #8806097 - Flags: review?(bugs) → review+
Pushed by rhunt@eqrion.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4460a5913c3
Don't use Preferences in MainThreadIdlePeriod until they are available r=smaug
https://hg.mozilla.org/mozilla-central/rev/e4460a5913c3
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Duplicate of this bug: 1314060
You need to log in before you can comment on or make changes to this bug.