-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Scoped values #50958
Scoped values #50958
Conversation
👍 Very good proposal. I think we should use the name |
Or a |
Looks very neat! A couple of questions:
|
Yeah I remarked to Gabriel earlier that I should have named it ScopedValue.
It's not mutable and that is what I would expect from
Must it? I think there is something possible, but this is essentially global state and sending that across the wire is messy at best.
I think this would mean entering a new dynamic scope everytime we create a task. I intentionally choose a coarser granularity (and this is a useful concept without tasks). I managed to get the overhead downs a fair bit (see https://github.com/vchuravy/ScopedValues.jl for some numbers for a more clever implementation than in this PR.), but a scope creation is still ~70.2 ns and 112 bytes over 4 allocs. |
FWIW, not knowing Java, I don't think I would guess what "scoped value" is, whereas "dynamic variable" is more directly obvious. Sure |
I do think there is some value in using the same name as Java (where this concept is also very new). I think the larger point is that it is actually not a variable, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! I'm super excited for this. Various tweaks and nitpicks; feel free to ignore them if you don't agree.
27ae621
to
0c0c0ef
Compare
The context of this was #48121. I think you're right, in that you don't want to ship global state across the wire. I'm not sure what is a more general solution. |
Note that this is something you can do with Dagger.jl, which uses ContextVariablesX.jl (but I would switch to ScopedValues once they're available). Dagger doesn't do this for all scoped variables, but only for its own variables for which propagating their values makes sense (and this works quite well for the composability of nested task spawning). But I wouldn't say a simple model like what Distributed.jl exposes should propagate anything by default. |
Yeah I think we could definitely implement a distributed dynamic scope on top of this. |
Based on the feedback from triage and some more discussion with @gbaraldi I improved the implementation.
In previous versions each scope contained an immutable dict. This allowed implementing
This saved a pointer lookup, an allocation and a couple of bytes. |
I still dislike the name |
ScopedValues are containers whose observed value depends the current dynamic scope. This implementation is inspired by https://openjdk.org/jeps/446 A scope is introduced with the `scoped` function that takes a lambda to execute within the new scope. The value of a `ScopedVariable` is constant within that scope and can only be set upon introduction of a new scope. Scopes are propagated across tasks boundaries. Variable storage is implemented using a persistent dictionary. Upon scope entry a small amount of data is copied and the unchanged pieces are shared.
912257a
to
98074e0
Compare
ScopedVariables are containers whose observed value depends the current dynamic scope. This implementation is inspired by https://openjdk.org/jeps/446 A scope is introduced with the `scoped` function that takes a lambda to execute within the new scope. The value of a `ScopedValue` is constant within that scope and can only be set upon introduction of a new scope. Scopes are propagated across tasks boundaries. Storage is implemented using a persistent dictionary.
Just came across this. We use Would this PR make |
It's a new Base export, not a keyword. |
Note you should only add a method to |
Looking closer, I export Still, it would have been nice if someone had done a quick |
Ah Base also exports
I tried, but didn't find anything obvious. |
Looks like there are plenty of others: |
Any chance of backporting this to v1.10? I know it's very (very) late in the game - it's just that this is one of those things that might influence the basic approach packages take to certain aspects, and depending on which Julia version might become the next LTS ... |
We don't backport features on principle. Especially not for the reason that 1.10 may be an LTS (LTS decisions are made after an release not before it) That being said ScopedValues.jl is fully functional on 1.8+ and can be used with some care (e.g. you need to use it's logstate method) |
I see it does:
so that it works, also on 1.11. But can't people e.g. define Does it make sure to define in 1.10 at least something like: with() = error("install ScopedValues.jl or upgrade to 1.11")? I'm not sure about this new functionality, it seems very important, though clearly optional since we've managed without until now. I'm thinking we want all code that works for LTS, assuming 1.10 will become that, to work in future versions. Of course we can't backport everything, but there are seemingly justifiable exceptions for LTS (at least eventually). |
So ScopedValues.jl could basically be used as a "Compat" here? |
|
||
const ScopeStorage = Base.PersistentDict{ScopedValue, Any} | ||
|
||
mutable struct Scope |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this mutable? I don't see any place where we actually mutate it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah that's an artifact from an earlier implementation, should be changed or values should be const
Addresses keno review comment in #50958 (comment)
If you adopted an "alternative" to the original RFC, why isn't there a new RFC that even tries to explain what the API is? This is totally unprofessional. |
@mind6 I think the API is documented here: https://docs.julialang.org/en/v1/base/scopedvalues/#scoped-values (Also, #35833 was abandoned for over a year, before it was revived in a new shape here, as far as I know.) |
Alternative proposal to #35833, based on a discussion with @gbaraldi.
ScopedVariables are containers whose observed value depends the current
dynamic scope. This implementation is inspired by https://openjdk.org/jeps/446
A scope is introduced with the
scoped
function that takes a lambda toexecute within the new scope. The value of a
ScopedValue
isconstant within that scope and can only be set upon introduction
of a new scope.
Scopes are propagated across tasks boundaries.
In contrast to #35833 the storage of the per-scope data does not require copies
upon scope entry. This also means that libraries can use scoped variables without
paying for scoped variables introduces in other libraries.
Finding the current value of a
ScopedValue
, involves walking thescope chain upwards and checking if the scoped variable has a value
for the current or one of its parent scopes. This means the cost of
a lookup scales with the depth of the dynamic scoping. This is amortized
by perform some caching.
https://github.com/vchuravy/ScopedValues.jl provides an implementation
reusing the logstate trick from
ContextVariablesX.jl
. We could use itas an implementation from Julia 1.7+ on-wards.