Skip to content

Allow emscripten_atomic_wait_suspending to return synchronously#26947

Open
sbc100 wants to merge 1 commit into
emscripten-core:mainfrom
sbc100:emscripten_atomic_wait_suspending2
Open

Allow emscripten_atomic_wait_suspending to return synchronously#26947
sbc100 wants to merge 1 commit into
emscripten-core:mainfrom
sbc100:emscripten_atomic_wait_suspending2

Conversation

@sbc100
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 commented May 14, 2026

Followup to the initial version landed in #26941. This version of the code allows for reporting of synchronous results without actually suspending.

The implementation of emscripten_atomic_wait_suspending is not in libc, and it works by first calling a non-JSPI function which can return a promise, or a sync result. Then, only in the case that a promise is returned do we actually need to suspend using a JSPI call.

@sbc100 sbc100 requested a review from tlively May 14, 2026 03:44
Comment thread system/lib/pthread/emscripten_atomic_wait_suspending.c Outdated
@sbc100 sbc100 force-pushed the emscripten_atomic_wait_suspending2 branch 2 times, most recently from aece5ac to 3f54471 Compare May 14, 2026 15:37
Comment thread src/lib/libatomic.js Outdated
@sbc100 sbc100 force-pushed the emscripten_atomic_wait_suspending2 branch 4 times, most recently from f2b3b85 to 8868476 Compare May 14, 2026 17:38
@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented May 14, 2026

I have a nice followup to which I think makes this solution nicer: f5cc8ee

@tlively
Copy link
Copy Markdown
Member

tlively commented May 14, 2026

Sounds reasonable.

@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented May 14, 2026

@tlively Do you think this approach makes sense then? i.e. the version where we return a promise and only suspend sometimes?

If so, are you OK landing this change? (with the "await_unchecked") as a separate followup?

@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented May 14, 2026

Actually maybe lets just land #26954 first

sbc100 added a commit that referenced this pull request May 15, 2026
This works with ASYNCIFY like the existing `emscripten_promise_await`
but is a lot simpler since it only handles the fulfilled case.

In this case we can simple return the result directly without needing to
allocate a `em_promise_result_t` struct to deal with the out param (i.e.
no memory access needed).

This can be useful in cases where we don't want to handle the rejections
case.

Split out from #26947
@sbc100 sbc100 force-pushed the emscripten_atomic_wait_suspending2 branch from 8868476 to c5a41e9 Compare May 15, 2026 00:43
@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented May 15, 2026

OK, this is now rebase on top of #26954. PTAL!

@sbc100 sbc100 force-pushed the emscripten_atomic_wait_suspending2 branch from c5a41e9 to 5e69766 Compare May 15, 2026 00:48
@sbc100 sbc100 force-pushed the emscripten_atomic_wait_suspending2 branch from 5e69766 to 9cd0cdb Compare May 15, 2026 03:09
@sbc100 sbc100 enabled auto-merge (squash) May 15, 2026 05:25
Copy link
Copy Markdown
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

Nice!

void _emscripten_thread_notify(pthread_t thread);

// Internal promise-returning API used to implement
// emscripten_atomic_wait_syspending.
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.

Suggested change
// emscripten_atomic_wait_syspending.
// emscripten_atomic_wait_suspending.

'MAX_WEBGL_VERSION': 0,
'BUILD_AS_WORKER': 1,
'LINK_AS_CXX': 1,
'SHARED_MEMORY': 0,
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 this?

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.

2 participants