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.

Sign up for GitHub

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

Implement Once type for pthread_once like functionality #55439

Closed
wants to merge 1 commit into from
Closed

Implement Once type for pthread_once like functionality #55439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions base/lock.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,3 +556,59 @@ end
import .Base: Event
export Event
end

const _NOT_CALLED = Ptr{Nothing}(0)
const _STARTED = Ptr{Nothing}(1)
const _DONE = Ptr{Nothing}(2)

"""
Once()

Create a `Once` object that can be used to execute a callback exactly once across all executions of the program.
The callback is executed by calling `call_once!`. This is similar to `pthread_once` in POSIX.

```
const once = Once()
foo() = println("This will be printed exactly once")
call_once!(once, foo)
call_once!(once, foo) # does nothing
```

!!! compat "Julia 1.12"
This functionality requires at least Julia 1.12
"""
mutable struct Once
@atomic done::Ptr{Nothing} # Ptr is used so it resets during precompilation
Copy link
Member

Choose a reason for hiding this comment

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

Off. Does this need to be the semantics? Remember precompilation is an optimization, normally Julia would execute the top-level code in the same session and precompilation is just "in the infinentisimal time window before init".

Now the argument for it resetting during precompilation would be that I am guarding a data structure that is being reset as well, but one might be making the opposing statement as well.

I kinda want to discourage people from running their precompilation workload inside the module while it is still open, rather we should run it outside the module and save the state as if the precompilation workload hasn't run yet (Nathan opened a PR, iirc)

Copy link
Member Author

Choose a reason for hiding this comment

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

So there's two use cases here. One where I think this should just error if hit during precompilation. But you might be running a precompile workload that needs a library to be loaded/initialized and the only thing that would have the right semantics here is Ptr

Copy link
Member

Choose a reason for hiding this comment

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

I think one might reasonably want both behavior Once and OncePtr. The challenge is that currently we don't know what the user is referring to. This kinda makes me wonder if we want this to be a ref to the data produces by F

Copy link
Member

Choose a reason for hiding this comment

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

So the scenario I am thinking about is:

mutable struct Cache
    once::Once
    data
end

When Cache is being serialized it depends on what data is if I want to reset once or not. If data does not contain foreign pointers then once ought to not be reset.

Precompilation is equivalent to top-level execution in the target process, but we cache the effect. So IMO Once ought to serialized like any other Julia struct and have a reset function that can be used to reset it at the end of precompilation if it is guarding a foreign pointer.

Copy link
Member

Choose a reason for hiding this comment

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

+1 that this is the really semantically trick part

When Cache is being serialized it depends on what data is if I want to reset once or not. If data does not contain foreign pointers then once ought to not be reset.

I think the rule is more complicated than that... In many cases Once is intended to replace __init__ functionality, in which case you might have cache sizes, local library paths, etc. which are selected - not just foreign pointers

None of those are supposed to affect the outcome of the computations at pre-compilation (even though they are generally different than what you'd compute with at run-time) so they are "sound" to compute twice, but you have to re-compute them again after pre-compilation or they're no longer valid.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah. I think have a reset! for Once would make sense so that the user can choose their behavior. It is IMO easier to explain what to do when your once computation needs to be to reset due to being run during precompilation, than to tell someone "it ran twice since we didn't think you would notice the first time"

But that is also the reason why __init__ is a separate thing.

Copy link
Member

@topolarity topolarity Aug 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm... Yeah, that's reasonable

Although, where would you put the reset! for a sysimage workflow? It'd have to go after all of your dependers in the sysimage, right? It's also a bit awkward that we already do some pretty terrible mutation tearing, since in the pkgimage workflow you get an automatic "reset" from us dropping mutations on the floor if you are run during pre-compilation for another package.

Oh, and the sysimage workflow is unfortunately more important now than before, since it's required for #55047.

It'd be ideal for Once to support both workflows, esp. since __init__ really does not (it's very easy to call dependent code during a sysimage build that is not initialized).

lock::Threads.Event #Ideally would be a Futex
Copy link
Member

Choose a reason for hiding this comment

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

Seems like this should be a Condition variable, since being an Event is redundant with @done

Suggested change
lock::Threads.Event #Ideally would be a Futex
lock::Threads.Condition

Copy link
Member Author

