Add a convenience object for expressing once-like / per-runtime patterns #55793

vtjnash · 2024-09-17T14:17:40Z

This adds 3 new types, to conveniently express 3 common concurrent code patterns:

PerProcess: an action that must be taken once per process
PerThread: an action that must be taken once per thread id
PerTask: an action that must be take once per task object

The PerProcess object should replace __init__ or similar hand rolled implementations of this.
The PerThread object should replace code that used to use nthreads() to implement a much less correct version of this (though this is not recommended in most new code, some foreign libraries may need this to interact well with C).
The PerTask object is simply a thin wrapper over task_local_storage().

For example, the jldoctest demonstrates how to lazily allocate a shared constant vector unique to the process:

julia> const global_state = Base.PerProcess{Vector{Int}}() do
           println("Making lazy global value...done.")
           return [rand()]
       end;

julia> a = global_state();
Making lazy global value...done.

julia> a === global_state()
true

julia> a === fetch(@async global_state())
true

base/lock.jl

IanButterworth · 2024-09-17T16:52:53Z

base/exports.jl

+    PerProcess,
+    PerTask,
+    PerThread,


Bikeshedding in a thread..
Something seemed off about Per* on first impression. I think I was expecting there to be a n somewhere i.e. not specifically tied to once.

What about ProcessSingleton / ThreadSingleton / TaskSingleton ?

Java used *Local naming convention for this construct, so in this context, that would be: ProcessLocal, ThreadLocal, TaskLocal

:-)

In the condext of a do block, I think OncePerProcess is great.

"Once per process do function body" if a fairly accurate description of what this does.

To help paint the bikeshed, I've come across a word that I think bears mentioning: "Hapax". It's probably a bit to esoteric to seriously consider, but it apparently refers to something (usually a word or expression) that occurs only once within a given context.

That seems like a pretty excellent semantic fit to me 😀

Other than that, OncePerProcess, ProcessSingleton, or ProcessInitializer seem like solid options to me.

base/lock.jl

PerProcess: once per process PerThread: once per thread id PerTask: once per task object

Somewhat generalizes our support for changing Ptr to C_NULL. Not particularly fast, since it is just using the builtins implementation of setfield, and delaying the actual stores, but it should suffice.

Prior to this, especially on macOS, the gc-safepoint here would cause the process to segfault as we had already freed the current_task state. Rearrange this code so that the GC interactions (except for the atomic store to current_task) are all handled before entering GC safe, and then signaling the thread is deleted (via setting current_task = NULL, published by jl_unlock_profile_wr to other threads) is last. ``` ERROR: Exception handler triggered on unmanaged thread. Process 53827 stopped * thread #5, stop reason = EXC_BAD_ACCESS (code=2, address=0x100018008) frame #0: 0x0000000100b74344 libjulia-internal.1.12.0.dylib`jl_delete_thread [inlined] jl_gc_state_set(ptls=0x000000011f8b3200, state='\x02', old_state=<unavailable>) at julia_threads.h:272:9 [opt] 269 assert(old_state != JL_GC_CONCURRENT_COLLECTOR_THREAD); 270 jl_atomic_store_release(&ptls->gc_state, state); 271 if (state == JL_GC_STATE_UNSAFE || old_state == JL_GC_STATE_UNSAFE) -> 272 jl_gc_safepoint_(ptls); 273 return old_state; 274 } 275 STATIC_INLINE int8_t jl_gc_state_save_and_set(jl_ptls_t ptls, Target 0: (julia) stopped. (lldb) up frame #1: 0x0000000100b74320 libjulia-internal.1.12.0.dylib`jl_delete_thread [inlined] jl_gc_state_save_and_set(ptls=0x000000011f8b3200, state='\x02') at julia_threads.h:278:12 [opt] 275 STATIC_INLINE int8_t jl_gc_state_save_and_set(jl_ptls_t ptls, 276 int8_t state) 277 { -> 278 return jl_gc_state_set(ptls, state, jl_atomic_load_relaxed(&ptls->gc_state)); 279 } 280 #ifdef __clang_gcanalyzer__ 281 // these might not be a safepoint (if they are no-op safe=>safe transitions), but we have to assume it could be (statically) (lldb) frame #2: 0x0000000100b7431c libjulia-internal.1.12.0.dylib`jl_delete_thread(value=0x000000011f8b3200) at threading.c:537:11 [opt] 534 ptls->root_task = NULL; 535 jl_free_thread_gc_state(ptls); 536 // then park in safe-region -> 537 (void)jl_gc_safe_enter(ptls); 538 } ```

Address reviewer feedback, add more fixes and more tests

StefanKarpinski · 2024-09-26T13:41:19Z

base/lock.jl

@@ -500,7 +507,7 @@ Create a level-triggered event source. Tasks that call [`wait`](@ref) on an
 After `notify` is called, the `Event` remains in a signaled state and
 tasks will no longer block when waiting for it, until `reset` is called.

-If `autoreset` is true, at most one task will be released from `wait` for
+If `autoreset` is true, at most one task will be released from `wait` for)


