From 634a9c162d015aec027c47d12ed08fa99b10619f Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Sun, 3 Oct 2021 09:33:58 +0200 Subject: [PATCH] Check cap instead of len in bloblru restic dump uses bloblru.Cache to keep buffers alive, but also reuses evicted buffers. That means large buffers may be used to store small blobs, causing the cache to think it's using less memory than it actually does. --- internal/bloblru/cache.go | 8 ++++---- internal/bloblru/cache_test.go | 8 +++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/bloblru/cache.go b/internal/bloblru/cache.go index dc977e650..b524f870c 100644 --- a/internal/bloblru/cache.go +++ b/internal/bloblru/cache.go @@ -47,7 +47,7 @@ func New(size int) *Cache { func (c *Cache) Add(id restic.ID, blob []byte) (old []byte) { debug.Log("bloblru.Cache: add %v", id) - size := len(blob) + overhead + size := cap(blob) + overhead if size > c.size { return } @@ -66,7 +66,7 @@ func (c *Cache) Add(id restic.ID, blob []byte) (old []byte) { for size > c.free { _, val, _ := c.c.RemoveOldest() b := val.([]byte) - if len(b) > len(old) { + if cap(b) > cap(old) { // We can only return one buffer, so pick the largest. old = b } @@ -91,6 +91,6 @@ func (c *Cache) Get(id restic.ID) ([]byte, bool) { func (c *Cache) evict(key, value interface{}) { blob := value.([]byte) - debug.Log("bloblru.Cache: evict %v, %d bytes", key, len(blob)) - c.free += len(blob) + overhead + debug.Log("bloblru.Cache: evict %v, %d bytes", key, cap(blob)) + c.free += cap(blob) + overhead } diff --git a/internal/bloblru/cache_test.go b/internal/bloblru/cache_test.go index c257a95e2..34280e35c 100644 --- a/internal/bloblru/cache_test.go +++ b/internal/bloblru/cache_test.go @@ -28,9 +28,11 @@ func TestCache(t *testing.T) { rtest.Equals(t, exp, blob) } - addAndCheck(id1, make([]byte, 32*kiB)) - addAndCheck(id2, make([]byte, 30*kiB)) - addAndCheck(id3, make([]byte, 10*kiB)) + // Our blobs have len 1 but larger cap. The cache should check the cap, + // since it more reliably indicates the amount of memory kept alive. + addAndCheck(id1, make([]byte, 1, 32*kiB)) + addAndCheck(id2, make([]byte, 1, 30*kiB)) + addAndCheck(id3, make([]byte, 1, 10*kiB)) _, ok := c.Get(id2) rtest.Assert(t, ok, "blob %v not present", id2)