From ece93bf87d0050509583f512bf7ccee76b9a827f Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Tue, 13 Dec 2022 20:33:58 +0800 Subject: [PATCH] On delete, always set promotions == -2 and node == nil Also, item.promotions doesn't need to be loaded/stored using atomic. Once upon a time it did. Cache was updated long ago to not use atomic operations on it, but LayeredCache wasn't. They are both consistent now (they don't use atomic operations). Fixes: https://github.com/karlseguin/ccache/issues/76 --- cache.go | 2 ++ cache_test.go | 2 +- layeredcache.go | 8 +++++--- layeredcache_test.go | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/cache.go b/cache.go index d8a929f..f27b075 100644 --- a/cache.go +++ b/cache.go @@ -394,6 +394,8 @@ func (c *Cache[T]) doDelete(item *Item[T]) { c.onDelete(item) } c.list.Remove(item.node) + item.node = nil + item.promotions = -2 } } diff --git a/cache_test.go b/cache_test.go index f8cc06c..c7ba85f 100644 --- a/cache_test.go +++ b/cache_test.go @@ -332,7 +332,7 @@ func Test_CachePrune(t *testing.T) { cache.Set(key, key, 5*time.Minute) } if epoch%500 == 0 { - assert.True(t, cache.GetSize() < 500) + assert.True(t, cache.GetSize() <= 500) } } } diff --git a/layeredcache.go b/layeredcache.go index b6798dd..e6a632b 100644 --- a/layeredcache.go +++ b/layeredcache.go @@ -266,13 +266,15 @@ func (c *LayeredCache[T]) worker() { } deleteItem := func(item *Item[T]) { if item.node == nil { - atomic.StoreInt32(&item.promotions, -2) + item.promotions = -2 } else { c.size -= item.size if c.onDelete != nil { c.onDelete(item) } c.list.Remove(item.node) + item.node = nil + item.promotions = -2 } } for { @@ -318,13 +320,13 @@ func (c *LayeredCache[T]) worker() { func (c *LayeredCache[T]) doPromote(item *Item[T]) bool { // deleted before it ever got promoted - if atomic.LoadInt32(&item.promotions) == -2 { + if item.promotions == -2 { return false } if item.node != nil { //not a new item if item.shouldPromote(c.getsPerPromote) { c.list.MoveToFront(item.node) - atomic.StoreInt32(&item.promotions, 0) + item.promotions = 0 } return false } diff --git a/layeredcache_test.go b/layeredcache_test.go index adf5483..f8b3e2f 100644 --- a/layeredcache_test.go +++ b/layeredcache_test.go @@ -391,7 +391,7 @@ func Test_LayeredCachePrune(t *testing.T) { cache.Set(key, key, key, 5*time.Minute) } if epoch%500 == 0 { - assert.True(t, cache.GetSize() < 500) + assert.True(t, cache.GetSize() <= 500) } } }