Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add docs on task migration #50047

Merged

Conversation

IanButterworth
Copy link
Sponsor Member

@IanButterworth IanButterworth commented Jun 3, 2023

#40715 introduced task migration (in 1.7)
#43496 removed pre-1.7 docs that said task migration didn't happen

It seems important-enough and surprising-enough to explicitly document.

@IanButterworth IanButterworth added docs This change adds or pertains to documentation multithreading Base.Threads and related functionality labels Jun 3, 2023
@IanButterworth IanButterworth added the backport 1.9 Change should be backported to release-1.9 label Jun 3, 2023
@jishnub
Copy link
Contributor

jishnub commented Jun 3, 2023

Would it be a good idea to suggest an alternative to indexing using threadid, such as using Channels or locks? This doesn't seem to be well known

@IanButterworth
Copy link
Sponsor Member Author

Would it be a good idea to suggest an alternative to indexing using threadid

Yeah. There's a bit of a concerted effort going on currently to help clarify behavior and guidance on that. One part is #48542. Another is a guide post that's being drafted.

[`@threads`](@ref Threads.@threads)), it may move to a different thread if the task yields.

This means that [`threadid()`](@ref Threads.threadid) should not be treated as constant within a task, and therefore
should not be used to index into a vector of buffers or stateful objects.
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Shall we say "Instead, prefer making judicious use of task_local_storage(), or pass values around as explicit arguments."

Copy link
Contributor

Choose a reason for hiding this comment

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

I think what we should advise is that people use @spawn because @spawned tasks can define and return values.

That's a lot nicer as an API than having users trying to reason about shared buffers, or them trying to reach into and keep track of task_local_storage()s

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

We could keep this PR as is then figure out the recommended patterns in #48542?

@IanButterworth IanButterworth merged commit ff23b37 into JuliaLang:master Jun 4, 2023
@IanButterworth IanButterworth deleted the ib/task_migration_docs branch June 4, 2023 01:27
IanButterworth added a commit to IanButterworth/julia that referenced this pull request Jun 4, 2023
@KristofferC KristofferC mentioned this pull request Jun 6, 2023
36 tasks
KristofferC pushed a commit that referenced this pull request Jun 27, 2023
(cherry picked from commit ff23b37)
KristofferC pushed a commit that referenced this pull request Jun 27, 2023
(cherry picked from commit ff23b37)
KristofferC pushed a commit that referenced this pull request Jun 27, 2023
(cherry picked from commit ff23b37)
@KristofferC KristofferC removed the backport 1.9 Change should be backported to release-1.9 label Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation multithreading Base.Threads and related functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants