Skip to content

Commit

Permalink
chore: delete ok when getting a node from a hash table
Browse files Browse the repository at this point in the history
  • Loading branch information
maypok86 committed Sep 26, 2024
1 parent f3ec528 commit 90ec9ea
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 52 deletions.
24 changes: 12 additions & 12 deletions cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,38 +170,38 @@ func (c *Cache[K, V]) Has(key K) bool {

// Get returns the value associated with the key in this cache.
func (c *Cache[K, V]) Get(key K) (V, bool) {
n, ok := c.GetNode(key)
if !ok {
n := c.GetNode(key)
if n == nil {
return zeroValue[V](), false
}

return n.Value(), true
}

// GetNode returns the node associated with the key in this cache.
func (c *Cache[K, V]) GetNode(key K) (node.Node[K, V], bool) {
n, ok := c.hashmap.Get(key)
if !ok || !n.IsAlive() || n.HasExpired(c.clock.Offset()) {
func (c *Cache[K, V]) GetNode(key K) node.Node[K, V] {
n := c.hashmap.Get(key)
if n == nil || !n.IsAlive() || n.HasExpired(c.clock.Offset()) {
c.stats.RecordMisses(1)
return nil, false
return nil
}

c.afterHit(n)

return n, true
return n
}

// GetNodeQuietly returns the node associated with the key in this cache.
//
// Unlike GetNode, this function does not produce any side effects
// such as updating statistics or the eviction policy.
func (c *Cache[K, V]) GetNodeQuietly(key K) (node.Node[K, V], bool) {
n, ok := c.hashmap.Get(key)
if !ok || !n.IsAlive() || n.HasExpired(c.clock.Offset()) {
return nil, false
func (c *Cache[K, V]) GetNodeQuietly(key K) node.Node[K, V] {
n := c.hashmap.Get(key)
if n == nil || !n.IsAlive() || n.HasExpired(c.clock.Offset()) {
return nil
}

return n, true
return n
}

func (c *Cache[K, V]) afterHit(got node.Node[K, V]) {
Expand Down
12 changes: 6 additions & 6 deletions extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ func (e Extension[K, V]) createEntry(n node.Node[K, V]) Entry[K, V] {
// Unlike Get in the cache, this function does not produce any side effects
// such as updating statistics or the eviction policy.
func (e Extension[K, V]) GetQuietly(key K) (V, bool) {
n, ok := e.cache.GetNodeQuietly(key)
if !ok {
n := e.cache.GetNodeQuietly(key)
if n == nil {
return zeroValue[V](), false
}

Expand All @@ -65,8 +65,8 @@ func (e Extension[K, V]) GetQuietly(key K) (V, bool) {

// GetEntry returns the cache entry associated with the key in this cache.
func (e Extension[K, V]) GetEntry(key K) (Entry[K, V], bool) {
n, ok := e.cache.GetNode(key)
if !ok {
n := e.cache.GetNode(key)
if n == nil {
return Entry[K, V]{}, false
}

Expand All @@ -78,8 +78,8 @@ func (e Extension[K, V]) GetEntry(key K) (Entry[K, V], bool) {
// Unlike GetEntry, this function does not produce any side effects
// such as updating statistics or the eviction policy.
func (e Extension[K, V]) GetEntryQuietly(key K) (Entry[K, V], bool) {
n, ok := e.cache.GetNodeQuietly(key)
if !ok {
n := e.cache.GetNodeQuietly(key)
if n == nil {
return Entry[K, V]{}, false
}

Expand Down
7 changes: 3 additions & 4 deletions internal/hashmap/map.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,7 @@ func newMapTable[K comparable, V any](minTableLen int, prevHasher maphash.Hasher

// Get returns the node stored in the map for a key, or nil
// if no value is present.
// The ok result indicates whether value was found in the map.
func (m *Map[K, V]) Get(key K) (got node.Node[K, V], ok bool) {
func (m *Map[K, V]) Get(key K) node.Node[K, V] {
table := (*mapTable[K, V])(atomic.LoadPointer(&m.table))
hash := table.hasher.Hash(key)
h1 := h1(hash)
Expand All @@ -200,14 +199,14 @@ func (m *Map[K, V]) Get(key K) (got node.Node[K, V], ok bool) {
if nptr != nil {
n := m.nodeManager.FromPointer(nptr)
if n.Key() == key {
return n, true
return n
}
}
markedw &= markedw - 1
}
bptr := atomic.LoadPointer(&b.next)
if bptr == nil {
return nil, false
return nil
}
b = (*bucketPadded)(bptr)
}
Expand Down
60 changes: 30 additions & 30 deletions internal/hashmap/map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ func TestMap_BucketStructSize(t *testing.T) {
func TestMap_MissingNode(t *testing.T) {
nm := testNodeManager[string, string]()
m := New(nm)
n, ok := m.Get("foo")
if ok {
n := m.Get("foo")
if n != nil {
t.Fatalf("node was not expected: %v", n)
}
var oldNode node.Node[string, string]
Expand All @@ -76,8 +76,8 @@ func TestMapOf_EmptyStringKey(t *testing.T) {
m.Compute("", func(n node.Node[string, string]) node.Node[string, string] {
return nm.Create("", "foobar", 0, 1)
})
n, ok := m.Get("")
if !ok {
n := m.Get("")
if n == nil {
t.Fatal("node was expected")
}
if n.Value() != "foobar" {
Expand All @@ -91,8 +91,8 @@ func TestMapSet_NilValue(t *testing.T) {
m.Compute("foo", func(n node.Node[string, *struct{}]) node.Node[string, *struct{}] {
return nm.Create("foo", nil, 0, 1)
})
n, ok := m.Get("foo")
if !ok {
n := m.Get("foo")
if n == nil {
t.Fatal("nil node was expected")
}
if n.Value() != nil {
Expand Down Expand Up @@ -184,7 +184,7 @@ func TestMapRange_NestedDelete(t *testing.T) {
return true
})
for i := 0; i < numNodes; i++ {
if _, ok := m.Get(strconv.Itoa(i)); ok {
if n := m.Get(strconv.Itoa(i)); n != nil {
t.Fatalf("node found for %d", i)
}
}
Expand All @@ -201,8 +201,8 @@ func TestMapStringSet(t *testing.T) {
})
}
for i := 0; i < numNodes; i++ {
n, ok := m.Get(strconv.Itoa(i))
if !ok {
n := m.Get(strconv.Itoa(i))
if n == nil {
t.Fatalf("node not found for %d", i)
}
if n.Value() != i {
Expand All @@ -221,8 +221,8 @@ func TestMapIntSet(t *testing.T) {
})
}
for i := 0; i < numNodes; i++ {
n, ok := m.Get(i)
if !ok {
n := m.Get(i)
if n == nil {
t.Fatalf("node not found for %d", i)
}
if n.Value() != i {
Expand All @@ -242,8 +242,8 @@ func TestMapSet_StructKeys_IntValues(t *testing.T) {
})
}
for i := 0; i < numNodes; i++ {
n, ok := m.Get(point{int32(i), -int32(i)})
if !ok {
n := m.Get(point{int32(i), -int32(i)})
if n == nil {
t.Fatalf("node not found for %d", i)
}
if n.Value() != i {
Expand All @@ -264,8 +264,8 @@ func TestMapSet_StructKeys_StructValues(t *testing.T) {
})
}
for i := 0; i < numNodes; i++ {
n, ok := m.Get(point{int32(i), -int32(i)})
if !ok {
n := m.Get(point{int32(i), -int32(i)})
if n == nil {
t.Fatalf("node not found for %d", i)
}
v := n.Value()
Expand Down Expand Up @@ -301,8 +301,8 @@ func TestMapSet_WithCollisions(t *testing.T) {
})
}
for i := 0; i < numNodes; i++ {
n, ok := m.Get(i)
if !ok {
n := m.Get(i)
if n == nil {
t.Fatalf("node not found for %d", i)
}
if n.Value() != i {
Expand Down Expand Up @@ -396,7 +396,7 @@ func TestMapStringSetThenDelete(t *testing.T) {
m.Compute(strconv.Itoa(i), func(n node.Node[string, int]) node.Node[string, int] {
return nil
})
if _, ok := m.Get(strconv.Itoa(i)); ok {
if n := m.Get(strconv.Itoa(i)); n != nil {
t.Fatalf("node was not expected for %d", i)
}
}
Expand All @@ -415,7 +415,7 @@ func TestMapIntSetThenDelete(t *testing.T) {
m.Compute(int32(i), func(n node.Node[int32, int32]) node.Node[int32, int32] {
return nil
})
if _, ok := m.Get(int32(i)); ok {
if n := m.Get(int32(i)); n != nil {
t.Fatalf("node was not expected for %d", i)
}
}
Expand All @@ -436,7 +436,7 @@ func TestMapStructSetThenDelete(t *testing.T) {
m.Compute(key, func(n node.Node[point, string]) node.Node[point, string] {
return nil
})
if _, ok := m.Get(key); ok {
if n := m.Get(key); n != nil {
t.Fatalf("node was not expected for %d", i)
}
}
Expand Down Expand Up @@ -587,8 +587,8 @@ func TestMapParallelResize(t *testing.T) {
<-cdone
// Verify map contents.
for i := 0; i < numNodes; i++ {
n, ok := m.Get(strconv.Itoa(i))
if !ok {
n := m.Get(strconv.Itoa(i))
if n == nil {
// The entry may be deleted and that's ok.
continue
}
Expand Down Expand Up @@ -656,8 +656,8 @@ func parallelSeqTypedSetter(t *testing.T, m *Map[string, int], storeEach, numIte
return m.nodeManager.Create(key, j, 0, 1)
})
// Due to atomic snapshots we must see a "<j>"/j pair.
n, ok := m.Get(key)
if !ok {
n := m.Get(key)
if n == nil {
t.Errorf("node was not found for %d", j)
break
}
Expand Down Expand Up @@ -687,8 +687,8 @@ func TestMapParallelSetter(t *testing.T) {
}
// Verify map contents.
for i := 0; i < numNodes; i++ {
n, ok := m.Get(strconv.Itoa(i))
if !ok {
n := m.Get(strconv.Itoa(i))
if n == nil {
t.Fatalf("node not found for %d", i)
}
if n.Value() != i {
Expand Down Expand Up @@ -740,7 +740,7 @@ func parallelTypedGetter(t *testing.T, m *Map[string, int], numIters, numNodes i
for i := 0; i < numIters; i++ {
for j := 0; j < numNodes; j++ {
// Due to atomic snapshots we must either see no entry, or a "<j>"/j pair.
if n, ok := m.Get(strconv.Itoa(j)); ok {
if n := m.Get(strconv.Itoa(j)); n != nil {
if n.Value() != j {
t.Errorf("value was not expected for %d: %d", j, n.Value())
}
Expand Down Expand Up @@ -814,8 +814,8 @@ func TestMapParallelComputes(t *testing.T) {
}
// Verify map contents.
for i := 0; i < numWorkers; i++ {
n, ok := m.Get(uint64(i))
if !ok {
n := m.Get(uint64(i))
if n == nil {
t.Fatalf("node not found for %d", i)
}
if n.Value() != numWorkers*numIters {
Expand Down Expand Up @@ -935,7 +935,7 @@ func parallelTypedUpdater(t *testing.T, m *Map[uint64, *point], idx int, stopFla
t.Errorf("value was present for %d: %v", idx, n.Value())
}
time.Sleep(time.Duration(sleepUs) * time.Microsecond)
if _, ok := m.Get(uint64(idx)); !ok {
if n := m.Get(uint64(idx)); n == nil {
t.Errorf("node was not found for %d", idx)
}
m.Compute(uint64(idx), func(n node.Node[uint64, *point]) node.Node[uint64, *point] {
Expand Down

0 comments on commit 90ec9ea

Please sign in to comment.