-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Description
I have a draft of an initial impeller web implementation here: #175442
With multi-threading turned on, this runs into frequent crashes/memory corruption. In single-threaded mode, it is stable.
The reason for this instability is a race condition with the SkStrikeCache that is used to calculate the size of glyphs for the glyph atlas. The strike cache is used on the main thread to do paragraph layout as well as part of our conversion from SkTextBlob to an impeller text frame. It also is used on the raster thread right when we allocate space on the glyph atlas for new glyphs.
Skia uses one global SkStrikeCache for all threads, and it is protected by a lock. However, with the WASM_WORKERS flag (which we are currently using) emscripten apparently stubs out mutexes and semaphores so that they are no-ops, which makes use of this global cache thread unsafe. (See this bug for details about emscripten's behavior).
We have a few different options moving forward.
- Convert to emscripten's pthreads flag instead of the wasm workers flag, which makes mutexes work. However, the mutex implementation provided with the pthreads flag, particularly on the main thread, is problematic. The browser does not allow you to block on the mutex, so if there is contention for the lock, the main thread busy-waits, which is really unfortunate. As a result, I'd consider this a not very attractive option.
- Skia has agreed to expose a flag which allows us to opt-in to thread-local caches instead of global (see this skia bug for details). I have tested with a hacked up version of this flag and it appears to resolve the stability issues. However, this does mean we don't actually get the benefits of caching across the different phases where we use the strike cache.
- Move the calculations of glyph bounds that we are doing on the raster thread to the main thread when we create the text frame, which will make sure we are only accessing the strike cache from the main thread. I started working on a change here to make this the case, but it's tricky, and I don't have it working completely yet.
In the meantime, I am going to make my initial impeller web implementation single-threaded only until we can use at least one of these solutions above to resolve the instability in the multi-threaded implementation.