[WASM_WORKERS] Replace call_once usage in libc++ with atomics#26425
[WASM_WORKERS] Replace call_once usage in libc++ with atomics#26425sbc100 wants to merge 1 commit intoemscripten-core:mainfrom
call_once usage in libc++ with atomics#26425Conversation
sbc100
force-pushed
the
libcxx_remove_call_once
branch
from
148c036 to
734e654
Compare
sbc100
force-pushed
the
libcxx_remove_call_once
branch
3 times, most recently
from
b501f06 to
a68e258
Compare
| __libcpp_atomic_compare_exchange(&__id_, &expected, proposed_id); | ||
| } | ||
| #else | ||
| call_once(__flag_, [&] { __id_ = __libcpp_atomic_add(&__next_id, 1); }); |
There was a problem hiding this comment.
Can we leave this file unchanged but reimplement call_once using the new code above?
There was a problem hiding this comment.
Indeed, I considered something like that. But to implement call_once correctly I think we would really need something like a mutex+condvar, at least that is currently how libc++ implements call_once (using __libcpp_mutex_t + __libcpp_condvar_t).
This little construct is less complex/powerful than mutex+condvar I think.
That is what the TODO here is for.
I maybe have through Gemini at this and have it try to implement that for us .. at least I'll open a bug now.
This avoid a dependency on pthread API (which are used to implement call_once under the hood) which makes libc++ much more usable from Wasm Workers (there pthreads are not available). The downside here is that when thread race to be first one to use a given facet we would "leak" an ID which could cause the `facets_` vector to become more spare that it otherwise would be. IIUC this `facets_` vector can already be sparse so it not clear to me what impact this would have in practice. We could make this change only for the Wasm Workers build of libc++ but I'd like to get as much test coverage of it as I can. See: emscripten-core#26375
sbc100
force-pushed
the
libcxx_remove_call_once
branch
from
a68e258 to
03ba3a3
Compare
sbc100
changed the title
call_once usage in libc++ with atomicscall_once usage in libc++ with atomics
|
I updated this change so it only effect wasm workers builds. Now there are not codesize changes to test updates needed. |
| self.do_run_in_out_file_test('wasm_worker/wasm_worker_cxx_init.cpp', cflags=['-sWASM_WORKERS']) | ||
|
|
||
| def test_wasm_worker_hello_libcxx(self): | ||
| self.do_runf('codesize/hello_libcxx.cpp', 'hello, world!\n', cflags=['-sWASM_WORKERS']) |
There was a problem hiding this comment.
| self.do_runf('codesize/hello_libcxx.cpp', 'hello, world!\n', cflags=['-sWASM_WORKERS']) | |
| self.do_runf('hello_libcxx.cpp', 'hello, world!\n', cflags=['-sWASM_WORKERS']) |
|
I think I might hold off on this change to see if we can instead use emscripten_lock_xx from wasm_worker.h |
3 participants
This avoid a dependency on pthread API (which are used to implement call_once under the hood) which makes libc++ much more usable from Wasm workers (there pthreads are not available).
The downside here is that when thread race to be first one to use a given facet we would "leak" an ID which could cause the
facets_vector to become more spare that it otherwise would be.IIUC this
facets_vector can already be sparse so it not clear to me what impact this would have in practice.See: #26375