This seems like a complete non-starter to me. You are completely replacing the API with a different API, one that requires additional per-operation allocations, when the current API seems just fine, all with very little justification as to why the new API is more compelling.

If all you need is Swap and CompareAndSwap, it seems to me that those are possible to implement without completely rewriting the internals.

And the few cases that truly need type mutability can always store a *interface{} into the atomic.Value.

Hi @rsc,

I think I responded to most of your criticisms in the proposal. Would you mind including more technical detail about what specifically you don't like?

As a side note: I randomly looked through some usages of atomic.*Pointer in codesearch. In pretty much every case, using atomic.Value (with my proposed change) would have been more safe and equally as fast.

Expanding my earlier response:

1. Operations in sync/atomic must not require memory allocation.
   It matters a lot that Go operations, especially very low-level ones like these,
   give programmers control over what allocates and what does not.
   It matters that code written to not allocate today should not start allocating tomorrow.
2. Type mutability in atomic.Value seems useless/borderline harmful to me.
   If we'd had generics then atomic.Value would be an atomic.Value[T].
   Code in which a single atomic.Value changes its underlying type seems to me
   more complex and difficult to understand than code in which an atomic.Value
   has one specific (hopefully documented) underlying type.
3. The forced memory allocation in Value.Store etc is being done to support type mutability.
   Given the previous two points, this is not the right tradeoff.
4. CompareAndSwap and Swap can be implemented in the current code,
   without introducing type mutability and forcing memory allocation.
   If there is a compelling motivation for adding them, then let's do that by itself.

Operations in sync/atomic must not require memory allocation.
It matters a lot that Go operations, especially very low-level ones like these,
give programmers control over what allocates and what does not.
It matters that code written to not allocate today should not start allocating tomorrow.

It does matter I agree, which is why there is a clear and accessible way to avoid allocations already: atomic.*Pointer operations. atomic.Value should be about type safety. I don't understand why allocations are not allowed. I expected you to say something along the lines of "it might crash if allocations aren't allowed" or "it would be a surprising slowdown", but you didn't. Both of these are countered in the proposal, so I don't know what other reason there could be.

Type mutability in atomic.Value seems useless/borderline harmful to me.
If we'd had generics then atomic.Value would be an atomic.Value[T].
Code in which a single atomic.Value changes its underlying type seems to me
more complex and difficult to understand than code in which an atomic.Value
has one specific (hopefully documented) underlying type.

Again, I offer that atomic.Value is a parallel of interface{}, which does allow nil and type mutability. Additionally, other languages permit the equivalent type mutability in their atomics, which is useful. I'd like to call on your experience in other languages as reason why it's useful.

As a brief example, consider an atomic.Value which stores the last error set. error is an interface, and it would be cumbersome to have to assert that all errors stored in the value had identical types. That forces users to create a wrapper type for no reason.

CompareAndSwap and Swap can be implemented in the current code,
without introducing type mutability and forcing memory allocation.
If there is a compelling motivation for adding them, then let's do that by itself.

Continuing with the atomic.Value which stores an error: Try making an atomic.Value hold the first error, and ignore the rest. value.CompareAndSwap(nil, err) is tricky because there was never a previous store of nil. Additionally, if multiple CompareAndSwap race, it may be true that only one wins, but they may have had different types. I called out why all these changes are needed in a single proposal in the very first sentence.

By adopting this proposal, I don't think there is an example where code would be more bug prone, nor would the performance become measurably worse.

If we do CompareAndSwap and Swap, they will be subject to the same type-immutability constraints as Store. Racing CompareAndSwap is no different than racing Store.

Using an atomic.Value to store an error is a mistake. If that's what motivates all this angst, store a *error (or a *interface{}) instead. Yes, it's a little clumsy, because the API as written (even your new API) drops the interface wrapping, but that's a property of interface-based general APIs, not this one. It doesn't merit workarounds here.

Right now atomic.Value is suitable for users who want performance and type safety. The bad idea that if you want performance (no unexpected allocations) then you have to give up type safety is exactly what we were trying to address by adding atomic.Value in the first place. Under no circumstance should we be telling users "this code worked for you before, but not anymore, and oh by the way the fix is to introduce new uses of unsafe into your code."

If we do CompareAndSwap and Swap, they will be subject to the same type-immutability constraints as Store. Racing CompareAndSwap is no different than racing Store.

Agree.

Using an atomic.Value to store an error is a mistake. If that's what motivates all this angst, store a *error (or a *interface{}) instead.

This was the first example I found in our code that used Atomics, not a special case.

but that's a property of interface-based general APIs, not this one. It doesn't merit workarounds here.

Not sure how you can interpret it as not interface based. The only two methods of Value are interface{} based. The only thing that distinguishes it from the rest of the atomics is that it accepts interface{}. Calling the proposed methods as "workarounds" ignores the clear shortcomings in the API.

Right now atomic.Value is suitable for users who want performance and type safety.

Let's take a look at another usage of atomics and see if they are satisfied by the current API: sync.Map. This implementation is basically the same as my proposal, except that it doesn't use Value at all, and does use Pointer. It boxes the interface value, and uses an additional sentinel variable expunged to get around not being able to use nil. If this proposal is accepted, sync.Map could take advantage of Value.

Here is another example of Value's implementation warts appearing on the API, forcing an unnecessary cast:

cc.retryThrottler.Store((*retryThrottler)(nil))

These weren't even that hard to find. Look through atomic\.(Value|.*Pointer) on codesearch or godoc, and you'll see numerous examples where Value underperforms. Lot's of comments on Value's API. Lot's of places where unsafe.Pointer is used due to not being able to use nil.

Under no circumstance should we be telling users "this code worked for you before, but not anymore, and oh by the way the fix is to introduce new uses of unsafe into your code."

Could you clarify what you mean by "worked"? The proposal was specifically chosen to be backwards compatible with existing usage.

Add a CompareAndSwap method to Value

See #11260.

Add a Swap method to Value.

See #20164.

If this proposal is accepted, sync.Map could take advantage of Value.

sync.Map, being in the standard library, should ideally be more efficient than using individual Values. (That's #21031.)

Note @rsc's comment in particular:

If we'd had generics then atomic.Value would be an atomic.Value[T].

I think this proposal should wait until we figure out what (if anything) is happening with generics (#15292).

If we can add a type-parametric version of atomic.Value, that would probably address the awkwardness around nil and around changing concrete types: a typed atomic.Value should allow the concrete type to change if (and only if) the new type is assignable to the Value's type parameter.

Narrowing this proposal to "add Swap and CompareAndSwap", not changing the rest of the API.

Can anyone make a strong case for adding Swap and CompareAndSwap?

Can anyone make a strong case for adding Swap and CompareAndSwap?

AppEngine code cannot use unsafe pointers, but it can use atomic.Value, so having Swap & CompareAndSwap in atomic.Value would be functionality that is otherwise inaccessible to some users.

It seems to me that environments where unsafe pointers are disallowed is the only real value of atomic.Value. atomic.Value is slower than unsafe pointer atomics, has less functionality, and the type information provided by it is mostly unnecessary, since its type can never change after initialization.

There aren't many environments that reject unsafe anymore, and we shouldn't contort the library for those.

It would be fine for someone who has a compelling use for these to create a new issue proposing to add Swap and CompareAndSwap (or to reopen this one, if you have permission). For now, given the lack of a real-world use case for them, we will err on the side of leaving it out.