In Go sync.Map why this part of implementation is inconsistent or do I misunderstand something?

Question

sync.Map is a concurrent-safe map implementation. the type of raw maps in sync.Map actually is map[any]*entry.

When we call Map.LoadOrStore and the entry exists, entry.tryLoadOrStore is invoked and the following is the code of this function

func (e *entry) tryLoadOrStore(i any) (actual any, loaded, ok bool) {
    p := e.p.Load()
    if p == expunged {
        return nil, false, false
    }
    if p != nil {
        return *p, true, true
    }

    // Copy the interface after the first load to make this method more amenable
    // to escape analysis: if we hit the "load" path or the entry is expunged, we
    // shouldn't bother heap-allocating.
    ic := i
    for {
        if e.p.CompareAndSwap(nil, &ic) {
            return i, false, true
        }
        p = e.p.Load()
        if p == expunged {
            return nil, false, false
        }
        if p != nil {
            return *p, true, true
        }
    }
}

And this is another function trySwap, when we call Swap or Store, this function is also invoked.

func (e *entry) trySwap(i *any) (*any, bool) {
    for {
        p := e.p.Load()
        if p == expunged {
            return nil, false
        }
        if e.p.CompareAndSwap(p, i) {
            return p, true
        }
    }
}

tryLoadOrStore can be implemented like trySwap just based on its logic but it doesn't. Here is my question: since their logic is similar, why they are not implemented in same way?

when I try to understand, I think its because the difference of parameter type, if I is *any, it doesn't need to do a copy since it's already a pointer and we does not need to care about the escape analysis. But there seems to be no special reason to get address from the outer caller.

    if e, ok := read.m[key]; ok {
        if v, ok := e.trySwap(&value); ok {
            if v == nil {
                return nil, false
            }
            return *v, true
        }
    }

Then I have no idea why this two functions (and other functions) are implemented in different way.

Answer 1

Frirst, a quote from Bryan Mills, the original author of sync.Map:

sync.Map is pretty gnarly to begin with!

The code in sync.Map is very sensitive to escape analysis, and the implementation is driven by benchmark.

Let's dig into the commit history. It should help us understand why they're implemented in different way.

The initial implementation in CL 37342:

1. The draft implementations in the first patchset are similar:

func (e *entry) tryStore(i interface{}) bool {
    for {
        p := atomic.LoadPointer(&e.p)
        if p == expunged {
            return false
        }
        if atomic.CompareAndSwapPointer(&e.p, p, unsafe.Pointer(&i)) {
            return true
        }
    }
}

func (e *entry) tryLoadOrStore(i interface{}) (actual interface{}, loaded, clean bool) {
    for {
        p := atomic.LoadPointer(&e.p)
        if p == expunged {
            return nil, false, false
        }
        if p != nil {
            return *(*interface{})(p), true, true
        }
        if atomic.CompareAndSwapPointer(&e.p, nil, unsafe.Pointer(&i)) {
            return i, false, true
        }
    }
}

2. The interface-copying trick was added to both implementation in patchset 3:

// Copy the interface to make this method more amenable to escape analysis:
// if we hit the "load" path or the entry is expunged, we shouldn't bother
// heap-allocating.
ic := i

3. (*entry).tryStore is modified to accept a pointer in patchset 5:

I can not find a comment on this change. It's most likely a result of the escape analysis and the benchmark tests.

func (e *entry) tryStore(i *interface{}) bool {
    p := atomic.LoadPointer(&e.p)
    if p == expunged {
        return false
    }
    for {
        if atomic.CompareAndSwapPointer(&e.p, p, unsafe.Pointer(i)) {
            return true
        }
        p = atomic.LoadPointer(&e.p)
        if p == expunged {
            return false
        }
    }
}

func (e *entry) tryLoadOrStore(i interface{}) (actual interface{}, loaded, ok bool) {
    p := atomic.LoadPointer(&e.p)
    if p == expunged {
        return nil, false, false
    }
    if p != nil {
        return *(*interface{})(p), true, true
    }

    // Copy the interface after the first load to make this method more amenable
    // to escape analysis: if we hit the "load" path or the entry is expunged, we
    // shouldn't bother heap-allocating.
    ic := i
    for {
        if atomic.CompareAndSwapPointer(&e.p, nil, unsafe.Pointer(&ic)) {
            return i, false, true
        }
        p = atomic.LoadPointer(&e.p)
        if p == expunged {
            return nil, false, false
        }
        if p != nil {
            return *(*interface{})(p), true, true
        }
    }
}

(*entry).tryStore was simplified in CL 137441:

The change prevents this escape to heap:

sync/map.go:178:26: &e.p escapes to heap
sync/map.go:178:26:     from &e.p (passed to call[argument escapes]) at

func (e *entry) tryStore(i *interface{}) bool {
    for {
        p := atomic.LoadPointer(&e.p)
        if p == expunged {
            return false
        }
        if atomic.CompareAndSwapPointer(&e.p, p, unsafe.Pointer(i)) {
            return true
        }
    }
}

(*entry).tryStore was renamed to (*entry).trySwap in CL 399094:

func (e *entry) trySwap(i *any) (*any, bool) {
   for {
       p := e.p.Load()
       if p == expunged {
           return nil, false
       }
       if e.p.CompareAndSwap(p, i) {
           return p, true
       }
   }
}

That's all.

Note: some other small CLs are not listed, for example, the CL 426074 that switches the implementation to use atomic.Pointer.