Merge pull request #3528 from greatroar/tree-insert

internal/restic: Don't allocate in Tree.Insert

cmd/restic/cmd_recover.go

@@ -107,7 +107,7 @@ func runRecover(gopts GlobalOptions) error {
 		return nil
 	}
 
-	tree := restic.NewTree()
+	tree := restic.NewTree(len(roots))
 	for id := range roots {
 		var subtreeID = id
 		node := restic.Node{

internal/archiver/archiver.go

@@ -550,12 +550,13 @@ func (arch *Archiver) statDir(dir string) (os.FileInfo, error) {
 func (arch *Archiver) SaveTree(ctx context.Context, snPath string, atree *Tree, previous *restic.Tree) (*restic.Tree, error) {
 	debug.Log("%v (%v nodes), parent %v", snPath, len(atree.Nodes), previous)
 
-	tree := restic.NewTree()
+	nodeNames := atree.NodeNames()
+	tree := restic.NewTree(len(nodeNames))
 
 	futureNodes := make(map[string]FutureNode)
 
 	// iterate over the nodes of atree in lexicographic (=deterministic) order
-	for _, name := range atree.NodeNames() {
+	for _, name := range nodeNames {
 		subatree := atree.Nodes[name]
 
 		// test if context has been cancelled

internal/archiver/tree_saver.go

@@ -103,7 +103,8 @@ type saveTreeResponse struct {
 func (s *TreeSaver) save(ctx context.Context, snPath string, node *restic.Node, nodes []FutureNode) (*restic.Node, ItemStats, error) {
 	var stats ItemStats
 
-	tree := restic.NewTree()
+	tree := restic.NewTree(len(nodes))
+
 	for _, fn := range nodes {
 		fn.wait(ctx)
 

internal/restic/tree.go

@@ -14,10 +14,10 @@ type Tree struct {
 	Nodes []*Node `json:"nodes"`
 }
 
-// NewTree creates a new tree object.
-func NewTree() *Tree {
+// NewTree creates a new tree object with the given initial capacity.
+func NewTree(capacity int) *Tree {
 	return &Tree{
-		Nodes: []*Node{},
+		Nodes: make([]*Node, 0, capacity),
 	}
 }
 
@@ -51,8 +51,8 @@ func (t *Tree) Insert(node *Node) error {
 		return errors.Errorf("node %q already present", node.Name)
 	}
 
-	// https://code.google.com/p/go-wiki/wiki/SliceTricks
-	t.Nodes = append(t.Nodes, &Node{})
+	// https://github.com/golang/go/wiki/SliceTricks
+	t.Nodes = append(t.Nodes, nil)
 	copy(t.Nodes[pos+1:], t.Nodes[pos:])
 	t.Nodes[pos] = node
 

internal/restic/tree_test.go

@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strconv"
 	"testing"
 
 	"github.com/restic/restic/internal/repository"
@@ -98,7 +99,7 @@ func TestLoadTree(t *testing.T) {
 	defer cleanup()
 
 	// save tree
-	tree := restic.NewTree()
+	tree := restic.NewTree(0)
 	id, err := repo.SaveTree(context.TODO(), tree)
 	rtest.OK(t, err)
 
@@ -113,3 +114,24 @@ func TestLoadTree(t *testing.T) {
 		"trees are not equal: want %v, got %v",
 		tree, tree2)
 }
+
+func BenchmarkBuildTree(b *testing.B) {
+	const size = 100 // Directories of this size are not uncommon.
+
+	nodes := make([]restic.Node, size)
+	for i := range nodes {
+		// Archiver.SaveTree inputs in sorted order, so do that here too.
+		nodes[i].Name = strconv.Itoa(i)
+	}
+
+	b.ResetTimer()
+	b.ReportAllocs()
+
+	for i := 0; i < b.N; i++ {
+		t := restic.NewTree(size)
+
+		for i := range nodes {
+			_ = t.Insert(&nodes[i])
+		}
+	}
+}

internal/walker/walker_test.go

@@ -23,7 +23,7 @@ func BuildTreeMap(tree TestTree) (m TreeMap, root restic.ID) {
 }
 
 func buildTreeMap(tree TestTree, m TreeMap) restic.ID {
-	res := restic.NewTree()
+	res := restic.NewTree(0)
 
 	for name, item := range tree {
 		switch elem := item.(type) {