LilithHafner · 2024-09-26T13:41:36Z

base/lock.jl

@@ -500,7 +507,7 @@ Create a level-triggered event source. Tasks that call [`wait`](@ref) on an
 After `notify` is called, the `Event` remains in a signaled state and
 tasks will no longer block when waiting for it, until `reset` is called.

-If `autoreset` is true, at most one task will be released from `wait` for
+If `autoreset` is true, at most one task will be released from `wait` for)


Suggested change

If `autoreset` is true, at most one task will be released from `wait` for)

If `autoreset` is true, at most one task will be released from `wait` for

LilithHafner · 2024-09-26T13:45:57Z

base/exports.jl

@@ -70,6 +70,9 @@ export
    OrdinalRange,
    Pair,
    PartialQuickSort,
+    OncePerProcess,
+    OncePerTask,
+    OncePerThread,


This should probably not be exported?

*This = OncePerThread only. And still public, just not exported.

LilithHafner · 2024-09-26T13:49:25Z

base/lock.jl

+Warning: it is not necessarily true that a Task only runs on one thread, therefore the value
+returned here may alias other values or change in the middle of your program. This type may
+get deprecated in the future. If initializer yields, the thread running the current task
+after the call might not be the same as the one at the start of the call.
+
+See also: [`OncePerTask`](@ref).


Perhaps lead with this warning and the admonition to almost always use OncePerTask instead.

Should probably also be a !!! warning

This type may get deprecated in the future.

Should it be under the Experimental module namespace?

LilithHafner · 2024-09-26T13:52:58Z

base/lock.jl

+    end
+    return xs[tid]
+end
+@inline (once::OncePerThread)() = once[Threads.threadid()]


Why not once(threadid)?

function once::OncePerThread(threadid=Threads.threadid()) ... end

LilithHafner · 2024-09-26T13:57:12Z

base/lock.jl

+julia> const global_state = Base.OncePerProcess{Vector{UInt32}}() do
+           println("Making lazy global value...done.")
+           return [Libc.rand()]
+       end;


Suggested change

end;

end

...

Showing the return type here may be helpful. That will show us that global_state isa Base.OncePerProcess.

LilithHafner · 2024-09-26T13:59:20Z

base/lock.jl

