947025 - limit time spent in font loader RunLoader calls

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[John Daggett (:jtd)]

The chrome hang data on bug 859558 shows a significant number of hangs within RunLoader calls.  For some reason I thought we already had an explicit time limit to prevent this but apparently not.

The gfxPlatformFontList::RunLoader method is called on a timer and loads the data for a pref-specified number of font families.  The pref can be tweaked but I think we should explicitly limit each pass to prevent hangs.

Comment 1

[John Daggett (:jtd)]

Attachment: patch, add time limit to RunLoader

Limit the time spent within one pass of RunLoader to 500ms.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8343444 [details] [diff] [review]
patch, add time limit to RunLoader

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

::: gfx/thebes/gfxPlatformFontList.cpp
@@ +689,5 @@
>      mStartIndex = 0;
>      mNumFamilies = mFontFamiliesToLoad.Length();
>  }
>  
> +#define FONT_LOADER_MAX_TIMESLICE 500  // max time for one pass through RunLoader = 500ms

How about lowering this to 100ms?

Comment 3

[John Daggett (:jtd)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #2)

> > +#define FONT_LOADER_MAX_TIMESLICE 500  // max time for one pass through RunLoader = 500ms
> 
> How about lowering this to 100ms?

Sure.

Comment 4

[John Daggett (:jtd)]

Pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dba508605c1b

Comment 5

[Carsten Book [:Tomcat]]

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

Comment 6

[Ioana (away)]

Please re-add "verifyme" if you want QA to test the fix for this bug (details on how to reproduce it and the used tools would also be necessary in this case).