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

make two globals in loading.jl into ScopedValues #56378

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KristofferC
Copy link
Member

Seems less brittle to have this be ScopedValues when there is work going on for parallel loading etc.

If we go with this, this also needs to be updated:

https://github.com/JuliaLang/Distributed.jl/blob/729ba6aa45a9c3e1d492b53ecb9f6190f05f6428/src/Distributed.jl#L81

@KristofferC KristofferC added the packages Package management and loading label Oct 29, 2024
@KristofferC KristofferC requested a review from vtjnash October 29, 2024 08:33
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

This appears to be technically a minor breaking change, since loading is supposed to work correctly with Tasks as well, while this would cause loading to run incorrectly if any loading code spawned a Task to do some of the work in parallel

@Keno
Copy link
Member

Keno commented Oct 29, 2024

This seems strictly better in that case, since at least the value that the task sees is well defined rather than randomly switching off at a non-deterministic point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages Package management and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants