Bug 233488 - ☂️ Make font code usable from multiple threads
Summary: ☂️ Make font code usable from multiple threads
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on: 233971 233972 233974 233975 233976 233978 233979 233981 233982 233983 233984 233986 234042 234044 234045 234049 234050 234052 234053 234054 234056 234057 234058 234060 234061 233747 233749 233750
Blocks:
  Show dependency treegraph
 
Reported: 2021-11-24 23:10 PST by Cameron McCormack (:heycam)
Modified: 2021-12-08 18:51 PST (History)
7 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Cameron McCormack (:heycam) 2021-11-24 23:10:34 PST
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.
Comment 1 Chris Lord 2021-11-25 01:22:34 PST
(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.
Comment 2 Chris Lord 2021-11-25 01:24:58 PST
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).
Comment 3 Chris Lord 2021-11-25 01:39:42 PST
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
Comment 4 Cameron McCormack (:heycam) 2021-11-25 13:02:58 PST
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.
Comment 5 Radar WebKit Bug Importer 2021-12-01 23:11:24 PST
<rdar://problem/85957172>