1313978 - MainThreadIdlePeriod doesn't work in the GPU process

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Ryan Hunt [:rhunt]]

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

Comment 1

[Matt Woodrow (:mattwoodrow)]

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

Comment 2

[Ryan Hunt [:rhunt]]

Attachment: idle-prefs-gpu.patch

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)

Comment 3

[Andreas Farre [:farre]]

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?

Comment 4

[Andreas Farre [:farre]]

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?

Comment 5

[Ryan Hunt [:rhunt]]

Attachment: main-thread-idle.patch

Don't use Preferences in MainThreadIdle until the preferences is available.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4460a5913c3
Don't use Preferences in MainThreadIdlePeriod until they are available r=smaug

Comment 7

[Phil Ringnalda (:philor)]

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