Merge pull request #5 from jahnestacado/fix/mutex-read-locks

Fix/mutex read locks
jahnestacado · Oct 4, 2022 · 3be47aa · 3be47aa
2 parents f707ce1 + ec00a04
commit 3be47aa
Show file tree

Hide file tree

Showing 8 changed files with 294 additions and 165 deletions.
diff --git a/.github/workflows/build.yaml → .github/workflows/run-tests.yaml b/.github/workflows/build.yaml → .github/workflows/run-tests.yaml
@@ -1,4 +1,4 @@
-name: Build
+name: Run Tests
 
 on: [ push ]
 
@@ -34,19 +34,3 @@ jobs:
 
     - name: Run vulnerabilities scan
       run: make scan
-
-  tag:
-    runs-on: ubuntu-latest
-    needs: test
-    if: github.ref == 'refs/heads/master'
-    steps:
-    - uses: actions/checkout@v2
-      with:
-        fetch-depth: '0'
-    - name: Bump version and push tag
-      uses: anothrNick/[email protected]
-      env:
-        GITHUB_TOKEN: ${{ secrets.PAT }}
-        WITH_V: true
-        VERBOSE: true
-        DEFAULT_BUMP: patch
diff --git a/.github/workflows/tag-master.yaml b/.github/workflows/tag-master.yaml
@@ -0,0 +1,23 @@
+name: Tag Master branch
+
+on:
+  workflow_run:
+    workflows: Run Tests
+    branches: master
+    types: completed
+
+jobs:
+  tag:
+    runs-on: ubuntu-latest
+    if: ${{ github.event.workflow_run.conclusion == 'success' }}
+    steps:
+    - uses: actions/checkout@v2
+      with:
+        fetch-depth: '0'
+    - name: Bump version and push tag
+      uses: anothrNick/[email protected]
+      env:
+        GITHUB_TOKEN: ${{ secrets.PAT }}
+        WITH_V: true
+        VERBOSE: true
+        DEFAULT_BUMP: patch
diff --git a/README.md b/README.md
@@ -47,7 +47,7 @@ When an entry from the cache is accessed via the `Get` method it is marked as th
 * Behavior upon `Set`
   - If the key entry doesn't exist then it inserts it as the most recently used entry with Counter = 0
   - If the key entry already exists then it will return an error
-  - If the cache is full (Config.Size) then the least recently accessed entry(the node before the tailNode) will be dropped and an EvictedEntry will be emitted to the EvictionChannel(if present) with EvictionReasonDropped
+  - If the cache is full (Config.MaxSize) then the least recently accessed entry(the node before the tailNode) will be dropped and an EvictedEntry will be emitted to the EvictionChannel(if present) with EvictionReasonDropped
 
 #### Example
 
