I was super confused at first that the exact oposite change would have an advantage. But after looking more closely it adds both the fast path and the atomic operation in the same PR. What I'm suggesting is doing the fast path change without the atomic operation, just do it non atomic (maybe with a boolean like it was before).

Related question, are there docs somewhere on how to run these benchmarks myself?

@JelteF

What I'm suggesting is doing the fast path change without the atomic operation, just do it non atomic (maybe with a boolean like it was before).

that would potentially introduce a data race

Related question, are there docs somewhere on how to run these benchmarks myself?

These are standard Go testing benchmarks.

The atomic load is required. Without it, the second caller of sync.Once may not see all of the modifications done by the first call to sync.Once.

@randall77 I believe in this case stale reads are OK since the slow path actually has to relock and recheck anyway. This is not to say that there isn't a race there, but I believe on modern platforms with support for hardware atomic read / write with 32bit memory this would be impossible.

@JelteF I think you'd need some sort of atomic.LoadUint32RelaxedMemoryOrdering() operation if you wanted this to be race free... on all platforms.

Oh wow, and also need to have correct alignment for atomic operations.

I think the added complexity isn't worth it, it isn't the go way.

@skabbes: You're right that the atomic load is not required for the slow path. But the atomic load is required for the fast path, when it reads a 1 and immediately returns. There needs to be some synchronization so that the second+ callers of Once.Do can see all the writes that f() did in the first call to Once.Do.

Completely didn't think about that as an API guarantee, but it definitely is. Kudos.

Thank you for the discussion. I’m going to close this as there is no obvious change required.

If you wish to continue to discuss this, please see https://golang.org/wiki/Questions for good places to ask. Thanks.

Thanks for the feedback everyone. I learnt something new about dataraces, namely that they are possible even when the read and assignment is guarded by a mutex. I checked out moving the atomic load to the slow instead of the fast path and the race detector was still complaining. Even with your comments it's still not clear to me why that would not be a valid solution.

@JelteF let's take this off the issue tracker, please see https://golang.org/wiki/Questions for good places to ask. Thanks.