Skip to content

Remove loop from emscripten_thread_sleep. NFC#26585

Merged
sbc100 merged 1 commit intoemscripten-core:mainfrom
sbc100:emscripten_thread_sleep
Apr 3, 2026
Merged

Remove loop from emscripten_thread_sleep. NFC#26585
sbc100 merged 1 commit intoemscripten-core:mainfrom
sbc100:emscripten_thread_sleep

Conversation

@sbc100
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 commented Mar 31, 2026

We have code in the lower level emscrpeten_futex_wait that takes care of splitting up the wait for the main runtime thread.

For normal pthreads I don't see any need to run the message queue constantly while the thread is sleeping.

As part of this change I found that I had to improve the accuracy of the timeslicing emscripten_futex_wait.
The new code uses an absolute target time, so regardless of how much time it spends between calls to
atomic.wait it still returns at the correct time.

Previously we were assuming that we could call atomic.wait32 with 1ms timeout N times to achieve Nms of sleeping, but this doesn't always hold, for various reasons.

Followup to #26471

@sbc100 sbc100 force-pushed the emscripten_thread_sleep branch 10 times, most recently from 86bb623 to 47ee417 Compare April 1, 2026 19:25
@sbc100 sbc100 requested review from dschuff, kripken and tlively April 1, 2026 20:25
We have code in the lower level emscrpeten_futex_wait that take care
of splitting up the wait for the main runtime thread.  For normal
pthreads I don't see any need to run the message queue while the thread
is sleeping like this.

Instead, it can run from the event loop, or explicitly run.

Followup to emscripten-core#26471
@sbc100 sbc100 force-pushed the emscripten_thread_sleep branch from 47ee417 to 6911619 Compare April 1, 2026 20:54
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.

What is the intent of the changes in this file?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

See the final paragraph on the PR description. Basically: To make the total time slept for within this function more accurate.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Maybe I'll split that part of the change out actually..

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Actually I think this makes sense to keep as part of this PR.

Basically I've removing the outer loops which means the inner loops now needs to be more accruate. I'll add more to the PR description.

@sbc100 sbc100 requested review from dschuff and tlively April 2, 2026 21:57
@sbc100 sbc100 merged commit 01b61e4 into emscripten-core:main Apr 3, 2026
38 checks passed
@sbc100 sbc100 deleted the emscripten_thread_sleep branch April 3, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants