Skip to content

Conversation

kateinoigakukun
Copy link
Member

SWIFT_STDLIB_SINGLE_THREADED_CONCURRENCY just means that the global executor is cooperative, but it doesn't mean that the target platform is always single-threaded. For example, on wasm32-unknown-wasip1-threads, the global executor is cooperative, but users can still set up their own TaskExecutor with multiple threads.

This patch guards the TaskGroup state with a mutex even with a cooperative executor by respecting threading package instead. This change effectively affects only wasm32-unknown-wasip1-threads.

`SWIFT_STDLIB_SINGLE_THREADED_CONCURRENCY` just means that the global
executor is cooperative, but it doesn't mean that the target platform is
always single-threaded. For example, on wasm32-unknown-wasip1-threads,
the global executor is cooperative, but users can still set up their own
TaskExecutor with multiple threads.

This patch guards the `TaskGroup` state with a mutex even with a
cooperative executor by respecting threading package instead. This
change effectively affects only wasm32-unknown-wasip1-threads.
@kateinoigakukun
Copy link
Member Author

@swift-ci smoke test

@kateinoigakukun
Copy link
Member Author

@swift-ci test WebAssembly

@kateinoigakukun kateinoigakukun marked this pull request as ready for review July 6, 2024 11:16
@kateinoigakukun kateinoigakukun requested a review from ktoso as a code owner July 6, 2024 11:16
@MaxDesiatov MaxDesiatov requested review from etcwilde and al45tair July 7, 2024 07:44
@ktoso ktoso added the concurrency Feature: umbrella label for concurrency language features label Jul 8, 2024
@ktoso
Copy link
Contributor

ktoso commented Jul 8, 2024

I added your comment in the PR as a source comment, keeping track of the "why" of those #if is very tricky so we need to document such in source IMHO, pending some other improved refactoring of the runtime that would make this less difficult to manage/understand.

LGTM though, merging

@ktoso ktoso enabled auto-merge July 8, 2024 01:51
@ktoso
Copy link
Contributor

ktoso commented Jul 8, 2024

@kateinoigakukun do we want this on 6.0 as well? If so please prepare a PR and ping me :)

@kateinoigakukun
Copy link
Member Author

Thanks! Yes, we need this in 6.0 too

@kateinoigakukun
Copy link
Member Author

@swift-ci smoke test

@ktoso ktoso merged commit a8f68aa into swiftlang:main Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concurrency Feature: umbrella label for concurrency language features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants