Summary: | ☂️ Make font code usable from multiple threads | ||
---|---|---|---|
Product: | WebKit | Reporter: | Cameron McCormack (:heycam) <heycam> |
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> |
Status: | NEW --- | ||
Severity: | Normal | CC: | ap, bfulgham, clord, mmaxfield, simon.fraser, webkit-bug-importer, zalan |
Priority: | P2 | Keywords: | InRadar |
Version: | WebKit Local Build | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Bug Depends on: | 233971, 233979, 234052, 234058, 233747, 233749, 233750, 233972, 233974, 233975, 233976, 233978, 233981, 233982, 233983, 233984, 233986, 234042, 234044, 234045, 234049, 234050, 234053, 234054, 234056, 234057, 234060, 234061 | ||
Bug Blocks: |
Description
Cameron McCormack (:heycam)
2021-11-24 23:10: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. 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). 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 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. |