golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.23k stars 17.7k forks source link

proposal: sync: Map LoadOrStore interpret functions for `value` #45414

Closed pjebs closed 3 years ago

pjebs commented 3 years ago
func (m *Map) LoadOrStore(key, value interface{}) (actual interface{}, loaded bool)

The proposal states:

  1. That LoadOrStore accept an optional argument that forces it to interpret value more intelligently.
  2. "More intelligently" is defined as: if value is of type FuncLit with a single return value, then if the key does not exist in the map, it calls value and stores the return value.

Depending on the ambit of Go1 compatibility promise, feel free to mark this as a Go2 proposal.

Alternatively, a new method can be added to Map that has this behaviour built-in.

pjebs commented 3 years ago

Essentially an atomic version of:

    val, exists := map.Load(key)
    if !exists {
        val, _ = map.LoadOrStore(key, func())
    }

    ... = val
ianlancetaylor commented 3 years ago

CC @bcmills

We can't change the existing LoadOrStore method but we could in principle add a new method.

cespare commented 3 years ago

@ianlancetaylor if I understand correctly the proposal is to add a special case where if value interface{} is a func() interface{} (say) then use it as a callback. Now that isn't backward compatible as-is, but if we added a new type type LoadFunc func() interface{} and only looked for values of that named type then it would be.

To me, overloading this API by shoehorning a callback into the value interface{} seems worse than adding a new method.

Either way, though, I think this has a sharp edge. The callback function cannot block any other Map method, including concurrent LoadOrStores (that would be antithetical to sync.Map). (To put it another way, it is not possible to implement that code snippet which includes a user-provided callback atomically without introducing unacceptable blocking.) It follows that the callbacks may be called multiple times concurrently. I think users may find that surprising.

tmthrgd commented 3 years ago

This seems like a clear duplicate of the recently declined #44159.

rsc commented 3 years ago

Duplicate of #44159.