Simplifying synchronization primitives

[kprotty]

With the merge of #9070 it gives us the opportunity to base all the synchronization primitives around it. The existence of {Auto|Static}ResetEvent types felt unnecessary and was thinking it should be changed. Have been debating on what exactly to change it to but couldn't think of any good replacements. Decided to just go with the synchronization primitives offered by posix (pthread) since those are generally well known and can be used to implement other stuff.

We should also consider whether to keep the Held abstraction (as discussed in #8637, cc @cescude @jcmoyer ) while this is here. I was originally on the side of removing it, but after trying to realized that it's alotta code to change and in most instances, the code needed is often shorter compared to not having Held. I also added Held to RwLock and made it have only one release() function - costs a load() to differentiate but reduces the opportunity to call the wrong release().

Any thoughts? @andrewrk

[kprotty]

Thinking of replacing Condvar with Notify from Rust Tokio. Removes the need to hold a lock for waiting, handles broadcast(), and the corresponding signal() handles missed wakeup by storing a semaphore-like token/permit. I'm really liking the idea as it sits between the api of Condvar and the efficiency of Event.

[kprotty]

Ive implemented it using Futex here and it looks very efficient at first glace.

[kprotty]

Will continue on this once the fate of #9175 has been decided

[kprotty]

I've been making progress on this in the meantime. Bringing them in here

[andrewrk]

Looks like this PR has conflicts with master branch and it is not passing the CI tests. Is this waiting on a review or something else?

[kprotty]

This was put on hold for a bit while the CI was down. I can start back up work on this tomorrow or the day after. Most of the sync primitives are done here, but once RwLock tests are added it will be "ready for review".

[andrewrk]

Held is removed. Mind rebasing this PR and getting it ready for review & merge? The CI is also in a better place, good enough to get this green I think.

[LemonBoy]

I strongly oppose to this change, making the function a parameter rather than an initialization-time value makes the usage more complex for no good reason.

If memory bloat is your concern here make f a comptime-known field.

[kprotty]

Thinking of making this comptime f: anytype, args: anytype to mimic Thread.spawn, the @call builtin, and general flexibility.

Passing the function at the call site mimics POSIX (which these primitives are based on) pthread_once + supports context based initialization if people want that. Along with POSIX, this is also the API taken by Rust std::sync::Once and C++ std::once_flag.

Not sure how complexity is increased in use case. Mind throwing out an example?

[LemonBoy]

Thinking of making this comptime f: anytype, args: anytype to mimic Thread.spawn, the @call builtin, and general flexibility.

Tried in #9028 and got some unexpected backslash, hopefully it'll go better this time.

Passing the function at the call site mimics POSIX (which these primitives are based on) pthread_once + supports context based initialization if people want that.

POSIX is not a good example of API design.

Along with POSIX, this is also the API taken by Rust std::sync::Once and C++ std::once_flag.

But not by Go's sync.Once.

Not sure how complexity is increased in use case.

The caller now has to think of the function to call and ensure all the callsites use the same one. Do you have any use case for "hot swapping" functions?

[kprotty]

Tried in #9028 and got some unexpected backslash

Read through the issue a bit. Seems like it's a valid use case? I'm not really too strong on keeping or removing args as a feature of Once.

POSIX is not a good example of API design.

POSIX in general is not good I agree (e.g. condvar being only flexible blocking primitive requires holding a lock), but passing the function at call site doesn't seem like additional cognitive overhead (I elaborate below)

Do you have any use case for "hot swapping" functions?

Frankly, not really. The Once use cases that I can come up with are in a single invocation spot and all call the same function. Such cases do include saving context of the call like if the function failed. Multi function/argument on same Once instance was supported by OS so my thought was "why not support it too".

Reasoning in the "practical" sense of Once usage, I'm having trouble imagining its use case at multiple call sites. It's generally just used in one function and multiple things call that function - no further "thinking" like you described.

Do you have different usages? @LemonBoy

P.S. starting to think the sync primitives shouldn't be designed around pthreads. Maybe just only:

• Lock for non-atomic-type mutual exclusion + rwlock functionality
• Signal (an event counts) for blocking on generically (can be used to impl pthread_cond_t)
• Semaphore blocking for producer/consumer workloads + notification allowed to free the datastructure (unlike Signal)
• Once for lazy init

but it's probably too late at this point.

[LemonBoy]

Read through the issue a bit. Seems like it's a valid use case? I'm not really too strong on keeping or removing args as a feature of Once.

Definitely a valid use case in my book, the main argument against that was that adding the arguments to the callsite made Once easier to misuse. I, for one, would love to have this.

POSIX in general is not good I agree (e.g. condvar being only flexible blocking primitive requires holding a lock), but passing the function at call site doesn't seem like additional cognitive overhead (I elaborate below)

Since we have an easy way of capturing the function pointer to call at compile-time and given the lack of use-cases for having swappable functions (we don't have closures, that's IMO why other languages kept the pthread-like behaviour) I'd push for a general simplification of the API, keeping the fn parameter in the constructor.

The mental overhead I was talking about shows when you have a single Once cell that has multiple call sites, keeping all of them in sync requires you to audit every single one (been there, done that. Had to track down some silly bug caused by me calling pthread_once with different functions).

But after thinking about this for a while I'm retiring my "strong" opposition, if the new interface ends up being a problem we can always switch back to the previous one.

but it's probably too late at this point.

Nah, that's the beauty of a pre 1.0 language :^)

[andrewrk]

CI checks failing, conflicts with master branch, no recent activity. Please open a new PR against master branch to continue this effort.