Skip to content

Commit 56c46cb

Browse files
committed
Move sleep slicing from musl's __wait to the lower level emscripten_futex_wait. NFC
This means we only need to do this breaking up on our `wait` operations in a single place. It also means that other users the `emscripten_futex_wait` API don't break pthread proxying or async cancellation. The moved code is only included in pthread-enabled builds so should not effect Wasm Workers builders. This change also paves the way for enabling musl's `__wait` to work with `WASM_WORKERS`.
1 parent a6d71cc commit 56c46cb

File tree

5 files changed

+91
-56
lines changed

5 files changed

+91
-56
lines changed

system/lib/libc/musl/src/thread/__wait.c

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,26 +15,11 @@ void __wait(volatile int *addr, volatile int *waiters, int val, int priv)
1515
}
1616
if (waiters) a_inc(waiters);
1717
#ifdef __EMSCRIPTEN__
18-
int is_runtime_thread = emscripten_is_main_runtime_thread();
19-
20-
// Main runtime thread may need to run proxied calls, so sleep in very small slices to be responsive.
21-
double max_ms_slice_to_sleep = is_runtime_thread ? 1 : 100;
22-
23-
while (*addr==val) {
24-
if (is_runtime_thread || pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) {
25-
int e;
26-
do {
27-
if (pthread_self()->cancel) {
28-
if (waiters) a_dec(waiters);
29-
return;
30-
}
31-
// Must wait in slices in case this thread is cancelled in between.
32-
e = emscripten_futex_wait((void*)addr, val, max_ms_slice_to_sleep);
33-
} while (e == -ETIMEDOUT);
34-
} else {
35-
// Can wait in one go.
36-
emscripten_futex_wait((void*)addr, val, INFINITY);
37-
}
18+
// loop here to handler spurious wakeups from the underlying
19+
// emscripten_futex_wait.
20+
int ret = 0;
21+
while (*addr==val && ret == 0) {
22+
ret = emscripten_futex_wait((void*)addr, val, INFINITY);
3823
}
3924
#else
4025
while (*addr==val) {

system/lib/pthread/emscripten_futex_wait.c

Lines changed: 74 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ extern void* _emscripten_main_thread_futex;
1919

2020
static int futex_wait_main_browser_thread(volatile void* addr,
2121
uint32_t val,
22-
double timeout) {
22+
double timeout, bool cancelable) {
2323
// Atomics.wait is not available in the main browser thread, so simulate it
2424
// via busy spinning. Only the main browser thread is allowed to call into
2525
// this function. It is not thread-safe to be called from any other thread.
@@ -45,6 +45,12 @@ static int futex_wait_main_browser_thread(volatile void* addr,
4545
assert(last_addr == 0);
4646

4747
while (1) {
48+
#ifdef __EMSCRIPTEN_PTHREADS__
49+
// We if we were cancelled
50+
if (cancelable && pthread_self()->cancel) {
51+
return -ETIMEDOUT;
52+
}
53+
#endif
4854
// Check for a timeout.
4955
now = emscripten_get_now();
5056
if (now > end) {
@@ -119,48 +125,90 @@ int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms)
119125
return -EINVAL;
120126
}
121127

122-
// Pass 0 here, which means we don't have access to the current time in this
123-
// function. This tells _emscripten_yield to call emscripten_get_now if (and
124-
// only if) it needs to know the time.
125-
_emscripten_yield(0);
126-
127128
int ret;
128129
emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_RUNNING, EM_THREAD_STATUS_WAITFUTEX);
129130

131+
#ifdef __EMSCRIPTEN_PTHREADS__
132+
bool cancelable = pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS;
133+
#else
134+
bool cancelable = false;
135+
#endif
136+
130137
// For the main browser thread and audio worklets we can't use
131138
// __builtin_wasm_memory_atomic_wait32 so we have busy wait instead.
132139
if (!_emscripten_thread_supports_atomics_wait()) {
133-
ret = futex_wait_main_browser_thread(addr, val, max_wait_ms);
140+
ret = futex_wait_main_browser_thread(addr, val, max_wait_ms, cancelable);
134141
emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_WAITFUTEX, EM_THREAD_STATUS_RUNNING);
135142
return ret;
136143
}
137144

138145
// -1 (or any negative number) means wait indefinitely.
139146
int64_t max_wait_ns = ATOMICS_WAIT_DURATION_INFINITE;
140147
if (max_wait_ms != INFINITY) {
141-
max_wait_ns = (int64_t)(max_wait_ms*1000*1000);
148+
max_wait_ns = (int64_t)(max_wait_ms * 1e6);
142149
}
143-
#ifdef EMSCRIPTEN_DYNAMIC_LINKING
144-
// After the main thread queues dlopen events, it checks if the target threads
145-
// are sleeping.
146-
// If `sleeping` is set then the main thread knows that event will be
147-
// processed after the sleep (before any other user code). In this case the
148-
// main thread does not wait for any kind of response form the thread.
149-
// If `sleeping` is not set then we know we should wait for the thread process
150-
// the queue, either from the call here directly after setting `sleeping` to
151-
// 1, or from another callsite (e.g. the one in `emscripten_yield`).
152-
int is_runtime_thread = emscripten_is_main_runtime_thread();
153-
if (!is_runtime_thread) {
154-
__pthread_self()->sleeping = 1;
155-
_emscripten_process_dlopen_queue();
150+
151+
#ifdef __EMSCRIPTEN_PTHREADS__
152+
// When building with pthread support there are two conditions under which we
153+
// need to limit the amount of time we spend in atomic.wait.
154+
// 1. We are the main runtime thread. In this case we need to be able to
155+
// process proxied events from workers. Note that this is not always
156+
// the same as being the main browser thread. For example, when running
157+
// under node or when launching and emscripten-built program in a Web
158+
// Worker. This this case we limit our wait slices to 1ms intervals.
159+
// 2. When the current thread has async cancellation enabled. In this case
160+
// we limit the wait duration to 100ms intervals.
161+
int64_t wakeup_interval = 0;
162+
bool is_runtime_thread = emscripten_is_main_runtime_thread();
163+
if (is_runtime_thread) {
164+
// If the current thread is the main runtime thread then only wait in 1ms slices.
165+
wakeup_interval = 1 * 1e6;
166+
}
167+
else if (cancelable) {
168+
// If the current thread is async cancellable then only wait in 100ms slices.
169+
wakeup_interval = 100 * 1e6;
170+
}
171+
172+
int64_t interations;
173+
if (wakeup_interval) {
174+
interations = max_wait_ns / wakeup_interval;
175+
max_wait_ns = wakeup_interval;
156176
}
177+
178+
do {
157179
#endif
158-
ret = __builtin_wasm_memory_atomic_wait32((int*)addr, val, max_wait_ns);
180+
// Pass 0 here, which means we don't have access to the current time in this
181+
// function. This tells _emscripten_yield to call emscripten_get_now if (and
182+
// only if) it needs to know the time.
183+
_emscripten_yield(0);
184+
159185
#ifdef EMSCRIPTEN_DYNAMIC_LINKING
160-
if (!is_runtime_thread) {
161-
__pthread_self()->sleeping = 0;
162-
_emscripten_process_dlopen_queue();
163-
}
186+
// After the main thread queues dlopen events, it checks if the target threads
187+
// are sleeping.
188+
// If `sleeping` is set then the main thread knows that event will be
189+
// processed after the sleep (before any other user code). In this case the
190+
// main thread does not wait for any kind of response form the thread.
191+
// If `sleeping` is not set then we know we should wait for the thread process
192+
// the queue, either from the call here directly after setting `sleeping` to
193+
// 1, or from another callsite (e.g. the one in `emscripten_yield`).
194+
if (!is_runtime_thread) {
195+
__pthread_self()->sleeping = 1;
196+
_emscripten_process_dlopen_queue();
197+
}
198+
#endif
199+
ret = __builtin_wasm_memory_atomic_wait32((int*)addr, val, max_wait_ns);
200+
#ifdef EMSCRIPTEN_DYNAMIC_LINKING
201+
if (!is_runtime_thread) {
202+
__pthread_self()->sleeping = 0;
203+
_emscripten_process_dlopen_queue();
204+
}
205+
#endif
206+
#ifdef __EMSCRIPTEN_PTHREADS__
207+
if (cancelable && ret == ATOMICS_WAIT_TIMED_OUT && pthread_self()->cancel) {
208+
// We were cancelled
209+
break;
210+
}
211+
} while (wakeup_interval && ret == ATOMICS_WAIT_TIMED_OUT && (max_wait_ms == INFINITY || interations--));
164212
#endif
165213

166214
emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_WAITFUTEX, EM_THREAD_STATUS_RUNNING);

system/lib/pthread/library_pthread.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,10 @@ void emscripten_thread_sleep(double msecs) {
8787
// If we have less than this many msecs left to wait, busy spin that instead.
8888
double min_ms_slice_to_sleep = 0.1;
8989

90-
// runtime thread may need to run proxied calls, so sleep in very small slices to be responsive.
91-
double max_ms_slice_to_sleep = emscripten_is_main_runtime_thread() ? 1 : 100;
90+
// Break up sleeping so that we process proxied work at regular intervals.
91+
// TODO(sbc): This should be remove and/or moved down into
92+
// `emscripten_futex_wait`.
93+
double max_ms_slice_to_sleep = 100;
9294

9395
emscripten_conditional_set_current_thread_status(
9496
EM_THREAD_STATUS_RUNNING, EM_THREAD_STATUS_SLEEPING);

test/codesize/test_codesize_minimal_pthreads.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
22
"a.out.js": 7884,
33
"a.out.js.gz": 3855,
4-
"a.out.nodebug.wasm": 19707,
5-
"a.out.nodebug.wasm.gz": 9122,
6-
"total": 27591,
7-
"total_gz": 12977,
4+
"a.out.nodebug.wasm": 19775,
5+
"a.out.nodebug.wasm.gz": 9202,
6+
"total": 27659,
7+
"total_gz": 13057,
88
"sent": [
99
"a (memory)",
1010
"b (emscripten_get_now)",

test/codesize/test_codesize_minimal_pthreads_memgrowth.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
22
"a.out.js": 8306,
33
"a.out.js.gz": 4060,
4-
"a.out.nodebug.wasm": 19708,
5-
"a.out.nodebug.wasm.gz": 9123,
6-
"total": 28014,
7-
"total_gz": 13183,
4+
"a.out.nodebug.wasm": 19776,
5+
"a.out.nodebug.wasm.gz": 9203,
6+
"total": 28082,
7+
"total_gz": 13263,
88
"sent": [
99
"a (memory)",
1010
"b (emscripten_get_now)",

0 commit comments

Comments
 (0)