Bug 233488 – ☂️ Make font code usable from multiple threads

• Status: NEW
• Component: Layout and Rendering

[Cameron McCormack (:heycam)]

OffscreenCanvas needs the ability to call into font code from worker threads.  Bug 219283 was filed to make FontCache and FontCascadeCache thread-safe, but I'm planning to take the approach of giving each worker thread (that needs it) its own FontCache object.  Rather than sharing font objects between threads, each thread will create and use completely independent font objects.  This will mean having all font-related caches exist for each thread, and I plan to handle that by hanging them off FontCache.  There are also a bunch of static variables with initializers in font code, which in the presence of -fno-threadsafe-statics, either need to be non-static, have locking around them, or be made thread-specific by being moved onto FontCache.

I have a bunch of small patches to address the issues I've found so far, which I'll file blocking this bug.

[Chris Lord]

(In reply to Cameron McCormack (:heycam) from comment #0)
> OffscreenCanvas needs the ability to call into font code from worker
> threads.  Bug 219283 was filed to make FontCache and FontCascadeCache
> thread-safe, but I'm planning to take the approach of giving each worker
> thread (that needs it) its own FontCache object.  Rather than sharing font
> objects between threads, each thread will create and use completely
> independent font objects.  This will mean having all font-related caches
> exist for each thread, and I plan to handle that by hanging them off
> FontCache.  There are also a bunch of static variables with initializers in
> font code, which in the presence of -fno-threadsafe-statics, either need to
> be non-static, have locking around them, or be made thread-specific by being
> moved onto FontCache.
> 
> I have a bunch of small patches to address the issues I've found so far,
> which I'll file blocking this bug.

This already happens? As far as I can tell, you're describing how it currently works - There's no thread-safe FontCache singleton, each worker creates their own FontCache. As far as I know, static variables are only accessed on the main thread, and if they aren't, that's a bug that needs addressing.

[Chris Lord]

Ah, unless this isn't specific to OffscreenCanvas and you're talking about a different entry point? The Worker FontCache is only used by OffscreenCanvas as it is at the moment (but it does exist, so hopefully you can just use that and modify whatever's using FontCache::singleton to use the appropriate FontCache from the WorkerGlobalScope instead).

[Chris Lord]

No, I see you mention OffscreenCanvas. Here's some context:

FontCache on WorkerGlobalScope:
https://webkit-search.igalia.com/webkit/source/Source/WebCore/workers/WorkerGlobalScope.cpp#529

Font cache/loading functions on ScriptExecutionContext:
https://webkit-search.igalia.com/webkit/source/Source/WebCore/dom/ScriptExecutionContext.h#171

Worker font loading:
https://webkit-search.igalia.com/webkit/source/Source/WebCore/workers/WorkerFontLoadRequest.h#44
https://webkit-search.igalia.com/webkit/source/Source/WebCore/workers/WorkerFontLoadRequest.cpp#50

CSSFontFace using the correct FontCache:
https://webkit-search.igalia.com/webkit/source/Source/WebCore/css/CSSFontFace.cpp#679

On the whole, this was how any cache objects that were needed by OffscreenCanvas were modified - WorkerGlobalScope provides the cache object, created on demand. You can see this with both WorkerGlobalScope::fontCache() and cssValuePool() (though possibly we can remove this latter one given there are raw parsers for everything now I think?)

There are (almost) no thread-safe objects iirc in the OffscreenCanvas chain. I think BidiContext might be the one exception? https://bugs.webkit.org/show_bug.cgi?id=224179

[Cameron McCormack (:heycam)]

You are right that you already did the work of making each thread have a FontCache!  I described that incorrectly in my comment 0.  But I think there are still a bunch of other, related caches, which are used in a non-thread-safe manner.  For example:

* the FontDatabase objects in FontCacheCoreText.cpp
* the FontMap object in FontFamilySpecificationCoreText.cpp
* the SystemFallbackCache in Font.cpp
* matchSystemFontUse in FontDescriptionCocoa.cpp

and some other smaller, static variables that would have their initializers run concurrently:

* rotateLeftTransform in FontCascadeCoreText.cpp
* expandAroundIdeographs in ComplexTextController::adjustGlyphsAndAdvances
* cascadeToLastResortAndVariationsFontDescriptor in FontPlatformDataCoreText.cpp

Most (but not all) of the things I've found so far are in Cocoa-specific code.

[Radar WebKit Bug Importer]

<rdar://problem/85957172>