@@ -74,10 +74,11 @@ var (
 func main() {
 	evictionChannel := make(chan tlru.EvictedEntry, 0)
 	config := tlru.Config{
-		Size:            2,
-		TTL:             ttl,
-		EvictionPolicy:  tlru.LRA,
-		EvictionChannel: &evictionChannel,
+		MaxSize:                   2,
+		TTL:                       ttl,
+		EvictionPolicy:            tlru.LRA,
+		EvictionChannel:           &evictionChannel,
+		GarbageCollectionInterval: &ttl,
 	}
 	cache := tlru.New(config)
 
@@ -135,7 +136,7 @@ multiple insertion of entries with the same key.
   - If the key entry doesn't exist then it inserts it as the most recently used entry with Counter = 1
   - If the key entry already exists then it will update the Value, Counter and LastUsedAt properties of
     the existing entry and mark it as the most recently used entry
-  - If the cache is full (Config.Size) then the least recently inserted entry(the node before the tailNode)
+  - If the cache is full (Config.MaxSize) then the least recently inserted entry(the node before the tailNode)
     will be dropped and an EvictedEntry will be emitted to the EvictionChannel(if present) with EvictionReasonDropped
 
 #### Example
@@ -163,10 +164,11 @@ var (
 func main() {
 	evictionChannel := make(chan tlru.EvictedEntry, 0)
 	config := tlru.Config{
-		Size:            3,
-		TTL:             ttl,
-		EvictionPolicy:  tlru.LRI,
-		EvictionChannel: &evictionChannel,
+		MaxSize:                   3,
+		TTL:                       ttl,
+		EvictionPolicy:            tlru.LRI,
+		EvictionChannel:           &evictionChannel,
+		GarbageCollectionInterval: &ttl,
 	}
 	cache := tlru.New(config)
 
@@ -216,7 +218,6 @@ the use of ingestion timestamps
 
 ```go
 config := tlru.Config{
-  Size:            100,
   TTL:             ttl,
   EvictionPolicy:  tlru.LRI,
 }
@@ -275,37 +276,37 @@ goos: darwin
 goarch: amd64
 pkg: github.com/jahnestacado/tlru
 cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
-BenchmarkGet_EmptyCache_LRA-16                        							23577234	    	45.52 ns/op
-BenchmarkGet_EmptyCache_LRI-16                        							27169711	    	45.60 ns/op
-BenchmarkGet_NonExistingKey_LRA-16                      						48853821	    	27.36 ns/op
-BenchmarkGet_NonExistingKey_LRI-16                      						45801547	    	25.17 ns/op
-BenchmarkGet_ExistingKey_LRA-16                        	 						 4631190	    	234.6 ns/op
-BenchmarkGet_ExistingKey_LRI-16                        	 						 5760564	    	182.7 ns/op
-BenchmarkGet_FullCache_1000000_Parallel_LRA-16                						19806835	    	58.29 ns/op
-BenchmarkGet_FullCache_1000000_Parallel_LRI-16                						16803621	    	63.58 ns/op
-BenchmarkGet_FullCache_1000000_WithTinyTTL_Parallel_LRA-16          	 				 9002697	    	165.8 ns/op
-BenchmarkGet_FullCache_1000000_WithTinyTTL_Parallel_LRI-16          	 				 8929759	    	132.2 ns/op
-BenchmarkHas_FullCache_1000000_Parallel-16                  						14102200	    	73.63 ns/op
-BenchmarkSet_LRA-16                              	 						 1387353	    	829.3 ns/op
-BenchmarkSet_LRI-16                              		 					 1379410	    	832.6 ns/op
-BenchmarkSet_EvictionChannelAttached_LRA-16                 	 				  	  990680	   	 1197 ns/op
-BenchmarkSet_EvictionChannelAttached_LRI-16                  	 				 	 1000000	   	 1162 ns/op
-BenchmarkSet_ExistingKey_LRA-16                        	 						 3146720	    	417.8 ns/op
-BenchmarkSet_ExistingKey_LRI-16                        	 						 3876129	    	304.1 ns/op
-BenchmarkSet_Parallel_LRA-16                         	 						 1403163	    	943.2 ns/op
-BenchmarkSet_Parallel_LRI-16                         	 						 1391424	    	940.9 ns/op
-BenchmarkDelete_FullCache_1000000_Parallel_LRA-16               	 				 3293047	    	386.2 ns/op
-BenchmarkDelete_FullCache_1000000_Parallel_LRI-16               	 			 	 2478804	    	410.1 ns/op
-BenchmarkDelete_FullCache_1000000_Parallel_EvictionChannelAttached_LRA-16   				 1696992	    	589.3 ns/op
-BenchmarkDelete_FullCache_1000000_Parallel_EvictionChannelAttached_LRI-16   	 			 1825086	    	571.8 ns/op
-BenchmarkKeys_EmptyCache_LRA-16                       							33238173	    	36.14 ns/op
-BenchmarkKeys_EmptyCache_LRI-16                      						 	33220628	    	35.83 ns/op
-BenchmarkKeys_FullCache_1000000_LRA-16                    	  				    	       7	    153565363 ns/op
-BenchmarkKeys_FullCache_1000000_LRI-16                    	   					       7	    151832828 ns/op
-BenchmarkEntries_EmptyCache_LRA-16                      						36623247	    	38.97 ns/op
-BenchmarkEntries_EmptyCache_LRI-16                      						31233552	    	35.47 ns/op
-BenchmarkEntries_FullCache_1000000_LRA-16                   	   					       4	    256872533 ns/op
-BenchmarkEntries_FullCache_1000000_LRI-16                   	   				   	       4	    260215655 ns/op
+BenchmarkGet_EmptyCache_LRA-16                                              	29204826	        44.55 ns/op
+BenchmarkGet_EmptyCache_LRI-16                                              	28680338	        43.96 ns/op
+BenchmarkGet_NonExistingKey_LRA-16                                          	51993621	        22.72 ns/op
+BenchmarkGet_NonExistingKey_LRI-16                                          	53127615	        22.59 ns/op
+BenchmarkGet_ExistingKey_LRA-16                                             	15633502	        72.49 ns/op
+BenchmarkGet_ExistingKey_LRI-16                                             	14106654	        72.84 ns/op
+BenchmarkGet_FullCache_100000_Parallel_LRA-16                               	19972464	        57.66 ns/op
+BenchmarkGet_FullCache_100000_Parallel_LRI-16                               	21799681	        58.68 ns/op
+BenchmarkGet_FullCache_100000_WithTinyTTL_Parallel_LRA-16                   	22081754	        53.26 ns/op
+BenchmarkGet_FullCache_100000_WithTinyTTL_Parallel_LRI-16                   	20454691	        54.20 ns/op
+BenchmarkHas_FullCache_100000_Parallel-16                                   	19996741	        57.73 ns/op
+BenchmarkSet_LRA-16                                                         	 2263455	       561.0 ns/op
+BenchmarkSet_LRI-16                                                         	 2284095	       536.5 ns/op
+BenchmarkSet_EvictionChannelAttached_LRA-16                                 	 1348555	       877.9 ns/op
+BenchmarkSet_EvictionChannelAttached_LRI-16                                 	 1353513	       863.0 ns/op
+BenchmarkSet_ExistingKey_LRA-16                                             	 5201346	       258.9 ns/op
+BenchmarkSet_ExistingKey_LRI-16                                             	 5345276	       220.7 ns/op
+BenchmarkSet_Parallel_LRA-16                                                	 1789500	       682.0 ns/op
+BenchmarkSet_Parallel_LRI-16                                                	 1793680	       690.5 ns/op
+BenchmarkDelete_FullCache_100000_Parallel_LRA-16                            	 6431373	       185.6 ns/op
+BenchmarkDelete_FullCache_100000_Parallel_LRI-16                            	 6575835	       185.6 ns/op
+BenchmarkDelete_FullCache_100000_Parallel_EvictionChannelAttached_LRA-16    	 5505640	       187.3 ns/op
+BenchmarkDelete_FullCache_100000_Parallel_EvictionChannelAttached_LRI-16    	 5221836	       198.1 ns/op
+BenchmarkKeys_EmptyCache_LRA-16                                             	22313456	        52.64 ns/op
+BenchmarkKeys_EmptyCache_LRI-16                                             	23257614	        51.02 ns/op
+BenchmarkKeys_FullCache_100000_LRA-16                                       	     100	  10792932 ns/op
+BenchmarkKeys_FullCache_100000_LRI-16                                       	     100	  10719558 ns/op
+BenchmarkEntries_EmptyCache_LRA-16                                          	23483229	        51.20 ns/op
+BenchmarkEntries_EmptyCache_LRI-16                                          	22836296	        52.65 ns/op
+BenchmarkEntries_FullCache_100000_LRA-16                                    	      74	  16529933 ns/op
+BenchmarkEntries_FullCache_100000_LRI-16                                    	      76	  15255575 ns/op
 ```
 
 ### License

diff --git a/examples/lra/tlru_lra.go b/examples/lra/tlru_lra.go
@@ -20,10 +20,11 @@ var (
 func main() {
 	evictionChannel := make(chan tlru.EvictedEntry, 0)
 	config := tlru.Config{
-		Size:            2,
-		TTL:             ttl,
-		EvictionPolicy:  tlru.LRA,
-		EvictionChannel: &evictionChannel,
+		MaxSize:                   2,
+		TTL:                       ttl,
+		EvictionPolicy:            tlru.LRA,
+		EvictionChannel:           &evictionChannel,
+		GarbageCollectionInterval: &ttl,
 	}
 	cache := tlru.New(config)
 

diff --git a/examples/lri/tlru_lri.go b/examples/lri/tlru_lri.go
@@ -20,10 +20,11 @@ var (
 func main() {
 	evictionChannel := make(chan tlru.EvictedEntry, 0)
 	config := tlru.Config{
-		Size:            3,
-		TTL:             ttl,
-		EvictionPolicy:  tlru.LRI,
-		EvictionChannel: &evictionChannel,
+		MaxSize:                   3,
+		TTL:                       ttl,
+		EvictionPolicy:            tlru.LRI,
+		EvictionChannel:           &evictionChannel,
+		GarbageCollectionInterval: &ttl,
 	}
 	cache := tlru.New(config)
 

diff --git a/tlru.go b/tlru.go
@@ -31,7 +31,7 @@ type TLRU interface {
 	//		- If the key entry doesn't exist then it inserts it as the most
 	//		  recently used entry with Counter = 0
 	//		- If the key entry already exists then it will return an error
-	//		- If the cache is full (Config.Size) then the least recently accessed
+	//		- If the cache is full (Config.MaxSize) then the least recently accessed
 	//		  entry(the node before the tailNode) will be dropped and an
 	//		  EvictedEntry will be emitted to the EvictionChannel(if present)
 	//		  with EvictionReasonDropped
@@ -41,7 +41,7 @@ type TLRU interface {
 	//		- If the key entry already exists then it will update
 	//		  the Value, Counter and LastUsedAt properties of
 	//		  the existing entry and mark it as the most recently used entry
-	//		- If the cache is full (Config.Size) then
+	//		- If the cache is full (Config.MaxSize) then
 	//		  the least recently inserted entry(the node before the tailNode)
 	//		  will be dropped and an EvictedEntry will be emitted to
 	//		  the EvictionChannel(if present) with EvictionReasonDropped
@@ -80,13 +80,15 @@ type TLRU interface {
 // Config of tlru cache
 type Config struct {
 	// Max size of cache
-	Size int
+	MaxSize int
 	// Time to live of cached entries
 	TTL time.Duration
 	// Channel to listen for evicted entries events
 	EvictionChannel *chan EvictedEntry
 	// Eviction policy of tlru. Default is LRA
 	EvictionPolicy evictionPolicy
+	// GarbageCollectionInterval. If not set it defaults to 10 seconds
+	GarbageCollectionInterval *time.Duration
 }
 
 // Entry to be cached
@@ -168,12 +170,17 @@ const (
 	EvictionReasonDeleted
 )
 
+const (
+	defaultGarbageCollectionInterval = 10 * time.Second
+)
+
 type tlru struct {
 	sync.RWMutex
-	cache    map[string]*doublyLinkedNode
-	config   Config
-	headNode *doublyLinkedNode
-	tailNode *doublyLinkedNode
+	cache                     map[string]*doublyLinkedNode
+	config                    Config
+	headNode                  *doublyLinkedNode
+	tailNode                  *doublyLinkedNode
+	garbageCollectionInterval time.Duration
 }
 
 // New returns a new instance of TLRU cache
@@ -183,35 +190,49 @@ func New(config Config) TLRU {
 	headNode.next = tailNode
 	tailNode.previous = headNode
 
+	garbageCollectionInterval := defaultGarbageCollectionInterval
+	if config.GarbageCollectionInterval != nil {
+		garbageCollectionInterval = *config.GarbageCollectionInterval
+	}
+
 	cache := &tlru{
-		config: config,
-		cache:  make(map[string]*doublyLinkedNode, 0),
+		config:                    config,
+		cache:                     make(map[string]*doublyLinkedNode, 0),
+		garbageCollectionInterval: garbageCollectionInterval,
 	}
 
 	cache.initializeDoublyLinkedList()
-	cache.startTTLEvictionDaemon()
+	go cache.startTTLEvictionDaemon()
 
 	return cache
 }
 
 func (c *tlru) Get(key string) *CacheEntry {
-	defer c.RUnlock()
 	c.RLock()
 
 	linkedNode, exists := c.cache[key]
 	if !exists {
+		c.RUnlock()
 		return nil
 	}
 
 	if c.config.TTL < time.Since(linkedNode.lastUsedAt) {
+		c.RUnlock()
+		c.Lock()
+		defer c.Unlock()
 		c.evictEntry(linkedNode, EvictionReasonExpired)
 		return nil
 	}
 
 	if c.config.EvictionPolicy == LRA {
+		c.RUnlock()
+		c.Lock()
 		c.handleNodeState(Entry{Key: key, Value: linkedNode.value})
+		c.Unlock()
+		c.RLock()
 	}
 
+	defer c.RUnlock()
 	cacheEntry := linkedNode.ToCacheEntry()
 
 	return &cacheEntry
@@ -222,7 +243,7 @@ func (c *tlru) Set(entry Entry) error {
 	c.Lock()
 
 	_, exists := c.cache[entry.Key]
-	if !exists && len(c.cache) == c.config.Size {
+	if c.config.MaxSize != 0 && !exists && len(c.cache) == c.config.MaxSize {
 		c.evictEntry(c.tailNode.previous, EvictionReasonDropped)
 	}
 
@@ -246,9 +267,12 @@ func (c *tlru) Delete(key string) {
 }
 
 func (c *tlru) Keys() []string {
+	c.Lock()
+	c.evictExpiredEntries()
+	c.Unlock()
+
 	defer c.RUnlock()
 	c.RLock()
-	c.evictExpiredEntries()
 
 	keys := make([]string, 0, len(c.cache))
 	for key := range c.cache {
@@ -259,9 +283,12 @@ func (c *tlru) Keys() []string {
 }
 
 func (c *tlru) Entries() []CacheEntry {
+	c.Lock()
+	c.evictExpiredEntries()
+	c.Unlock()
+
 	defer c.RUnlock()
 	c.RLock()
-	c.evictExpiredEntries()
 
 	entries := make([]CacheEntry, 0, len(c.cache))
 	for _, linkedNode := range c.cache {
@@ -465,12 +492,10 @@ func (c *tlru) evictExpiredEntries() {
 }
 
 func (c *tlru) startTTLEvictionDaemon() {
-	go func() {
-		for {
-			time.Sleep(c.config.TTL)
-			c.Lock()
-			c.evictExpiredEntries()
-			c.Unlock()
-		}
-	}()
+	for {
+		time.Sleep(c.garbageCollectionInterval)
+		c.Lock()
+		c.evictExpiredEntries()
+		c.Unlock()
+	}
 }