From d129baba7ac68fc8c2ec280365521b28262ba989 Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Fri, 27 Jan 2023 15:01:54 +0100 Subject: [PATCH] repository: Reuse buffers in Repository.LoadUnpacked This method had a buffer argument, but that was nil at all call sites. That's removed, and instead LoadUnpacked now reuses whatever it allocates inside its retry loop. --- cmd/restic/cmd_cat.go | 2 +- internal/index/index_parallel.go | 2 +- internal/repository/repository.go | 17 +++++++---------- internal/repository/repository_test.go | 6 +++--- internal/restic/config_test.go | 8 ++++---- internal/restic/json.go | 2 +- internal/restic/repository.go | 8 +++----- 7 files changed, 20 insertions(+), 25 deletions(-) diff --git a/cmd/restic/cmd_cat.go b/cmd/restic/cmd_cat.go index f46502d5a..e7253e5b6 100644 --- a/cmd/restic/cmd_cat.go +++ b/cmd/restic/cmd_cat.go @@ -72,7 +72,7 @@ func runCat(ctx context.Context, gopts GlobalOptions, args []string) error { Println(string(buf)) return nil case "index": - buf, err := repo.LoadUnpacked(ctx, restic.IndexFile, id, nil) + buf, err := repo.LoadUnpacked(ctx, restic.IndexFile, id) if err != nil { return err } diff --git a/internal/index/index_parallel.go b/internal/index/index_parallel.go index a76b08a4e..e7e46e88a 100644 --- a/internal/index/index_parallel.go +++ b/internal/index/index_parallel.go @@ -24,7 +24,7 @@ func ForAllIndexes(ctx context.Context, repo restic.Repository, var idx *Index oldFormat := false - buf, err := repo.LoadUnpacked(ctx, restic.IndexFile, id, nil) + buf, err := repo.LoadUnpacked(ctx, restic.IndexFile, id) if err == nil { idx, oldFormat, err = DecodeIndex(buf, id) } diff --git a/internal/repository/repository.go b/internal/repository/repository.go index df8a6fb68..35826d9c9 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -170,14 +170,8 @@ func (r *Repository) SetDryRun() { r.be = dryrun.New(r.be) } -// LoadUnpacked loads and decrypts the file with the given type and ID, using -// the supplied buffer (which must be empty). If the buffer is nil, a new -// buffer will be allocated and returned. -func (r *Repository) LoadUnpacked(ctx context.Context, t restic.FileType, id restic.ID, buf []byte) ([]byte, error) { - if len(buf) != 0 { - panic("buf is not empty") - } - +// LoadUnpacked loads and decrypts the file with the given type and ID. +func (r *Repository) LoadUnpacked(ctx context.Context, t restic.FileType, id restic.ID) ([]byte, error) { debug.Log("load %v with id %v", t, id) if t == restic.ConfigFile { @@ -189,15 +183,17 @@ func (r *Repository) LoadUnpacked(ctx context.Context, t restic.FileType, id res h := restic.Handle{Type: t, Name: id.String()} retriedInvalidData := false var dataErr error + wr := new(bytes.Buffer) + err := r.be.Load(ctx, h, 0, 0, func(rd io.Reader) error { // make sure this call is idempotent, in case an error occurs - wr := bytes.NewBuffer(buf[:0]) + wr.Reset() _, cerr := io.Copy(wr, rd) if cerr != nil { return cerr } - buf = wr.Bytes() + buf := wr.Bytes() if t != restic.ConfigFile && !restic.Hash(buf).Equal(id) { debug.Log("retry loading broken blob %v", h) if !retriedInvalidData { @@ -221,6 +217,7 @@ func (r *Repository) LoadUnpacked(ctx context.Context, t restic.FileType, id res return nil, err } + buf := wr.Bytes() nonce, ciphertext := buf[:r.key.NonceSize()], buf[r.key.NonceSize():] plaintext, err := r.key.Open(ciphertext[:0], nonce, ciphertext, nil) if err != nil { diff --git a/internal/repository/repository_test.go b/internal/repository/repository_test.go index 6c04f1f95..13b5ad79f 100644 --- a/internal/repository/repository_test.go +++ b/internal/repository/repository_test.go @@ -232,7 +232,7 @@ func benchmarkLoadUnpacked(b *testing.B, version uint) { b.SetBytes(int64(length)) for i := 0; i < b.N; i++ { - data, err := repo.LoadUnpacked(context.TODO(), restic.PackFile, storageID, nil) + data, err := repo.LoadUnpacked(context.TODO(), restic.PackFile, storageID) rtest.OK(b, err) // See comment in BenchmarkLoadBlob. @@ -261,7 +261,7 @@ func TestRepositoryLoadIndex(t *testing.T) { // loadIndex loads the index id from backend and returns it. func loadIndex(ctx context.Context, repo restic.Repository, id restic.ID) (*index.Index, error) { - buf, err := repo.LoadUnpacked(ctx, restic.IndexFile, id, nil) + buf, err := repo.LoadUnpacked(ctx, restic.IndexFile, id) if err != nil { return nil, err } @@ -289,7 +289,7 @@ func TestRepositoryLoadUnpackedBroken(t *testing.T) { rtest.OK(t, err) // without a retry backend this will just return an error that the file is broken - _, err = repo.LoadUnpacked(context.TODO(), restic.IndexFile, id, nil) + _, err = repo.LoadUnpacked(context.TODO(), restic.IndexFile, id) if err == nil { t.Fatal("missing expected error") } diff --git a/internal/restic/config_test.go b/internal/restic/config_test.go index 662a2e69e..759788f6b 100644 --- a/internal/restic/config_test.go +++ b/internal/restic/config_test.go @@ -21,11 +21,11 @@ func (s saver) Connections() uint { } type loader struct { - fn func(restic.FileType, restic.ID, []byte) ([]byte, error) + fn func(restic.FileType, restic.ID) ([]byte, error) } -func (l loader) LoadUnpacked(ctx context.Context, t restic.FileType, id restic.ID, buf []byte) (data []byte, err error) { - return l.fn(t, id, buf) +func (l loader) LoadUnpacked(ctx context.Context, t restic.FileType, id restic.ID) (data []byte, err error) { + return l.fn(t, id) } func (l loader) Connections() uint { @@ -49,7 +49,7 @@ func TestConfig(t *testing.T) { err = restic.SaveConfig(context.TODO(), saver{save}, cfg1) rtest.OK(t, err) - load := func(tpe restic.FileType, id restic.ID, in []byte) ([]byte, error) { + load := func(tpe restic.FileType, id restic.ID) ([]byte, error) { rtest.Assert(t, tpe == restic.ConfigFile, "wrong backend type: got %v, wanted %v", tpe, restic.ConfigFile) diff --git a/internal/restic/json.go b/internal/restic/json.go index 6ad4b5f39..05d049b59 100644 --- a/internal/restic/json.go +++ b/internal/restic/json.go @@ -11,7 +11,7 @@ import ( // LoadJSONUnpacked decrypts the data and afterwards calls json.Unmarshal on // the item. func LoadJSONUnpacked(ctx context.Context, repo LoaderUnpacked, t FileType, id ID, item interface{}) (err error) { - buf, err := repo.LoadUnpacked(ctx, t, id, nil) + buf, err := repo.LoadUnpacked(ctx, t, id) if err != nil { return err } diff --git a/internal/restic/repository.go b/internal/restic/repository.go index e01d204e6..6990200e4 100644 --- a/internal/restic/repository.go +++ b/internal/restic/repository.go @@ -51,10 +51,8 @@ type Repository interface { StartPackUploader(ctx context.Context, wg *errgroup.Group) Flush(context.Context) error - // LoadUnpacked loads and decrypts the file with the given type and ID, - // using the supplied buffer (which must be empty). If the buffer is nil, a - // new buffer will be allocated and returned. - LoadUnpacked(ctx context.Context, t FileType, id ID, buf []byte) (data []byte, err error) + // LoadUnpacked loads and decrypts the file with the given type and ID. + LoadUnpacked(ctx context.Context, t FileType, id ID) (data []byte, err error) SaveUnpacked(context.Context, FileType, []byte) (ID, error) } @@ -67,7 +65,7 @@ type Lister interface { type LoaderUnpacked interface { // Connections returns the maximum number of concurrent backend operations Connections() uint - LoadUnpacked(ctx context.Context, t FileType, id ID, buf []byte) (data []byte, err error) + LoadUnpacked(ctx context.Context, t FileType, id ID) (data []byte, err error) } // SaverUnpacked allows saving a blob not stored in a pack file