diff --git a/export_test.go b/export_test.go index 6bfcc82..ebd63c9 100644 --- a/export_test.go +++ b/export_test.go @@ -29,3 +29,11 @@ func StoreTopHash(hash, topHashes uint64, idx int) uint64 { func EraseTopHash(topHashes uint64, idx int) uint64 { return eraseTopHash(topHashes, idx) } + +func EnableAssertions() { + assertionsEnabled = true +} + +func DisableAssertions() { + assertionsEnabled = false +} diff --git a/map.go b/map.go index a2c2074..f75fea4 100644 --- a/map.go +++ b/map.go @@ -87,11 +87,6 @@ type rangeEntry struct { value unsafe.Pointer } -// special type to mark nil values -type nilValue struct{} - -var nilVal = new(nilValue) - // NewMap creates a new Map instance. func NewMap() *Map { m := &Map{} @@ -169,9 +164,6 @@ func (m *Map) LoadAndStore(key string, value interface{}) (actual interface{}, l } func (m *Map) doStore(key string, value interface{}, loadIfExists bool) (interface{}, bool) { - if value == nil { - value = nilVal - } // Read-only path. if loadIfExists { if v, ok := m.Load(key); ok { @@ -222,7 +214,11 @@ func (m *Map) doStore(key string, value interface{}, loadIfExists bool) (interfa // interface{} on each call, thus the live value pointers are // unique. Otherwise atomic snapshot won't be correct in case // of multiple Store calls using the same value. - atomic.StorePointer(&b.values[i], unsafe.Pointer(&value)) + nvp := unsafe.Pointer(&value) + if assertionsEnabled && vp == nvp { + panic("non-unique value pointer") + } + atomic.StorePointer(&b.values[i], nvp) b.mu.Unlock() return derefValue(vp), true } @@ -477,11 +473,7 @@ func derefKey(keyPtr unsafe.Pointer) string { } func derefValue(valuePtr unsafe.Pointer) interface{} { - value := *(*interface{})(valuePtr) - if _, ok := value.(*nilValue); ok { - return nil - } - return value + return *(*interface{})(valuePtr) } func bucketIdx(table *mapTable, hash uint64) uint64 { diff --git a/map_test.go b/map_test.go index 30e7ca8..4195936 100644 --- a/map_test.go +++ b/map_test.go @@ -50,6 +50,61 @@ func TestMap_BucketStructSize(t *testing.T) { } } +func TestMap_UniqueValuePointers_Int(t *testing.T) { + EnableAssertions() + m := NewMap() + v := 42 + m.Store("foo", v) + m.Store("foo", v) + DisableAssertions() +} + +func TestMap_UniqueValuePointers_Struct(t *testing.T) { + type foo struct{} + EnableAssertions() + m := NewMap() + v := foo{} + m.Store("foo", v) + m.Store("foo", v) + DisableAssertions() +} + +func TestMap_UniqueValuePointers_Pointer(t *testing.T) { + type foo struct{} + EnableAssertions() + m := NewMap() + v := &foo{} + m.Store("foo", v) + m.Store("foo", v) + DisableAssertions() +} + +func TestMap_UniqueValuePointers_Slice(t *testing.T) { + EnableAssertions() + m := NewMap() + v := make([]int, 13) + m.Store("foo", v) + m.Store("foo", v) + DisableAssertions() +} + +func TestMap_UniqueValuePointers_String(t *testing.T) { + EnableAssertions() + m := NewMap() + v := "bar" + m.Store("foo", v) + m.Store("foo", v) + DisableAssertions() +} + +func TestMap_UniqueValuePointers_Nil(t *testing.T) { + EnableAssertions() + m := NewMap() + m.Store("foo", nil) + m.Store("foo", nil) + DisableAssertions() +} + func TestMap_MissingEntry(t *testing.T) { m := NewMap() v, ok := m.Load("foo") diff --git a/mapof.go b/mapof.go index dfa58b6..6427f3f 100644 --- a/mapof.go +++ b/mapof.go @@ -146,11 +146,16 @@ func (m *MapOf[V]) doStore(key string, value V, loadIfExists bool) (V, bool) { b.mu.Unlock() return derefTypedValue[V](vp), true } - // In-place update case. Luckily we get a copy of the value + // In-place update case. We get a copy of the value via an // interface{} on each call, thus the live value pointers are // unique. Otherwise atomic snapshot won't be correct in case // of multiple Store calls using the same value. - atomic.StorePointer(&b.values[i], unsafe.Pointer(&value)) + var wv interface{} = value + nvp := unsafe.Pointer(&wv) + if assertionsEnabled && vp == nvp { + panic("non-unique value pointer") + } + atomic.StorePointer(&b.values[i], nvp) b.mu.Unlock() return derefTypedValue[V](vp), true } @@ -159,7 +164,8 @@ func (m *MapOf[V]) doStore(key string, value V, loadIfExists bool) (V, bool) { // Insertion case. First we update the value, then the key. // This is important for atomic snapshot states. atomic.StoreUint64(&b.topHashes, storeTopHash(hash, b.topHashes, emptyidx)) - atomic.StorePointer(emptyvp, unsafe.Pointer(&value)) + var wv interface{} = value + atomic.StorePointer(emptyvp, unsafe.Pointer(&wv)) atomic.StorePointer(emptykp, unsafe.Pointer(&key)) b.mu.Unlock() addSize(table, bidx, 1) @@ -336,7 +342,7 @@ func (m *MapOf[V]) Size() int { } func derefTypedValue[V any](valuePtr unsafe.Pointer) (val V) { - return *(*V)(valuePtr) + return (*(*interface{})(valuePtr)).(V) } // O(N) operation; use for debug purposes only diff --git a/mapof_test.go b/mapof_test.go index 86f2d9a..2d3b9ed 100644 --- a/mapof_test.go +++ b/mapof_test.go @@ -12,6 +12,61 @@ import ( . "github.com/puzpuzpuz/xsync" ) +func TestMapOf_UniqueValuePointers_Int(t *testing.T) { + EnableAssertions() + m := NewMapOf[int]() + v := 42 + m.Store("foo", v) + m.Store("foo", v) + DisableAssertions() +} + +func TestMapOf_UniqueValuePointers_Struct(t *testing.T) { + type foo struct{} + EnableAssertions() + m := NewMapOf[foo]() + v := foo{} + m.Store("foo", v) + m.Store("foo", v) + DisableAssertions() +} + +func TestMapOf_UniqueValuePointers_Pointer(t *testing.T) { + type foo struct{} + EnableAssertions() + m := NewMapOf[*foo]() + v := &foo{} + m.Store("foo", v) + m.Store("foo", v) + DisableAssertions() +} + +func TestMapOf_UniqueValuePointers_Slice(t *testing.T) { + EnableAssertions() + m := NewMapOf[[]int]() + v := make([]int, 13) + m.Store("foo", v) + m.Store("foo", v) + DisableAssertions() +} + +func TestMapOf_UniqueValuePointers_String(t *testing.T) { + EnableAssertions() + m := NewMapOf[string]() + v := "bar" + m.Store("foo", v) + m.Store("foo", v) + DisableAssertions() +} + +func TestMapOf_UniqueValuePointers_Nil(t *testing.T) { + EnableAssertions() + m := NewMapOf[*struct{}]() + m.Store("foo", nil) + m.Store("foo", nil) + DisableAssertions() +} + func TestMapOf_MissingEntry(t *testing.T) { m := NewMapOf[string]() v, ok := m.Load("foo") diff --git a/util.go b/util.go index 693ff73..3145262 100644 --- a/util.go +++ b/util.go @@ -5,6 +5,9 @@ import ( "unsafe" ) +// test-only assert()-like flag +var assertionsEnabled = false + const ( // used in paddings to prevent false sharing; // 64B are used instead of 128B as a compromise between