@gbaraldi gbaraldi Aug 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Does Condition actually do what we want here? Event is here serving just as something for threads to sleep in if they enter the slow path. Specifically I want something that if somehow enters the slow path after the function has happened to go right through it.

Copy link
Member

Choose a reason for hiding this comment

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

You need to call it with @lock cond while !once.done; wait(cond); end

Copy link
Member Author

Choose a reason for hiding this comment

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

Right. But does it allow anyone to go through once it's being notified? Because that's the behaviour I want that Event has.

Once() = new(_NOT_CALLED, Threads.Event(false))
end

"""
call_once!(once::Once, f::Function)

Execute the callback `f` if it's the first time `call_once!` is called on this `Once` object.

!!! compat "Julia 1.12"
This functionality requires at least Julia 1.12
"""
@inline function call_once!(once::Once, f::F) where {F <: Function} #TODO: Do we want this specialization
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@inline function call_once!(once::Once, f::F) where {F <: Function} #TODO: Do we want this specialization
@inline function call_once!(f::F, once::Once) where {F}

if (@atomic :acquire once.done) == _DONE #Make sure this is the right atomic
return
end
call_once_slow_path!(once, f)
return
end

@noinline function call_once_slow_path!(once::Once, f::F) where {F <: Function}
if (@atomicreplace :acquire_release once.done _NOT_CALLED => _STARTED).success #TODO: Split into slow path function
try
f()
finally #What if we error
@atomic :release once.done = _DONE
notify(once.lock)
end
else
wait(once.lock)
end
return
end
18 changes: 18 additions & 0 deletions test/threads.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,3 +366,21 @@ end
@test jl_setaffinity(0, mask, cpumasksize) == 0
end
end

@testset "Once" begin
mutable struct MyRandomCounter
@atomic x::Int
end
const cnt = MyRandomCounter(0)
const once = Once()
Copy link
Member

@vtjnash vtjnash Aug 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Some API sketch ideas, via macros:

function foo()
    @once global hasrun1 = true
    @lazycall :per_thread global hasrun2 = true
    @assert hasrun && hasrun2
end

Or via global objects:

const startup1 = LazyCall(:onlyonce) do # errors if runs during precompile
    global hasrun1 = true
end
const startup2 = LazyCall(:concurrently) do # may run on all threads concurrently, as long as none have finished
    startup()
    global hasrun2
    @atomic @__MODULE__.hasrun2 = true
end
const startup3 = LazyCall(:per_process) do # runs once per process, reset on serialization
    startup2()
    global hasrun3 = true
end
global hasrun4 = Lockable(Dict{Int,Bool}())
const startup4 = LazyCall(:per_thread, () -> @lock hasrun4 empty!(hasrun4[])) do tid # runs once per thread, runs the reset on serialization
    startup3()
    @lock hasrun4 hasrun4[][tid]  = true
end
global hasrun5 = Lockable(WeakRefDict{Task,Bool}())
const startup5 = LazyCall(:per_task) do
    startup3()
    @lock hasrun5 hasrun5[][current_task()]  = true
    finalize(current_task()) do t
        @lock hasrun5 pop!(hasrun5[], t)
    end
    nothing
end
function foo()
    startup4()
    @assert hasrun && hasrun2 && hasrun3
    @assert @lock hasrun4 hasrun4[][Threads.threadid()]
    @assert @lock hasrun5 hasrun5[][current_task()]
end

Very minimal implementation sketch:

const Lock = ReentrantLock
mutable struct LazyCall
    @atomic hasrun::Ptr{Cvoid}
    const allow_compile_time::Bool
    const initializer
    const lock::Lock
    LazyCall(initializer; allow_compile_time::Bool) = new(C_NULL, allow_compile_time, initializer, Lock())
end
function (once::LazyCall)()
    (@atomic :acquire once.hasrun) != C_NULL && return
    (@noinline function()
        Base.__precompile__(once.allow_compile_time)
        @lock once.lock begin
            (@atomic :acquire once.hasrun) != C_NULL && return
            once.initializer()
            (@atomic :release once.hasrun = C_NULL + 1)
            nothing
        end
     end)()
     nothing
end

function foo()
sleep(1)
@atomic :monotonic cnt.x += 1
end
@sync begin
Threads.@spawn call_once!(once, foo)
Threads.@spawn call_once!(once, foo) # does nothing
end
@test cnt.x == 1

end