grab lookup len under read lock

bucket.go

@@ -23,9 +23,9 @@ func (b *bucket) get(key string) *Item {
 	return b.lookup[key]
 }
 
-func (b *bucket) set(key string, value interface{}, duration time.Duration, track bool) (*Item, *Item) {
+func (b *bucket) set(key string, value interface{}, duration time.Duration) (*Item, *Item) {
 	expires := time.Now().Add(duration).UnixNano()
-	item := newItem(key, value, expires, track)
+	item := newItem(key, value, expires)
 	b.Lock()
 	existing := b.lookup[key]
 	b.lookup[key] = item
@@ -54,9 +54,14 @@ func (b *bucket) delete(key string) *Item {
 // the item from the map. I'm pretty sure this is 100% fine, but it is unique.
 // (We do this so that the write to the channel is under the read lock and not the
 // write lock)
-func (b *bucket) deleteFunc(matches func(key string, item *Item) bool, deletables chan *Item) int {
+func (b *bucket) deleteFunc(matches func(key string, item interface{}) bool, deletables chan *Item) int {
 	lookup := b.lookup
-	items := make([]*Item, 0)
+
+	b.RLock()
+	l := len(lookup)
+	b.RUnlock()
+
+	items := make([]*Item, 0, l/10)
 
 	b.RLock()
 	for key, item := range lookup {
@@ -81,7 +86,7 @@ func (b *bucket) deleteFunc(matches func(key string, item *Item) bool, deletable
 }
 
 func (b *bucket) deletePrefix(prefix string, deletables chan *Item) int {
-	return b.deleteFunc(func(key string, item *Item) bool {
+	return b.deleteFunc(func(key string, item interface{}) bool {
 		return strings.HasPrefix(key, prefix)
 	}, deletables)
 }

bucket_test.go

@@ -1,10 +1,9 @@
 package ccache
 
 import (
+	. "github.com/karlseguin/expect"
 	"testing"
 	"time"
-
-	. "github.com/karlseguin/expect"
 )
 
 type BucketTests struct {
@@ -33,7 +32,7 @@ func (_ *BucketTests) DeleteItemFromBucket() {
 
 func (_ *BucketTests) SetsANewBucketItem() {
 	bucket := testBucket()
-	item, existing := bucket.set("spice", TestValue("flow"), time.Minute, false)
+	item, existing := bucket.set("spice", TestValue("flow"), time.Minute)
 	assertValue(item, "flow")
 	item = bucket.get("spice")
 	assertValue(item, "flow")
@@ -42,7 +41,7 @@ func (_ *BucketTests) SetsANewBucketItem() {
 
 func (_ *BucketTests) SetsAnExistingItem() {
 	bucket := testBucket()
-	item, existing := bucket.set("power", TestValue("9001"), time.Minute, false)
+	item, existing := bucket.set("power", TestValue("9001"), time.Minute)
 	assertValue(item, "9001")
 	item = bucket.get("power")
 	assertValue(item, "9001")

cache.go

@@ -17,10 +17,6 @@ type setMaxSize struct {
 	size int64
 }
 
-type clear struct {
-	done chan struct{}
-}
-
 type Cache struct {
 	*Configuration
 	list        *list.List
@@ -59,10 +55,6 @@ func (c *Cache) ItemCount() int {
 	return count
 }
 
-func (c *Cache) Size() int64 {
-	return c.size
-}
-
 func (c *Cache) DeletePrefix(prefix string) int {
 	count := 0
 	for _, b := range c.buckets {
@@ -72,7 +64,7 @@ func (c *Cache) DeletePrefix(prefix string) int {
 }
 
 // Deletes all items that the matches func evaluates to true.
-func (c *Cache) DeleteFunc(matches func(key string, item *Item) bool) int {
+func (c *Cache) DeleteFunc(matches func(key string, item interface{}) bool) int {
 	count := 0
 	for _, b := range c.buckets {
 		count += b.deleteFunc(matches, c.deletables)
@@ -106,15 +98,9 @@ func (c *Cache) TrackingGet(key string) TrackedItem {
 	return item
 }
 
-// Used when the cache was created with the Track() configuration option.
-// Sets the item, and returns a tracked reference to it.
-func (c *Cache) TrackingSet(key string, value interface{}, duration time.Duration) TrackedItem {
-	return c.set(key, value, duration, true)
-}
-
 // Set the value in the cache for the specified duration
 func (c *Cache) Set(key string, value interface{}, duration time.Duration) {
-	c.set(key, value, duration, false)
+	c.set(key, value, duration)
 }
 
 // Replace the value if it exists, does not set if it doesn't.
@@ -141,7 +127,7 @@ func (c *Cache) Fetch(key string, duration time.Duration, fetch func() (interfac
 	if err != nil {
 		return nil, err
 	}
-	return c.set(key, value, duration, false), nil
+	return c.set(key, value, duration), nil
 }
 
 // Remove the item from the cache, return true if the item was present, false otherwise.
@@ -154,11 +140,13 @@ func (c *Cache) Delete(key string) bool {
 	return false
 }
 
-// Clears the cache
+//this isn't thread safe. It's meant to be called from non-concurrent tests
 func (c *Cache) Clear() {
-	done := make(chan struct{})
-	c.control <- clear{done: done}
-	<-done
+	for _, bucket := range c.buckets {
+		bucket.clear()
+	}
+	c.size = 0
+	c.list = list.New()
 }
 
 // Stops the background worker. Operations performed on the cache after Stop
@@ -194,8 +182,8 @@ func (c *Cache) deleteItem(bucket *bucket, item *Item) {
 	c.deletables <- item
 }
 
-func (c *Cache) set(key string, value interface{}, duration time.Duration, track bool) *Item {
-	item, existing := c.bucket(key).set(key, value, duration, track)
+func (c *Cache) set(key string, value interface{}, duration time.Duration) *Item {
+	item, existing := c.bucket(key).set(key, value, duration)
 	if existing != nil {
 		c.deletables <- existing
 	}
@@ -237,13 +225,6 @@ func (c *Cache) worker() {
 				if c.size > c.maxSize {
 					dropped += c.gc()
 				}
-			case clear:
-				for _, bucket := range c.buckets {
-					bucket.clear()
-				}
-				c.size = 0
-				c.list = list.New()
-				msg.done <- struct{}{}
 			}
 		}
 	}

cache_test.go

@@ -2,7 +2,6 @@ package ccache
 
 import (
 	"strconv"
-	"sync/atomic"
 	"testing"
 	"time"
 
@@ -64,17 +63,17 @@ func (_ CacheTests) DeletesAFunc() {
 	cache.Set("f", 6, time.Minute)
 	Expect(cache.ItemCount()).To.Equal(6)
 
-	Expect(cache.DeleteFunc(func(key string, item *Item) bool {
+	Expect(cache.DeleteFunc(func(key string, item interface{}) bool {
 		return false
 	})).To.Equal(0)
 	Expect(cache.ItemCount()).To.Equal(6)
 
-	Expect(cache.DeleteFunc(func(key string, item *Item) bool {
-		return item.Value().(int) < 4
+	Expect(cache.DeleteFunc(func(key string, item interface{}) bool {
+		return item.(*Item).Value().(int) < 4
 	})).To.Equal(3)
 	Expect(cache.ItemCount()).To.Equal(3)
 
-	Expect(cache.DeleteFunc(func(key string, item *Item) bool {
+	Expect(cache.DeleteFunc(func(key string, item interface{}) bool {
 		return key == "d"
 	})).To.Equal(1)
 	Expect(cache.ItemCount()).To.Equal(2)
@@ -82,10 +81,10 @@ func (_ CacheTests) DeletesAFunc() {
 }
 
 func (_ CacheTests) OnDeleteCallbackCalled() {
-	onDeleteFnCalled := int32(0)
+	onDeleteFnCalled := false
 	onDeleteFn := func(item *Item) {
 		if item.key == "spice" {
-			atomic.AddInt32(&onDeleteFnCalled, 1)
+			onDeleteFnCalled = true
 		}
 	}
 
@@ -99,7 +98,7 @@ func (_ CacheTests) OnDeleteCallbackCalled() {
 
 	Expect(cache.Get("spice")).To.Equal(nil)
 	Expect(cache.Get("worm").Value()).To.Equal("sand")
-	Expect(atomic.LoadInt32(&onDeleteFnCalled)).To.Eql(1)
+	Expect(onDeleteFnCalled).To.Equal(true)
 }
 
 func (_ CacheTests) FetchesExpiredItems() {
@@ -141,21 +140,18 @@ func (_ CacheTests) PromotedItemsDontGetPruned() {
 }
 
 func (_ CacheTests) TrackerDoesNotCleanupHeldInstance() {
-	cache := New(Configure().ItemsToPrune(11).Track())
-	item0 := cache.TrackingSet("0", 0, time.Minute)
-	for i := 1; i < 11; i++ {
+	cache := New(Configure().ItemsToPrune(10).Track())
+	for i := 0; i < 10; i++ {
 		cache.Set(strconv.Itoa(i), i, time.Minute)
 	}
-	item1 := cache.TrackingGet("1")
+	item := cache.TrackingGet("0")
 	time.Sleep(time.Millisecond * 10)
 	gcCache(cache)
 	Expect(cache.Get("0").Value()).To.Equal(0)
-	Expect(cache.Get("1").Value()).To.Equal(1)
-	item0.Release()
-	item1.Release()
+	Expect(cache.Get("1")).To.Equal(nil)
+	item.Release()
 	gcCache(cache)
 	Expect(cache.Get("0")).To.Equal(nil)
-	Expect(cache.Get("1")).To.Equal(nil)
 }
 
 func (_ CacheTests) RemovesOldestItemWhenFull() {

configuration_test.go

@@ -1,9 +1,8 @@
 package ccache
 
 import (
-	"testing"
-
 	. "github.com/karlseguin/expect"
+	"testing"
 )
 
 type ConfigurationTests struct{}

go.mod

@@ -1,8 +1,8 @@
-module github.com/lbryio/ccache/v2
+module github.com/karlseguin/ccache/v2
 
 go 1.13
 
 require (
 	github.com/karlseguin/expect v1.0.2-0.20190806010014-778a5f0c6003
-	github.com/wsxiaoys/terminal v0.0.0-20160513160801-0940f3fc43a0 // indirect
+	github.com/wsxiaoys/terminal v0.0.0-20160513160801-0940f3fc43a0
 )

go.sum

@@ -1,3 +1,5 @@
+github.com/karlseguin/expect v1.0.1 h1:z4wy4npwwHSWKjGWH85WNJO42VQhovxTCZDSzhjo8hY=
+github.com/karlseguin/expect v1.0.1/go.mod h1:zNBxMY8P21owkeogJELCLeHIt+voOSduHYTFUbwRAV8=
 github.com/karlseguin/expect v1.0.2-0.20190806010014-778a5f0c6003 h1:vJ0Snvo+SLMY72r5J4sEfkuE7AFbixEP2qRbEcum/wA=
 github.com/karlseguin/expect v1.0.2-0.20190806010014-778a5f0c6003/go.mod h1:zNBxMY8P21owkeogJELCLeHIt+voOSduHYTFUbwRAV8=
 github.com/wsxiaoys/terminal v0.0.0-20160513160801-0940f3fc43a0 h1:3UeQBvD0TFrlVjOeLOBz+CPAI8dnbqNSVwUwRrkp7vQ=

item.go

@@ -52,22 +52,18 @@ type Item struct {
 	element    *list.Element
 }
 
-func newItem(key string, value interface{}, expires int64, track bool) *Item {
+func newItem(key string, value interface{}, expires int64) *Item {
 	size := int64(1)
 	if sized, ok := value.(Sized); ok {
 		size = sized.Size()
 	}
-	item := &Item{
+	return &Item{
 		key:        key,
 		value:      value,
 		promotions: 0,
 		size:       size,
 		expires:    expires,
 	}
-	if track {
-		item.refCount = 1
-	}
-	return item
 }
 
 func (i *Item) shouldPromote(getsPerPromote int32) bool {

layeredbucket.go

@@ -38,7 +38,7 @@ func (b *layeredBucket) getSecondaryBucket(primary string) *bucket {
 	return bucket
 }
 
-func (b *layeredBucket) set(primary, secondary string, value interface{}, duration time.Duration, track bool) (*Item, *Item) {
+func (b *layeredBucket) set(primary, secondary string, value interface{}, duration time.Duration) (*Item, *Item) {
 	b.Lock()
 	bkt, exists := b.buckets[primary]
 	if exists == false {
@@ -46,7 +46,7 @@ func (b *layeredBucket) set(primary, secondary string, value interface{}, durati
 		b.buckets[primary] = bkt
 	}
 	b.Unlock()
-	item, existing := bkt.set(secondary, value, duration, track)
+	item, existing := bkt.set(secondary, value, duration)
 	item.group = primary
 	return item, existing
 }
@@ -71,7 +71,7 @@ func (b *layeredBucket) deletePrefix(primary, prefix string, deletables chan *It
 	return bucket.deletePrefix(prefix, deletables)
 }
 
-func (b *layeredBucket) deleteFunc(primary string, matches func(key string, item *Item) bool, deletables chan *Item) int {
+func (b *layeredBucket) deleteFunc(primary string, matches func(key string, item interface{}) bool, deletables chan *Item) int {
 	b.RLock()
 	bucket, exists := b.buckets[primary]
 	b.RUnlock()

layeredcache.go

@@ -102,14 +102,9 @@ func (c *LayeredCache) TrackingGet(primary, secondary string) TrackedItem {
 	return item
 }
 
-// Set the value in the cache for the specified duration
-func (c *LayeredCache) TrackingSet(primary, secondary string, value interface{}, duration time.Duration) TrackedItem {
-	return c.set(primary, secondary, value, duration, true)
-}
-
 // Set the value in the cache for the specified duration
 func (c *LayeredCache) Set(primary, secondary string, value interface{}, duration time.Duration) {
-	c.set(primary, secondary, value, duration, false)
+	c.set(primary, secondary, value, duration)
 }
 
 // Replace the value if it exists, does not set if it doesn't.
@@ -136,7 +131,7 @@ func (c *LayeredCache) Fetch(primary, secondary string, duration time.Duration,
 	if err != nil {
 		return nil, err
 	}
-	return c.set(primary, secondary, value, duration, false), nil
+	return c.set(primary, secondary, value, duration), nil
 }
 
 // Remove the item from the cache, return true if the item was present, false otherwise.
@@ -160,15 +155,17 @@ func (c *LayeredCache) DeletePrefix(primary, prefix string) int {
 }
 
 // Deletes all items that share the same primary key and where the matches func evaluates to true.
-func (c *LayeredCache) DeleteFunc(primary string, matches func(key string, item *Item) bool) int {
+func (c *LayeredCache) DeleteFunc(primary string, matches func(key string, item interface{}) bool) int {
 	return c.bucket(primary).deleteFunc(primary, matches, c.deletables)
 }
 
-// Clears the cache
+//this isn't thread safe. It's meant to be called from non-concurrent tests
 func (c *LayeredCache) Clear() {
-	done := make(chan struct{})
-	c.control <- clear{done: done}
-	<-done
+	for _, bucket := range c.buckets {
+		bucket.clear()
+	}
+	c.size = 0
+	c.list = list.New()
 }
 
 func (c *LayeredCache) Stop() {
@@ -196,8 +193,8 @@ func (c *LayeredCache) restart() {
 	go c.worker()
 }
 
-func (c *LayeredCache) set(primary, secondary string, value interface{}, duration time.Duration, track bool) *Item {
-	item, existing := c.bucket(primary).set(primary, secondary, value, duration, track)
+func (c *LayeredCache) set(primary, secondary string, value interface{}, duration time.Duration) *Item {
+	item, existing := c.bucket(primary).set(primary, secondary, value, duration)
 	if existing != nil {
 		c.deletables <- existing
 	}
@@ -247,13 +244,6 @@ func (c *LayeredCache) worker() {
 				if c.size > c.maxSize {
 					dropped += c.gc()
 				}
-			case clear:
-				for _, bucket := range c.buckets {
-					bucket.clear()
-				}
-				c.size = 0
-				c.list = list.New()
-				msg.done <- struct{}{}
 			}
 		}
 	}

layeredcache_test.go

@@ -2,7 +2,6 @@ package ccache
 
 import (
 	"strconv"
-	"sync/atomic"
 	"testing"
 	"time"
 
@@ -108,17 +107,17 @@ func (_ *LayeredCacheTests) DeletesAFunc() {
 	cache.Set("spice", "f", 6, time.Minute)
 	Expect(cache.ItemCount()).To.Equal(6)
 
-	Expect(cache.DeleteFunc("spice", func(key string, item *Item) bool {
+	Expect(cache.DeleteFunc("spice", func(key string, item interface{}) bool {
 		return false
 	})).To.Equal(0)
 	Expect(cache.ItemCount()).To.Equal(6)
 
-	Expect(cache.DeleteFunc("spice", func(key string, item *Item) bool {
-		return item.Value().(int) < 4
+	Expect(cache.DeleteFunc("spice", func(key string, item interface{}) bool {
+		return item.(*Item).Value().(int) < 4
 	})).To.Equal(2)
 	Expect(cache.ItemCount()).To.Equal(4)
 
-	Expect(cache.DeleteFunc("spice", func(key string, item *Item) bool {
+	Expect(cache.DeleteFunc("spice", func(key string, item interface{}) bool {
 		return key == "d"
 	})).To.Equal(1)
 	Expect(cache.ItemCount()).To.Equal(3)
@@ -126,10 +125,12 @@ func (_ *LayeredCacheTests) DeletesAFunc() {
 }
 
 func (_ *LayeredCacheTests) OnDeleteCallbackCalled() {
-	onDeleteFnCalled := int32(0)
+
+	onDeleteFnCalled := false
 	onDeleteFn := func(item *Item) {
+
 		if item.group == "spice" && item.key == "flow" {
-			atomic.AddInt32(&onDeleteFnCalled, 1)
+			onDeleteFnCalled = true
 		}
 	}
 
@@ -147,7 +148,7 @@ func (_ *LayeredCacheTests) OnDeleteCallbackCalled() {
 	Expect(cache.Get("spice", "worm")).To.Equal(nil)
 	Expect(cache.Get("leto", "sister").Value()).To.Equal("ghanima")
 
-	Expect(atomic.LoadInt32(&onDeleteFnCalled)).To.Eql(1)
+	Expect(onDeleteFnCalled).To.Equal(true)
 }
 
 func (_ *LayeredCacheTests) DeletesALayer() {
@@ -195,20 +196,17 @@ func (_ LayeredCacheTests) PromotedItemsDontGetPruned() {
 
 func (_ LayeredCacheTests) TrackerDoesNotCleanupHeldInstance() {
 	cache := Layered(Configure().ItemsToPrune(10).Track())
-	item0 := cache.TrackingSet("0", "a", 0, time.Minute)
-	for i := 1; i < 11; i++ {
+	for i := 0; i < 10; i++ {
 		cache.Set(strconv.Itoa(i), "a", i, time.Minute)
 	}
-	item1 := cache.TrackingGet("1", "a")
+	item := cache.TrackingGet("0", "a")
 	time.Sleep(time.Millisecond * 10)
 	gcLayeredCache(cache)
 	Expect(cache.Get("0", "a").Value()).To.Equal(0)
-	Expect(cache.Get("1", "a").Value()).To.Equal(1)
-	item0.Release()
-	item1.Release()
+	Expect(cache.Get("1", "a")).To.Equal(nil)
+	item.Release()
 	gcLayeredCache(cache)
 	Expect(cache.Get("0", "a")).To.Equal(nil)
-	Expect(cache.Get("1", "a")).To.Equal(nil)
 }
 
 func (_ LayeredCacheTests) RemovesOldestItemWhenFull() {
@@ -262,9 +260,7 @@ func (_ LayeredCacheTests) ResizeOnTheFly() {
 }
 
 func newLayered() *LayeredCache {
-	c := Layered(Configure())
-	c.Clear()
-	return c
+	return Layered(Configure())
 }
 
 func (_ LayeredCacheTests) RemovesOldestItemWhenFullBySizer() {

readme.md

@@ -96,11 +96,8 @@ cache.Delete("user:4")
 ### DeletePrefix
 `DeletePrefix` deletes all keys matching the provided prefix. Returns the number of keys removed.
 
-### DeleteFunc
-`DeleteFunc` deletes all items that the provded matches func evaluates to true. Returns the number of keys removed.
-
 ### Clear
-`Clear` clears the cache. If the cache's gc is running, `Clear` waits for it to finish.
+`Clear` clears the cache. This method is **not** thread safe. It is meant to be used from tests.
 
 ### Extend
 The life of an item can be changed via the `Extend` method. This will change the expiry of the item by the specified duration relative to the current time.
@@ -143,7 +140,7 @@ user := item.Value()   //will be nil if "user:4" didn't exist in the cache
 item.Release()  //can be called even if item.Value() returned nil
 ```
 
-In practice, `Release` wouldn't be called until later, at some other place in your code. `TrackingSet` can be used to set a value to be tracked.
+In practice, `Release` wouldn't be called until later, at some other place in your code.
 
 There's a couple reason to use the tracking mode if other parts of your code also hold references to objects. First, if you're already going to hold a reference to these objects, there's really no reason not to have them in the cache - the memory is used up anyways.
 

secondarycache.go

@@ -16,7 +16,7 @@ func (s *SecondaryCache) Get(secondary string) *Item {
 // Set the secondary key to a value.
 // The semantics are the same as for LayeredCache.Set
 func (s *SecondaryCache) Set(secondary string, value interface{}, duration time.Duration) *Item {
-	item, existing := s.bucket.set(secondary, value, duration, false)
+	item, existing := s.bucket.set(secondary, value, duration)
 	if existing != nil {
 		s.pCache.deletables <- existing
 	}

secondarycache_test.go

@@ -1,11 +1,10 @@
 package ccache
 
 import (
+	. "github.com/karlseguin/expect"
 	"strconv"
 	"testing"
 	"time"
-
-	. "github.com/karlseguin/expect"
 )
 
 type SecondaryCacheTests struct{}