Improve emscripten_futex_wait stub. NFC#26309
Conversation
sbc100
force-pushed
the
futex_wait_workers
branch
3 times, most recently
from
7ce2891 to
e36db36
Compare
|
This was also causing some confusion in #26283 |
sbc100
force-pushed
the
futex_wait_workers
branch
from
e36db36 to
1cd0d28
Compare
|
I'd like to start work on getting the futex API actually working in pure wasm workers, but I think we should land this change first so that we have a baseline / status quo in the tests. |
sbc100
force-pushed
the
futex_wait_workers
branch
from
1cd0d28 to
3f7dff6
Compare
|
I now have #26325 which depends on this, and makes the futex API available under wasm workers. |
| // TODO: Convert to emscripten_futex_wait once it becomes available in Wasm | ||
| // Workers. | ||
| while (1) { | ||
| if (workletToWorkerFlag == true) { |
There was a problem hiding this comment.
How long does this test run? This could be quite a CPU burner overhead for the test.
Maybe the while() part could be gated behind a
#ifndef __EMSCRIPTEN_WASM_WORKERS__
while (0 == emscripten_futex_wait(&workletToWorkerFutexLocation, 0, 30000))
#endif
{
...
}
so in the Wasm Workers mode only it would avoid the futex wait.
There was a problem hiding this comment.
On my machine this busy looks runs for 2 or 3 ms.
I added some log information to record how long the busy wait is.
Note that the code busy waits too because emscripten_futex_wait is a no-op on wasm workers.
This test only ever run in wasm workers only mode.
juj
left a comment
juj
left a comment
There was a problem hiding this comment.
LGTM modulo that one comment
The test for wasm workers with `emscripten_futex_wait` was added in back in emscripten-core#21618 but AFAICT it was only even working when pthread support was also enabled. However because the stub was simply returning 0 in all cases it was enough to make it seems as if the API was working when it wasn't. Indeed the `test/webaudio/audioworklet_worker.c` test seems to have been written under the assumption that this API was available for use in wasm workers. It seems better to return ENOTSUP unless the API is actually available.
sbc100
force-pushed
the
futex_wait_workers
branch
from
3f7dff6 to
1d879be
Compare
2 participants
The test for wasm workers with emscripten_futex_wait was added in back in #21618 but AFAICT it was only even working when pthread support was also enabled.
Then pthread support is not enabled only the stub version is used which obviously doesn't work as expected.
I think it would be great to get this API working in pure wasm workers, but that is separate issue.
See #26314