Skip to content

Multithreading 26/N: emscripten_thread_sleep - #6195

Merged
juj merged 1 commit into
emscripten-core:incomingfrom
juj:emscripten_thread_sleep
Nov 2, 2018
Merged

Multithreading 26/N: emscripten_thread_sleep#6195
juj merged 1 commit into
emscripten-core:incomingfrom
juj:emscripten_thread_sleep

Conversation

@juj

@juj juj commented Feb 5, 2018

Copy link
Copy Markdown
Collaborator

Expose internal do_sleep() as a public emscripten_thread_sleep() function.

This function is exposed to the main thread as well, since in some lock free structures the only thing one can do is to spinwait for access, even on the main thread, and doing a emscripten_thread_sleep(0) inside such loops can improve responsiveness.

In a worker thread, this function will save CPU power, since it relies on Atomics.wait to do the waiting.

@juj

juj commented Feb 13, 2018

Copy link
Copy Markdown
Collaborator Author

Oops, looking at the failures, this PR actually will depend on PR #6201 to land first, since emscripten_thread_sleep() depends on emscripten_current_thread_process_queued_calls() function that is landing in that PR. So good to defer looking at this until that one is resolved first.


__pthread_testcancel(); // pthreads spec: sleep is a cancellation point, so must test if this thread is cancelled during the sleep.
emscripten_current_thread_process_queued_calls();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to self: this is defined in #6201 (Multithreading 29/N)

@juj
juj force-pushed the emscripten_thread_sleep branch from 2a19975 to cd21b10 Compare July 21, 2018 17:21

@kripken kripken left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, but there is a risk of confusion with emscripten_sleep which is used for async code, https://github.com/kripken/emscripten/blob/20602efb955a7c6c20865a495932427e205651d2/src/library_async.js#L61

Please document the difference in threading.h.

@juj
juj force-pushed the emscripten_thread_sleep branch from cd21b10 to d9d75d9 Compare November 1, 2018 15:24
@juj

juj commented Nov 1, 2018

Copy link
Copy Markdown
Collaborator Author

Good point - added a note and rebased to latest.

@juj
juj force-pushed the emscripten_thread_sleep branch from d9d75d9 to 640a8bf Compare November 1, 2018 15:26
@juj
juj force-pushed the emscripten_thread_sleep branch from 640a8bf to d79256d Compare November 1, 2018 15:28
@juj
juj merged commit 55ee5c3 into emscripten-core:incoming Nov 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants