aboutsummaryrefslogtreecommitdiffstats
path: root/plumbing
diff options
context:
space:
mode:
authorMáximo Cuadros <mcuadros@gmail.com>2018-09-18 12:18:00 +0200
committerGitHub <noreply@github.com>2018-09-18 12:18:00 +0200
commitf69f5304328165153e452382f44aa2b41df36d0f (patch)
tree58a7fb3508e0f7dd637a1e4f62984bfcaa7cb84b /plumbing
parent2fb32d2a8601213b6db109d3e9028c6b64af1874 (diff)
parentedfc16e3ea6b0ce2533bacb5f370d042042b4784 (diff)
downloadgo-git-f69f5304328165153e452382f44aa2b41df36d0f.tar.gz
Merge pull request #958 from kuba--/fix-cachesize
Fix potential LRU cache size issue.
Diffstat (limited to 'plumbing')
-rw-r--r--plumbing/cache/buffer_lru.go24
-rw-r--r--plumbing/cache/buffer_test.go23
-rw-r--r--plumbing/cache/object_lru.go24
-rw-r--r--plumbing/cache/object_test.go19
4 files changed, 66 insertions, 24 deletions
diff --git a/plumbing/cache/buffer_lru.go b/plumbing/cache/buffer_lru.go
index f2c0f90..acaf195 100644
--- a/plumbing/cache/buffer_lru.go
+++ b/plumbing/cache/buffer_lru.go
@@ -45,19 +45,23 @@ func (c *BufferLRU) Put(key int64, slice []byte) {
c.ll = list.New()
}
+ bufSize := FileSize(len(slice))
if ee, ok := c.cache[key]; ok {
+ oldBuf := ee.Value.(buffer)
+ // in this case bufSize is a delta: new size - old size
+ bufSize -= FileSize(len(oldBuf.Slice))
c.ll.MoveToFront(ee)
ee.Value = buffer{key, slice}
- return
+ } else {
+ if bufSize > c.MaxSize {
+ return
+ }
+ ee := c.ll.PushFront(buffer{key, slice})
+ c.cache[key] = ee
}
- objSize := FileSize(len(slice))
-
- if objSize > c.MaxSize {
- return
- }
-
- for c.actualSize+objSize > c.MaxSize {
+ c.actualSize += bufSize
+ for c.actualSize > c.MaxSize {
last := c.ll.Back()
lastObj := last.Value.(buffer)
lastSize := FileSize(len(lastObj.Slice))
@@ -66,10 +70,6 @@ func (c *BufferLRU) Put(key int64, slice []byte) {
delete(c.cache, lastObj.Key)
c.actualSize -= lastSize
}
-
- ee := c.ll.PushFront(buffer{key, slice})
- c.cache[key] = ee
- c.actualSize += objSize
}
// Get returns a buffer by its key. It marks the buffer as used. If the buffer
diff --git a/plumbing/cache/buffer_test.go b/plumbing/cache/buffer_test.go
index 262138a..3e3adc2 100644
--- a/plumbing/cache/buffer_test.go
+++ b/plumbing/cache/buffer_test.go
@@ -1,6 +1,7 @@
package cache
import (
+ "bytes"
"sync"
. "gopkg.in/check.v1"
@@ -38,6 +39,28 @@ func (s *BufferSuite) TestPutSameBuffer(c *C) {
}
}
+func (s *ObjectSuite) TestPutSameBufferWithDifferentSize(c *C) {
+ aBuffer := []byte("a")
+ bBuffer := []byte("bbb")
+ cBuffer := []byte("ccccc")
+ dBuffer := []byte("ddddddd")
+
+ cache := NewBufferLRU(7 * Byte)
+ cache.Put(1, aBuffer)
+ cache.Put(1, bBuffer)
+ cache.Put(1, cBuffer)
+ cache.Put(1, dBuffer)
+
+ c.Assert(cache.MaxSize, Equals, 7*Byte)
+ c.Assert(cache.actualSize, Equals, 7*Byte)
+ c.Assert(cache.ll.Len(), Equals, 1)
+
+ buf, ok := cache.Get(1)
+ c.Assert(bytes.Equal(buf, dBuffer), Equals, true)
+ c.Assert(FileSize(len(buf)), Equals, 7*Byte)
+ c.Assert(ok, Equals, true)
+}
+
func (s *BufferSuite) TestPutBigBuffer(c *C) {
for _, o := range s.c {
o.Put(1, s.bBuffer)
diff --git a/plumbing/cache/object_lru.go b/plumbing/cache/object_lru.go
index 0494539..53d8b02 100644
--- a/plumbing/cache/object_lru.go
+++ b/plumbing/cache/object_lru.go
@@ -42,20 +42,24 @@ func (c *ObjectLRU) Put(obj plumbing.EncodedObject) {
c.ll = list.New()
}
+ objSize := FileSize(obj.Size())
key := obj.Hash()
if ee, ok := c.cache[key]; ok {
+ oldObj := ee.Value.(plumbing.EncodedObject)
+ // in this case objSize is a delta: new size - old size
+ objSize -= FileSize(oldObj.Size())
c.ll.MoveToFront(ee)
ee.Value = obj
- return
- }
-
- objSize := FileSize(obj.Size())
-
- if objSize > c.MaxSize {
- return
+ } else {
+ if objSize > c.MaxSize {
+ return
+ }
+ ee := c.ll.PushFront(obj)
+ c.cache[key] = ee
}
- for c.actualSize+objSize > c.MaxSize {
+ c.actualSize += objSize
+ for c.actualSize > c.MaxSize {
last := c.ll.Back()
lastObj := last.Value.(plumbing.EncodedObject)
lastSize := FileSize(lastObj.Size())
@@ -64,10 +68,6 @@ func (c *ObjectLRU) Put(obj plumbing.EncodedObject) {
delete(c.cache, lastObj.Hash())
c.actualSize -= lastSize
}
-
- ee := c.ll.PushFront(obj)
- c.cache[key] = ee
- c.actualSize += objSize
}
// Get returns an object by its hash. It marks the object as used. If the object
diff --git a/plumbing/cache/object_test.go b/plumbing/cache/object_test.go
index ac3f0a3..b3e5f79 100644
--- a/plumbing/cache/object_test.go
+++ b/plumbing/cache/object_test.go
@@ -45,6 +45,25 @@ func (s *ObjectSuite) TestPutSameObject(c *C) {
}
}
+func (s *ObjectSuite) TestPutSameObjectWithDifferentSize(c *C) {
+ const hash = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+
+ cache := NewObjectLRU(7 * Byte)
+ cache.Put(newObject(hash, 1*Byte))
+ cache.Put(newObject(hash, 3*Byte))
+ cache.Put(newObject(hash, 5*Byte))
+ cache.Put(newObject(hash, 7*Byte))
+
+ c.Assert(cache.MaxSize, Equals, 7*Byte)
+ c.Assert(cache.actualSize, Equals, 7*Byte)
+ c.Assert(cache.ll.Len(), Equals, 1)
+
+ obj, ok := cache.Get(plumbing.NewHash(hash))
+ c.Assert(obj.Hash(), Equals, plumbing.NewHash(hash))
+ c.Assert(FileSize(obj.Size()), Equals, 7*Byte)
+ c.Assert(ok, Equals, true)
+}
+
func (s *ObjectSuite) TestPutBigObject(c *C) {
for _, o := range s.c {
o.Put(s.bObject)