+```
+"""
+mutable struct OncePerProcess{T, F}
+    x::Union{Nothing,T}


Suggested change

x::Union{Nothing,T}

value::Union{Nothing,T}

LilithHafner · 2024-09-26T14:01:12Z

base/lock.jl

+julia> procstate = global_state();
+Making lazy global value...done.


Suggested change

julia> procstate = global_state();

Making lazy global value...done.

julia> procstate = global_state()

Making lazy global value...done.

[0x97b92da8]

LilithHafner · 2024-09-26T14:07:05Z

Everyone at triage was happy with OncePerProcess/OncePerTask, etc.
We had hopes of finding something shorter that was jus as descriptive but didn't find anything better.
We also had hopes of PerProcess, but given the extra clarity of Once and that other languages us the name Once, it seems prudent to keep OncePerProcess.

StefanKarpinski

Some comments from triage.

StefanKarpinski · 2024-09-26T13:45:15Z

base/lock.jl

+const PerStateConcurrent    = 0x03
+
+"""
+    OncePerProcess{T}


Suggested change

OncePerProcess{T}

OncePerProcess{T}(init::Function)

OncePerProcess{T}(init::Function)() -> T

This seems more suggestive of how this is actually used?

StefanKarpinski · 2024-09-26T13:51:14Z

base/lock.jl

+    end
+    return xs[tid]
+end
+@inline (once::OncePerThread)() = once[Threads.threadid()]


I would put this above since it's the primary API.

StefanKarpinski · 2024-09-26T13:52:04Z

base/lock.jl

+end
+OncePerThread{T}(initializer::F) where {T, F} = OncePerThread{T,F}(initializer)
+OncePerThread(initializer) = OncePerThread{Base.promote_op(initializer), typeof(initializer)}(initializer)
+@inline function getindex(once::OncePerThread, tid::Integer)


Calling, this getindex might be a bit too cute? Could just be an optional argument to the call method, i.e.:

function (once::OncePerThread)(tid::Integer = Threads.threadid())

StefanKarpinski · 2024-09-26T13:58:27Z

base/lock.jl

+```
+"""
+mutable struct OncePerProcess{T, F}
+    x::Union{Nothing,T}


Suggested change

x::Union{Nothing,T}

value::Union{Nothing,T}

Calling this value would be clearer.

StefanKarpinski · 2024-09-26T13:59:21Z

base/lock.jl

+           return [Libc.rand()]
+       end;
+
+julia> procstate = global_state();


Showing the value here would be helpful. Will cause problems with the doctest, but generating random values is kind of a bad example anyway. Maybe we can brainstorm a more meaningful example...

Would have been helpful (to me at least), to make clear that global_state returns a OncePerProcess{T} object and calling that object returns a T object.

vtjnash added parallelism Parallel or distributed computation don't squash Don't squash merge labels Sep 17, 2024

vtjnash requested a review from gbaraldi September 17, 2024 14:17

vtjnash force-pushed the jn/once branch from 3354e1c to ed7511a Compare September 17, 2024 15:43

KristofferC reviewed Sep 17, 2024

View reviewed changes

base/lock.jl Outdated Show resolved Hide resolved

IanButterworth reviewed Sep 17, 2024

View reviewed changes

topolarity reviewed Sep 18, 2024

View reviewed changes

base/lock.jl Outdated Show resolved Hide resolved

vtjnash added 4 commits September 23, 2024 16:54

add objects to concisely specify initialization

ccf1c8d

PerProcess: once per process PerThread: once per thread id PerTask: once per task object

add precompile support for recording fields to change

3876d43

Somewhat generalizes our support for changing Ptr to C_NULL. Not particularly fast, since it is just using the builtins implementation of setfield, and delaying the actual stores, but it should suffice.

improve OncePer implementation

539d600

Address reviewer feedback, add more fixes and more tests

vtjnash force-pushed the jn/once branch from ed7511a to 539d600 Compare September 23, 2024 16:55

fredrikekre mentioned this pull request Sep 23, 2024

Add TaskLocalValue(initializer) constructor vchuravy/TaskLocalValues.jl#6

Merged

rename: add Once

6099c23

simonbyrne mentioned this pull request Sep 25, 2024

add macro to create custom Ops also on aarch64 JuliaParallel/MPI.jl#871

Merged

StefanKarpinski reviewed Sep 26, 2024

View reviewed changes

LilithHafner reviewed Sep 26, 2024

View reviewed changes

StefanKarpinski reviewed Sep 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a convenience object for expressing once-like / per-runtime patterns #55793

Add a convenience object for expressing once-like / per-runtime patterns #55793

vtjnash commented Sep 17, 2024

IanButterworth Sep 17, 2024

grandinj Sep 18, 2024

LilithHafner Sep 24, 2024

tecosaur Sep 25, 2024

StefanKarpinski Sep 26, 2024

LilithHafner Sep 26, 2024

LilithHafner Sep 26, 2024

LilithHafner Sep 26, 2024

LilithHafner Sep 26, 2024

IanButterworth Sep 26, 2024

nickrobinson251 Oct 3, 2024

LilithHafner Sep 26, 2024 •

edited

Loading

LilithHafner Sep 26, 2024

LilithHafner Sep 26, 2024

LilithHafner Sep 26, 2024

LilithHafner commented Sep 26, 2024

StefanKarpinski left a comment

StefanKarpinski Sep 26, 2024

StefanKarpinski Sep 26, 2024

StefanKarpinski Sep 26, 2024

StefanKarpinski Sep 26, 2024

StefanKarpinski Sep 26, 2024

	If `autoreset` is true, at most one task will be released from `wait` for)
	If `autoreset` is true, at most one task will be released from `wait` for

		julia> procstate = global_state();
		Making lazy global value...done.

	OncePerProcess{T}
	OncePerProcess{T}(init::Function)
	OncePerProcess{T}(init::Function)() -> T

Add a convenience object for expressing once-like / per-runtime patterns #55793

Are you sure you want to change the base?

Add a convenience object for expressing once-like / per-runtime patterns #55793

Conversation

vtjnash commented Sep 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LilithHafner Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LilithHafner commented Sep 26, 2024

StefanKarpinski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LilithHafner Sep 26, 2024 •

edited

Loading