Add cross-platform atomics #327
Conversation
saghul
force-pushed
the
atomics
branch
3 times, most recently
from
0aec773 to
47f23e6
Compare
|
I think this is ready for review! Looks like there might be a bug somewhere because the test harness has gotten stuck on (some) Linux jobs, it seems.
Edit: I found a way around for WASI. There is some pthreads compatibility in the WASI SDK I had missed! |
saghul
force-pushed
the
atomics
branch
3 times, most recently
from
f523443 to
a538568
Compare
|
Alright, new strategy. I disabled atomics on Emscripten / WASI, but kept the bytecode compatible by always having the atoms defined. Threading support is still experimental on wasmtime, so I guess WASI can wait. Once tests pass this should be good to go. More Windows portability will follow to enable workers, but that is in the libc, I'm focusing on the engine first. |
| } | ||
| ret = pthread_cond_timedwait(&waiter->cond, &js_atomics_mutex, | ||
| &ts); | ||
| ret = js_cond_timedwait(&waiter->cond, &js_atomics_mutex, timeout * 1e6 /* to ns */); |
There was a problem hiding this comment.
@bnoordhuis This was the reason for the hang. Timeot is in ms for Atomics.wait but in the C wrapper I take ns like libuv 😅
|
And we are 💚 -- @bnoordhuis PTAL! |
| @@ -32561,7 +32560,7 @@ typedef enum BCTagEnum { | |||
| BC_TAG_OBJECT_REFERENCE, | |||
| } BCTagEnum; | |||
|
|
|||
| #define BC_VERSION 9 | |||
| #define BC_VERSION 10 | |||
There was a problem hiding this comment.
For my understanding, is the bump necessary because the atom ids change when the #ifdef CONFIG_ATOMICS is removed from quickjs-atom.h?
There was a problem hiding this comment.
Correct. That's the source of the bytecode incompatibility.
Fixes: #1