Skip to content

Commit 6b52611

Browse files
committed
Simplify emscripten_futex_wait on them main thread
The `_emscripten_futex_wait_non_blocking` contains a busy loop that and was calling out to `emscripten_main_thread_process_queued_calls` on each iteration. However, all the caller of `emscripten_futex_wait` already do their own busy loop calling `emscripten_main_thread_process_queued_calls`. See: - __wait.c: __wait - __timed_wait.c: __timedwait_cp - pthread_barrier_wait.c: pthread_barrier_wait - pthread_join.c: __pthread_join_internal - library_pthread.c: emscripten_thread_sleep In all these cases `emscripten_futex_wait` is limited to a max sleep of 1ms when running on the main thread. There is only one of other user and in this case emscripten_fetch_wait is only used when not running on the main thread: - emscripten_fetch.c: emscripten_fetch_wait
1 parent 6f2a8f5 commit 6b52611

File tree

1 file changed

+9
-72
lines changed

1 file changed

+9
-72
lines changed

src/library_pthread.js

Lines changed: 9 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -797,6 +797,8 @@ var LibraryPThread = {
797797
return 0;
798798
},
799799

800+
// Fallback function used by emscripten_futex_wait for when Atomics.wait is not
801+
// available (i.e. in the main browser thread).
800802
// Returns 0 on success, or one of the values -ETIMEDOUT, -EWOULDBLOCK or -EINVAL on error.
801803
_emscripten_futex_wait_non_blocking__deps: ['emscripten_main_thread_process_queued_calls'],
802804
_emscripten_futex_wait_non_blocking: function(addr, val, timeout) {
@@ -805,50 +807,30 @@ var LibraryPThread = {
805807
assert(ENVIRONMENT_IS_WEB);
806808
#endif
807809

808-
// Atomics.wait is not available in the main browser thread, so simulate it via busy spinning.
809810
var tNow = performance.now();
810811
var tEnd = tNow + timeout;
811812

813+
if (Atomics.load(HEAP32, addr >> 2) != val) {
814+
return -{{{ cDefine('EWOULDBLOCK') }}};
815+
}
816+
812817
// Register globally which address the main thread is simulating to be
813818
// waiting on. When zero, the main thread is not waiting on anything, and on
814819
// nonzero, the contents of the address pointed by __emscripten_main_thread_futex
815820
// tell which address the main thread is simulating its wait on.
816-
// We need to be careful of recursion here: If we wait on a futex, and
817-
// then call _emscripten_main_thread_process_queued_calls() below, that
818-
// will call code that takes the proxying mutex - which can once more
819-
// reach this code in a nested call. To avoid interference between the
820-
// two (there is just a single __emscripten_main_thread_futex at a time), unmark
821-
// ourselves before calling the potentially-recursive call. See below for
822-
// how we handle the case of our futex being notified during the time in
823-
// between when we are not set as the value of __emscripten_main_thread_futex.
824-
#if ASSERTIONS
825-
assert(__emscripten_main_thread_futex > 0);
826-
#endif
827821
var lastAddr = Atomics.exchange(HEAP32, __emscripten_main_thread_futex >> 2, addr);
828822
#if ASSERTIONS
829-
// We must not have already been waiting.
830823
assert(lastAddr == 0);
831824
#endif
832825

833-
while (1) {
826+
while (lastAddr) {
834827
// Check for a timeout.
835828
tNow = performance.now();
836829
if (tNow > tEnd) {
837-
// We timed out, so stop marking ourselves as waiting.
838-
lastAddr = Atomics.exchange(HEAP32, __emscripten_main_thread_futex >> 2, 0);
839-
#if ASSERTIONS
840-
// The current value must have been our address which we set, or
841-
// in a race it was set to 0 which means another thread just allowed
842-
// us to run, but (tragically) that happened just a bit too late.
843-
assert(lastAddr == addr || lastAddr == 0);
844-
#endif
830+
// We timed out
845831
return -{{{ cDefine('ETIMEDOUT') }}};
846832
}
847-
// We are performing a blocking loop here, so we must handle proxied
848-
// events from pthreads, to avoid deadlocks.
849-
// Note that we have to do so carefully, as we may take a lock while
850-
// doing so, which can recurse into this function; stop marking
851-
// ourselves as waiting while we do so.
833+
852834
lastAddr = Atomics.exchange(HEAP32, __emscripten_main_thread_futex >> 2, 0);
853835
#if ASSERTIONS
854836
assert(lastAddr == addr || lastAddr == 0);
@@ -857,51 +839,6 @@ var LibraryPThread = {
857839
// We were told to stop waiting, so stop.
858840
break;
859841
}
860-
_emscripten_main_thread_process_queued_calls();
861-
862-
// Check the value, as if we were starting the futex all over again.
863-
// This handles the following case:
864-
//
865-
// * wait on futex A
866-
// * recurse into emscripten_main_thread_process_queued_calls(),
867-
// which waits on futex B. that sets the __emscripten_main_thread_futex address to
868-
// futex B, and there is no longer any mention of futex A.
869-
// * a worker is done with futex A. it checks __emscripten_main_thread_futex but does
870-
// not see A, so it does nothing special for the main thread.
871-
// * a worker is done with futex B. it flips mainThreadMutex from B
872-
// to 0, ending the wait on futex B.
873-
// * we return to the wait on futex A. __emscripten_main_thread_futex is 0, but that
874-
// is because of futex B being done - we can't tell from
875-
// __emscripten_main_thread_futex whether A is done or not. therefore, check the
876-
// memory value of the futex.
877-
//
878-
// That case motivates the design here. Given that, checking the memory
879-
// address is also necessary for other reasons: we unset and re-set our
880-
// address in __emscripten_main_thread_futex around calls to
881-
// emscripten_main_thread_process_queued_calls(), and a worker could
882-
// attempt to wake us up right before/after such times.
883-
//
884-
// Note that checking the memory value of the futex is valid to do: we
885-
// could easily have been delayed (relative to the worker holding on
886-
// to futex A), which means we could be starting all of our work at the
887-
// later time when there is no need to block. The only "odd" thing is
888-
// that we may have caused side effects in that "delay" time. But the
889-
// only side effects we can have are to call
890-
// emscripten_main_thread_process_queued_calls(). That is always ok to
891-
// do on the main thread (it's why it is ok for us to call it in the
892-
// middle of this function, and elsewhere). So if we check the value
893-
// here and return, it's the same is if what happened on the main thread
894-
// was the same as calling emscripten_main_thread_process_queued_calls()
895-
// a few times times before calling emscripten_futex_wait().
896-
if (Atomics.load(HEAP32, addr >> 2) != val) {
897-
return -{{{ cDefine('EWOULDBLOCK') }}};
898-
}
899-
900-
// Mark us as waiting once more, and continue the loop.
901-
lastAddr = Atomics.exchange(HEAP32, __emscripten_main_thread_futex >> 2, addr);
902-
#if ASSERTIONS
903-
assert(lastAddr == 0);
904-
#endif
905842
}
906843
return 0;
907844
},

0 commit comments

Comments
 (0)