From a8b6d9b5cc78bbdf6a08b700e62acc874d0bf1ed Mon Sep 17 00:00:00 2001 From: Umputun Date: Thu, 31 May 2018 15:44:00 -0500 Subject: [PATCH] switch to lru cache --- Gopkg.lock | 17 ++++++---- app/rest/cache/cache.go | 62 +++++++++++------------------------- app/rest/cache/cache_test.go | 37 ++++++++------------- app/rest/cache/options.go | 9 ------ 4 files changed, 42 insertions(+), 83 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 54cb2ac2..68bd5afb 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -87,6 +87,15 @@ revision = "6edcbcd2d57fd0bbd7f39947a593ed0c06648388" version = "v1.1.0" +[[projects]] + branch = "master" + name = "github.com/hashicorp/golang-lru" + packages = [ + ".", + "simplelru" + ] + revision = "0fb14efe8c47ae851c0034ed7a448854d3d34cf3" + [[projects]] branch = "master" name = "github.com/hashicorp/logutils" @@ -105,12 +114,6 @@ packages = ["."] revision = "542fd4642604d0d0c26112396ce5b1a9d01eee0b" -[[projects]] - name = "github.com/patrickmn/go-cache" - packages = ["."] - revision = "a3647f8e31d79543b2d0f0ae2fe5c379d72cedc0" - version = "v2.1.0" - [[projects]] name = "github.com/pkg/errors" packages = ["."] @@ -201,6 +204,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "48cfa8d6976e8914b91fc23341763e819862c54011b4c57845c1965c1e546000" + inputs-digest = "244a48c2c3d963efcaa0289ee0e72a5083ae8ef333af547637efaf3b521c8bb3" solver-name = "gps-cdcl" solver-version = 1 diff --git a/app/rest/cache/cache.go b/app/rest/cache/cache.go index 625f7f38..73bb43c5 100644 --- a/app/rest/cache/cache.go +++ b/app/rest/cache/cache.go @@ -3,10 +3,9 @@ package cache import ( "log" "strings" - "sync" "time" - "github.com/patrickmn/go-cache" + "github.com/hashicorp/golang-lru" "github.com/pkg/errors" ) @@ -36,42 +35,29 @@ func parseKey(fullKey string) (key string, scopes []string, err error) { // loadingCache implements LoadingCache interface on top of cache.Cache (go-cache) type loadingCache struct { - bytesCache *cache.Cache - postFlushFn func() - defaultExpiration time.Duration - cleanupInterval time.Duration - maxKeys int - maxValueSize int - - activeKeys map[string]struct{} // keep all current cached keys - lock sync.Mutex + bytesCache *lru.Cache + postFlushFn func() + maxKeys int + maxValueSize int } // NewLoadingCache makes loadingCache implementation func NewLoadingCache(options ...Option) LoadingCache { res := loadingCache{ - defaultExpiration: time.Hour, - cleanupInterval: 5 * time.Minute, - postFlushFn: func() {}, - maxKeys: 0, - maxValueSize: 0, - activeKeys: map[string]struct{}{}, + postFlushFn: func() {}, + maxKeys: 1000, + maxValueSize: 0, } for _, opt := range options { if err := opt(&res); err != nil { log.Printf("[WARN] failed to set cache option, %v", err) } } - res.bytesCache = cache.New(res.defaultExpiration, res.cleanupInterval) // OnEvicted called automatically for expired and manually deleted - res.bytesCache.OnEvicted(func(key string, _ interface{}) { - res.withLock(func() { delete(res.activeKeys, key) }) - }) - - log.Printf("[DEBUG] create cache with cleanupInterval=%s, maxKeys=%d, maxValueSize=%d", - res.cleanupInterval, res.maxKeys, res.maxValueSize) + res.bytesCache, _ = lru.New(res.maxKeys) + log.Printf("[DEBUG] create lru cache, maxKeys=%d, maxValueSize=%d", res.maxKeys, res.maxValueSize) return &res } @@ -85,24 +71,16 @@ func (lc *loadingCache) Get(key string, ttl time.Duration, fn func() ([]byte, er return data, err } if lc.allowed(data) { - lc.bytesCache.Set(key, data, ttl) - lc.withLock(func() { lc.activeKeys[key] = struct{}{} }) + lc.bytesCache.Add(key, data) } return data, nil } -func (lc *loadingCache) withLock(fn func()) { - lc.lock.Lock() - fn() - lc.lock.Unlock() -} - // Flush clears cache and calls postFlushFn async func (lc *loadingCache) Flush(scopes ...string) { if len(scopes) == 0 { - lc.bytesCache.Flush() - lc.withLock(func() { lc.activeKeys = map[string]struct{}{} }) + lc.bytesCache.Purge() go lc.postFlushFn() return } @@ -126,15 +104,14 @@ func (lc *loadingCache) Flush(scopes ...string) { // all matchedKeys should be collected first // we can't delete it from locked section, it will lock on eviction callback matchedKeys := []string{} - lc.withLock(func() { - for k := range lc.activeKeys { - if inScope(k) { - matchedKeys = append(matchedKeys, k) - } + for _, k := range lc.bytesCache.Keys() { + key := k.(string) + if inScope(key) { + matchedKeys = append(matchedKeys, key) } - }) + } for _, mkey := range matchedKeys { - lc.bytesCache.Delete(mkey) + lc.bytesCache.Remove(mkey) } if lc.postFlushFn != nil { @@ -146,8 +123,5 @@ func (lc *loadingCache) allowed(data []byte) bool { if lc.maxValueSize > 0 && len(data) >= lc.maxValueSize { return false } - if lc.maxKeys > 0 && lc.bytesCache.ItemCount() >= lc.maxKeys { - return false - } return true } diff --git a/app/rest/cache/cache_test.go b/app/rest/cache/cache_test.go index c9ec7391..14a7c2a3 100644 --- a/app/rest/cache/cache_test.go +++ b/app/rest/cache/cache_test.go @@ -10,13 +10,14 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/umputun/remark/app/rest" "github.com/umputun/remark/app/store" ) func TestLoadingCache_Get(t *testing.T) { var postFnCall, coldCalls int32 - lc := NewLoadingCache(CleanupInterval(200*time.Millisecond), PostFlushFn(func() { atomic.AddInt32(&postFnCall, 1) })) + lc := NewLoadingCache(PostFlushFn(func() { atomic.AddInt32(&postFnCall, 1) })) res, err := lc.Get("key", time.Minute, func() ([]byte, error) { atomic.AddInt32(&coldCalls, 1) @@ -43,7 +44,7 @@ func TestLoadingCache_Get(t *testing.T) { func TestLoadingCache_MaxKeys(t *testing.T) { var postFnCall, coldCalls int32 - lc := NewLoadingCache(CleanupInterval(200*time.Millisecond), PostFlushFn(func() { atomic.AddInt32(&postFnCall, 1) }), + lc := NewLoadingCache(PostFlushFn(func() { atomic.AddInt32(&postFnCall, 1) }), MaxKeys(5), MaxValSize(10)) // put 5 keys to cache @@ -63,7 +64,7 @@ func TestLoadingCache_MaxKeys(t *testing.T) { return []byte("result-blah"), nil }) assert.Nil(t, err) - assert.Equal(t, "result-3", string(res), "should get cached") + assert.Equal(t, "result-3", string(res), "should be cached") // try to cache after maxKeys reached res, err = lc.Get("key-X", time.Minute, func() ([]byte, error) { @@ -72,16 +73,7 @@ func TestLoadingCache_MaxKeys(t *testing.T) { assert.Nil(t, err) assert.Equal(t, "result-X", string(res)) - cc := atomic.LoadInt32(&coldCalls) - res, err = lc.Get("key-X", time.Minute, func() ([]byte, error) { - atomic.AddInt32(&coldCalls, 1) - return []byte("result-not-cached"), nil - }) - assert.Nil(t, err) - assert.Equal(t, cc+1, atomic.LoadInt32(&coldCalls)) - assert.Equal(t, "result-not-cached", string(res), "not cached") - - time.Sleep(time.Second) // let cleanup to remove + assert.Equal(t, 5, lc.(*loadingCache).bytesCache.Len()) // put to cache and make sure it cached res, err = lc.Get("key-Z", time.Minute, func() ([]byte, error) { @@ -95,10 +87,11 @@ func TestLoadingCache_MaxKeys(t *testing.T) { }) assert.Nil(t, err) assert.Equal(t, "result-Z", string(res), "got cached value") + assert.Equal(t, 5, lc.(*loadingCache).bytesCache.Len()) } func TestLoadingCache_MaxSize(t *testing.T) { - lc := NewLoadingCache(CleanupInterval(200*time.Millisecond), MaxKeys(5), MaxValSize(10)) + lc := NewLoadingCache(MaxKeys(5), MaxValSize(10)) // put good size value to cache and make sure it cached res, err := lc.Get("key-Z", time.Minute, func() ([]byte, error) { @@ -147,7 +140,7 @@ func TestLoadingCache_URLKey(t *testing.T) { func TestLoadingCache_Parallel(t *testing.T) { var coldCalls int32 - lc := NewLoadingCache(CleanupInterval(time.Second)) + lc := NewLoadingCache() res, err := lc.Get("key", time.Minute, func() ([]byte, error) { return []byte("value"), nil @@ -174,7 +167,7 @@ func TestLoadingCache_Parallel(t *testing.T) { } func TestLoadingCache_Scopes(t *testing.T) { - lc := NewLoadingCache(CleanupInterval(time.Second)) + lc := NewLoadingCache() res, err := lc.Get(Key("key", "s1", "s2"), time.Minute, func() ([]byte, error) { return []byte("value"), nil @@ -188,9 +181,9 @@ func TestLoadingCache_Scopes(t *testing.T) { assert.Nil(t, err) assert.Equal(t, "value2", string(res)) - assert.Equal(t, 2, len(lc.(*loadingCache).activeKeys)) + assert.Equal(t, 2, lc.(*loadingCache).bytesCache.Len()) lc.Flush("s1") - assert.Equal(t, 1, len(lc.(*loadingCache).activeKeys)) + assert.Equal(t, 1, lc.(*loadingCache).bytesCache.Len()) lc.Get(Key("key2", "s2"), time.Minute, func() ([]byte, error) { assert.Fail(t, "should stay") @@ -204,7 +197,7 @@ func TestLoadingCache_Scopes(t *testing.T) { } func TestLoadingCache_Flush(t *testing.T) { - lc := NewLoadingCache(CleanupInterval(time.Second)) + lc := NewLoadingCache() addToCache := func(key string, scopes ...string) { res, err := lc.Get(key, time.Minute, func() ([]byte, error) { @@ -223,7 +216,7 @@ func TestLoadingCache_Flush(t *testing.T) { addToCache(Key("key5", "s2")) addToCache(Key("key6")) addToCache(Key("key7", "s4", "s3")) - require.Equal(t, 7, len(lc.(*loadingCache).activeKeys), "cache init") + require.Equal(t, 7, lc.(*loadingCache).bytesCache.Len(), "cache init") } tbl := []struct { @@ -244,9 +237,7 @@ func TestLoadingCache_Flush(t *testing.T) { for i, tt := range tbl { init() lc.Flush(tt.scopes...) - assert.Equal(t, tt.left, len(lc.(*loadingCache).activeKeys), "keys size, %s #%d", tt.msg, i) - assert.Equal(t, tt.left, len(lc.(*loadingCache).bytesCache.Items()), "items size, %s #%d", tt.msg, i) - + assert.Equal(t, tt.left, lc.(*loadingCache).bytesCache.Len(), "keys size, %s #%d", tt.msg, i) } } diff --git a/app/rest/cache/options.go b/app/rest/cache/options.go index 8fda85f6..f10dd15c 100644 --- a/app/rest/cache/options.go +++ b/app/rest/cache/options.go @@ -3,7 +3,6 @@ package cache import ( "net/http" "strings" - "time" "github.com/umputun/remark/app/rest" ) @@ -29,14 +28,6 @@ func MaxKeys(max int) Option { } } -// CleanupInterval functional option defines how often cleanup loop activated. -func CleanupInterval(interval time.Duration) Option { - return func(lc *loadingCache) error { - lc.cleanupInterval = interval - return nil - } -} - // PostFlushFn functional option defines how callback function called after each Flush. func PostFlushFn(postFlushFn func()) Option { return func(lc *loadingCache) error {