From 9a8a2cae4c8bbbeaf0471b90008515063882b5fb Mon Sep 17 00:00:00 2001 From: greatroar <@> Date: Wed, 22 Jul 2020 23:53:55 +0200 Subject: [PATCH 1/2] Move pack testing logic to test file --- internal/pack/pack.go | 5 ----- internal/pack/pack_test.go | 6 +++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/internal/pack/pack.go b/internal/pack/pack.go index d4f064476..0e7995c61 100644 --- a/internal/pack/pack.go +++ b/internal/pack/pack.go @@ -151,11 +151,6 @@ func (p *Packer) Blobs() []restic.Blob { return p.blobs } -// Writer return the underlying writer. -func (p *Packer) Writer() io.Writer { - return p.wr -} - func (p *Packer) String() string { return fmt.Sprintf("", len(p.blobs), p.bytes) } diff --git a/internal/pack/pack_test.go b/internal/pack/pack_test.go index e40e5bd2a..99755c36f 100644 --- a/internal/pack/pack_test.go +++ b/internal/pack/pack_test.go @@ -36,7 +36,8 @@ func newPack(t testing.TB, k *crypto.Key, lengths []int) ([]Buf, []byte, uint) { } // pack blobs - p := pack.NewPacker(k, new(bytes.Buffer)) + var buf bytes.Buffer + p := pack.NewPacker(k, &buf) for _, b := range bufs { p.Add(restic.TreeBlob, b.id, b.data) } @@ -44,8 +45,7 @@ func newPack(t testing.TB, k *crypto.Key, lengths []int) ([]Buf, []byte, uint) { _, err := p.Finalize() rtest.OK(t, err) - packData := p.Writer().(*bytes.Buffer).Bytes() - return bufs, packData, p.Size() + return bufs, buf.Bytes(), p.Size() } func verifyBlobs(t testing.TB, bufs []Buf, k *crypto.Key, rd io.ReaderAt, packSize uint) { From ab2b7d7f9a8d49e00693f0867a126ad85be60f0d Mon Sep 17 00:00:00 2001 From: greatroar <@> Date: Wed, 22 Jul 2020 18:46:55 +0200 Subject: [PATCH 2/2] Decrease allocation rate in internal/pack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit internal/repository benchmark results: name old time/op new time/op delta PackerManager-8 179ms ± 1% 181ms ± 1% +0.78% (p=0.009 n=10+10) name old speed new speed delta PackerManager-8 294MB/s ± 1% 292MB/s ± 1% -0.77% (p=0.009 n=10+10) name old alloc/op new alloc/op delta PackerManager-8 91.3kB ± 0% 72.2kB ± 0% -20.92% (p=0.000 n=9+7) name old allocs/op new allocs/op delta PackerManager-8 1.38k ± 0% 0.76k ± 0% -45.20% (p=0.000 n=10+7) --- internal/pack/pack.go | 97 +++++++++++++-------------- internal/pack/pack_internal_test.go | 35 ++++++++++ internal/repository/packer_manager.go | 5 +- 3 files changed, 85 insertions(+), 52 deletions(-) diff --git a/internal/pack/pack.go b/internal/pack/pack.go index 0e7995c61..2c39009b1 100644 --- a/internal/pack/pack.go +++ b/internal/pack/pack.go @@ -1,7 +1,6 @@ package pack import ( - "bytes" "encoding/binary" "fmt" "io" @@ -49,7 +48,8 @@ func (p *Packer) Add(t restic.BlobType, id restic.ID, data []byte) (int, error) var entrySize = uint(binary.Size(restic.BlobType(0)) + binary.Size(uint32(0)) + len(restic.ID{})) -// headerEntry is used with encoding/binary to read and write header entries +// headerEntry describes the format of header entries. It serves only as +// documentation. type headerEntry struct { Type uint8 Length uint32 @@ -64,16 +64,15 @@ func (p *Packer) Finalize() (uint, error) { bytesWritten := p.bytes - hdrBuf := bytes.NewBuffer(nil) - bytesHeader, err := p.writeHeader(hdrBuf) + header, err := p.makeHeader() if err != nil { return 0, err } - encryptedHeader := make([]byte, 0, hdrBuf.Len()+p.k.Overhead()+p.k.NonceSize()) + encryptedHeader := make([]byte, 0, len(header)+p.k.Overhead()+p.k.NonceSize()) nonce := crypto.NewRandomNonce() encryptedHeader = append(encryptedHeader, nonce...) - encryptedHeader = p.k.Seal(encryptedHeader, nonce, hdrBuf.Bytes(), nil) + encryptedHeader = p.k.Seal(encryptedHeader, nonce, header, nil) // append the header n, err := p.wr.Write(encryptedHeader) @@ -81,7 +80,7 @@ func (p *Packer) Finalize() (uint, error) { return 0, errors.Wrap(err, "Write") } - hdrBytes := restic.CiphertextLength(int(bytesHeader)) + hdrBytes := restic.CiphertextLength(len(header)) if n != hdrBytes { return 0, errors.New("wrong number of bytes written") } @@ -99,32 +98,27 @@ func (p *Packer) Finalize() (uint, error) { return bytesWritten, nil } -// writeHeader constructs and writes the header to wr. -func (p *Packer) writeHeader(wr io.Writer) (bytesWritten uint, err error) { - for _, b := range p.blobs { - entry := headerEntry{ - Length: uint32(b.Length), - ID: b.ID, - } +// makeHeader constructs the header for p. +func (p *Packer) makeHeader() ([]byte, error) { + buf := make([]byte, 0, len(p.blobs)*int(entrySize)) + for _, b := range p.blobs { switch b.Type { case restic.DataBlob: - entry.Type = 0 + buf = append(buf, 0) case restic.TreeBlob: - entry.Type = 1 + buf = append(buf, 1) default: - return 0, errors.Errorf("invalid blob type %v", b.Type) + return nil, errors.Errorf("invalid blob type %v", b.Type) } - err := binary.Write(wr, binary.LittleEndian, entry) - if err != nil { - return bytesWritten, errors.Wrap(err, "binary.Write") - } - - bytesWritten += entrySize + var lenLE [4]byte + binary.LittleEndian.PutUint32(lenLE[:], uint32(b.Length)) + buf = append(buf, lenLE[:]...) + buf = append(buf, b.ID[:]...) } - return + return buf, nil } // Size returns the number of bytes written so far. @@ -275,40 +269,19 @@ func List(k *crypto.Key, rd io.ReaderAt, size int64) (entries []restic.Blob, err return nil, err } - hdrRd := bytes.NewReader(buf) - entries = make([]restic.Blob, 0, uint(len(buf))/entrySize) pos := uint(0) - for { - e := headerEntry{} - err = binary.Read(hdrRd, binary.LittleEndian, &e) - if errors.Cause(err) == io.EOF { - break - } - + for len(buf) > 0 { + entry, err := parseHeaderEntry(buf) if err != nil { - return nil, errors.Wrap(err, "binary.Read") - } - - entry := restic.Blob{ - Length: uint(e.Length), - ID: e.ID, - Offset: pos, - } - - switch e.Type { - case 0: - entry.Type = restic.DataBlob - case 1: - entry.Type = restic.TreeBlob - default: - return nil, errors.Errorf("invalid type %d", e.Type) + return nil, err } + entry.Offset = pos entries = append(entries, entry) - - pos += uint(e.Length) + pos += entry.Length + buf = buf[entrySize:] } return entries, nil @@ -318,3 +291,25 @@ func List(k *crypto.Key, rd io.ReaderAt, size int64) (entries []restic.Blob, err func PackedSizeOfBlob(blobLength uint) uint { return blobLength + entrySize } + +func parseHeaderEntry(p []byte) (b restic.Blob, err error) { + if uint(len(p)) < entrySize { + err = errors.Errorf("parseHeaderEntry: buffer of size %d too short", len(p)) + return b, err + } + p = p[:entrySize] + + switch p[0] { + case 0: + b.Type = restic.DataBlob + case 1: + b.Type = restic.TreeBlob + default: + return b, errors.Errorf("invalid type %d", p[0]) + } + + b.Length = uint(binary.LittleEndian.Uint32(p[1:5])) + copy(b.ID[:], p[5:]) + + return b, nil +} diff --git a/internal/pack/pack_internal_test.go b/internal/pack/pack_internal_test.go index 6694b7333..84b03adc2 100644 --- a/internal/pack/pack_internal_test.go +++ b/internal/pack/pack_internal_test.go @@ -7,9 +7,44 @@ import ( "testing" "github.com/restic/restic/internal/crypto" + "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" ) +func TestParseHeaderEntry(t *testing.T) { + h := headerEntry{ + Type: 0, // Blob. + Length: 100, + } + for i := range h.ID { + h.ID[i] = byte(i) + } + + buf := new(bytes.Buffer) + _ = binary.Write(buf, binary.LittleEndian, &h) + + b, err := parseHeaderEntry(buf.Bytes()) + rtest.OK(t, err) + rtest.Equals(t, restic.DataBlob, b.Type) + t.Logf("%v %v", h.ID, b.ID) + rtest.Assert(t, bytes.Equal(h.ID[:], b.ID[:]), "id mismatch") + rtest.Equals(t, uint(h.Length), b.Length) + + h.Type = 0xae + buf.Reset() + _ = binary.Write(buf, binary.LittleEndian, &h) + + b, err = parseHeaderEntry(buf.Bytes()) + rtest.Assert(t, err != nil, "no error for invalid type") + + h.Type = 0 + buf.Reset() + _ = binary.Write(buf, binary.LittleEndian, &h) + + b, err = parseHeaderEntry(buf.Bytes()[:entrySize-1]) + rtest.Assert(t, err != nil, "no error for short input") +} + type countingReaderAt struct { delegate io.ReaderAt invocationCount int diff --git a/internal/repository/packer_manager.go b/internal/repository/packer_manager.go index 2c80f3572..491f888bc 100644 --- a/internal/repository/packer_manager.go +++ b/internal/repository/packer_manager.go @@ -57,7 +57,10 @@ func (r *packerManager) findPacker() (packer *Packer, err error) { // search for a suitable packer if len(r.packers) > 0 { p := r.packers[0] - r.packers = r.packers[1:] + last := len(r.packers) - 1 + r.packers[0] = r.packers[last] + r.packers[last] = nil // Allow GC of stale reference. + r.packers = r.packers[:last] return p, nil }