Centralize buffer allocation and size checking in Repository.LoadBlob

Benchmark results for internal/repository:

name              old time/op    new time/op    delta
LoadTree-8           479µs ± 2%     478µs ± 1%   ~     (p=0.780 n=10+9)
LoadBlob-8          11.6ms ± 2%    11.6ms ± 1%   ~     (p=0.631 n=10+10)
LoadAndDecrypt-8    13.2ms ± 2%    13.3ms ± 3%   ~     (p=0.631 n=10+10)

name              old alloc/op   new alloc/op   delta
LoadTree-8          41.2kB ± 0%    41.2kB ± 0%   ~     (all equal)
LoadBlob-8          2.28kB ± 0%    2.28kB ± 0%   ~     (all equal)
LoadAndDecrypt-8    2.10MB ± 0%    2.10MB ± 0%   ~     (all equal)

name              old allocs/op  new allocs/op  delta
LoadTree-8             652 ± 0%       652 ± 0%   ~     (all equal)
LoadBlob-8            24.0 ± 0%      24.0 ± 0%   ~     (all equal)
LoadAndDecrypt-8      30.0 ± 0%      30.0 ± 0%   ~     (all equal)

name              old speed      new speed      delta
LoadBlob-8        86.2MB/s ± 2%  86.4MB/s ± 1%   ~     (p=0.594 n=10+10)
LoadAndDecrypt-8  75.7MB/s ± 2%  75.4MB/s ± 3%   ~     (p=0.617 n=10+10)

internal/repository/repository.go

@@ -670,29 +670,28 @@ func (r *Repository) Close() error {
 	return r.be.Close()
 }
 
-// LoadBlob loads a blob of type t from the repository to the buffer. buf must
-// be large enough to hold the encrypted blob, since it is used as scratch
-// space.
-func (r *Repository) LoadBlob(ctx context.Context, t restic.BlobType, id restic.ID, buf []byte) (int, error) {
+// LoadBlob loads a blob of type t from the repository.
+// It may use all of buf[:cap(buf)] as scratch space.
+func (r *Repository) LoadBlob(ctx context.Context, t restic.BlobType, id restic.ID, buf []byte) ([]byte, error) {
 	debug.Log("load blob %v into buf (len %v, cap %v)", id, len(buf), cap(buf))
 	size, found := r.idx.LookupSize(id, t)
 	if !found {
-		return 0, errors.Errorf("id %v not found in repository", id)
+		return nil, errors.Errorf("id %v not found in repository", id)
 	}
 
 	if cap(buf) < restic.CiphertextLength(int(size)) {
-		return 0, errors.Errorf("buffer is too small for data blob (%d < %d)", cap(buf), restic.CiphertextLength(int(size)))
+		buf = restic.NewBlobBuffer(int(size))
 	}
 
 	n, err := r.loadBlob(ctx, id, t, buf)
 	if err != nil {
-		return 0, err
+		return nil, err
 	}
 	buf = buf[:n]
 
 	debug.Log("loaded %d bytes into buf %p", len(buf), buf)
 
-	return len(buf), err
+	return buf, err
 }
 
 // SaveBlob saves a blob of type t into the repository. If id is the null id, it

internal/repository/repository_test.go

@@ -43,10 +43,9 @@ func TestSave(t *testing.T) {
 		// rtest.OK(t, repo.SaveIndex())
 
 		// read back
-		buf := restic.NewBlobBuffer(size)
-		n, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, buf)
+		buf, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, nil)
 		rtest.OK(t, err)
-		rtest.Equals(t, len(buf), n)
+		rtest.Equals(t, size, len(buf))
 
 		rtest.Assert(t, len(buf) == len(data),
 			"number of bytes read back does not match: expected %d, got %d",
@@ -77,10 +76,9 @@ func TestSaveFrom(t *testing.T) {
 		rtest.OK(t, repo.Flush(context.Background()))
 
 		// read back
-		buf := restic.NewBlobBuffer(size)
-		n, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, buf)
+		buf, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, nil)
 		rtest.OK(t, err)
-		rtest.Equals(t, len(buf), n)
+		rtest.Equals(t, size, len(buf))
 
 		rtest.Assert(t, len(buf) == len(data),
 			"number of bytes read back does not match: expected %d, got %d",
@@ -164,33 +162,17 @@ func TestLoadBlob(t *testing.T) {
 	rtest.OK(t, err)
 	rtest.OK(t, repo.Flush(context.Background()))
 
-	// first, test with buffers that are too small
-	for _, testlength := range []int{length - 20, length, restic.CiphertextLength(length) - 1} {
-		buf = make([]byte, 0, testlength)
-		n, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, buf)
-		if err == nil {
-			t.Errorf("LoadBlob() did not return an error for a buffer that is too small to hold the blob")
-			continue
-		}
-
-		if n != 0 {
-			t.Errorf("LoadBlob() returned an error and n > 0")
-			continue
-		}
-	}
-
-	// then use buffers that are large enough
 	base := restic.CiphertextLength(length)
-	for _, testlength := range []int{base, base + 7, base + 15, base + 1000} {
+	for _, testlength := range []int{0, base - 20, base - 1, base, base + 7, base + 15, base + 1000} {
 		buf = make([]byte, 0, testlength)
-		n, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, buf)
+		buf, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, buf)
 		if err != nil {
 			t.Errorf("LoadBlob() returned an error for buffer size %v: %v", testlength, err)
 			continue
 		}
 
-		if n != length {
-			t.Errorf("LoadBlob() returned the wrong number of bytes: want %v, got %v", length, n)
+		if len(buf) != length {
+			t.Errorf("LoadBlob() returned the wrong number of bytes: want %v, got %v", length, len(buf))
 			continue
 		}
 	}
@@ -213,13 +195,14 @@ func BenchmarkLoadBlob(b *testing.B) {
 	b.SetBytes(int64(length))
 
 	for i := 0; i < b.N; i++ {
-		n, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, buf)
+		var err error
+		buf, err = repo.LoadBlob(context.TODO(), restic.DataBlob, id, buf)
 		rtest.OK(b, err)
-		if n != length {
-			b.Errorf("wanted %d bytes, got %d", length, n)
+		if len(buf) != length {
+			b.Errorf("wanted %d bytes, got %d", length, len(buf))
 		}
 
-		id2 := restic.Hash(buf[:n])
+		id2 := restic.Hash(buf)
 		if !id.Equal(id2) {
 			b.Errorf("wrong data returned, wanted %v, got %v", id.Str(), id2.Str())
 		}