Merge pull request #766 from aptly-dev/761-more-lazy

Reimplement DB collections for mirrors, repos and snapshots

Makefile

@@ -13,7 +13,7 @@ RUN_LONG_TESTS?=yes
 
 GO_1_10_AND_HIGHER=$(shell (printf '%s\n' go1.10 $(GOVERSION) | sort -cV >/dev/null 2>&1) && echo "yes")
 
-all: test check system-test
+all: test bench check system-test
 
 prepare:
 	go get -u github.com/alecthomas/gometalinter
@@ -57,6 +57,9 @@ else
 	go test -v `go list ./... | grep -v vendor/` -gocheck.v=true
 endif
 
+bench:
+	go test -v ./deb -run=nothing -bench=. -benchmem 
+
 mem.png: mem.dat mem.gp
 	gnuplot mem.gp
 	open mem.png

deb/local.go

@@ -2,6 +2,7 @@ package deb
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"log"
 	"sync"
@@ -93,8 +94,8 @@ func (repo *LocalRepo) RefKey() []byte {
 // LocalRepoCollection does listing, updating/adding/deleting of LocalRepos
 type LocalRepoCollection struct {
 	*sync.RWMutex
-	db   database.Storage
-	list []*LocalRepo
+	db    database.Storage
+	cache map[string]*LocalRepo
 }
 
 // NewLocalRepoCollection loads LocalRepos from DB and makes up collection
@@ -102,43 +103,59 @@ func NewLocalRepoCollection(db database.Storage) *LocalRepoCollection {
 	return &LocalRepoCollection{
 		RWMutex: &sync.RWMutex{},
 		db:      db,
+		cache:   make(map[string]*LocalRepo),
 	}
 }
 
-func (collection *LocalRepoCollection) loadList() {
-	if collection.list != nil {
-		return
-	}
-
-	blobs := collection.db.FetchByPrefix([]byte("L"))
-	collection.list = make([]*LocalRepo, 0, len(blobs))
-
-	for _, blob := range blobs {
-		r := &LocalRepo{}
-		if err := r.Decode(blob); err != nil {
-			log.Printf("Error decoding repo: %s\n", err)
-		} else {
-			collection.list = append(collection.list, r)
+func (collection *LocalRepoCollection) search(filter func(*LocalRepo) bool, unique bool) []*LocalRepo {
+	result := []*LocalRepo(nil)
+	for _, r := range collection.cache {
+		if filter(r) {
+			result = append(result, r)
 		}
 	}
+
+	if unique && len(result) > 0 {
+		return result
+	}
+
+	collection.db.ProcessByPrefix([]byte("L"), func(key, blob []byte) error {
+		r := &LocalRepo{}
+		if err := r.Decode(blob); err != nil {
+			log.Printf("Error decoding local repo: %s\n", err)
+			return nil
+		}
+
+		if filter(r) {
+			if _, exists := collection.cache[r.UUID]; !exists {
+				collection.cache[r.UUID] = r
+				result = append(result, r)
+				if unique {
+					return errors.New("abort")
+				}
+			}
+		}
+
+		return nil
+	})
+
+	return result
 }
 
 // Add appends new repo to collection and saves it
 func (collection *LocalRepoCollection) Add(repo *LocalRepo) error {
-	collection.loadList()
+	_, err := collection.ByName(repo.Name)
 
-	for _, r := range collection.list {
-		if r.Name == repo.Name {
-			return fmt.Errorf("local repo with name %s already exists", repo.Name)
-		}
+	if err == nil {
+		return fmt.Errorf("local repo with name %s already exists", repo.Name)
 	}
 
-	err := collection.Update(repo)
+	err = collection.Update(repo)
 	if err != nil {
 		return err
 	}
 
-	collection.list = append(collection.list, repo)
+	collection.cache[repo.UUID] = repo
 	return nil
 }
 
@@ -159,8 +176,6 @@ func (collection *LocalRepoCollection) Update(repo *LocalRepo) error {
 
 // LoadComplete loads additional information for local repo
 func (collection *LocalRepoCollection) LoadComplete(repo *LocalRepo) error {
-	collection.loadList()
-
 	encoded, err := collection.db.Get(repo.RefKey())
 	if err == database.ErrNotFound {
 		return nil
@@ -175,26 +190,39 @@ func (collection *LocalRepoCollection) LoadComplete(repo *LocalRepo) error {
 
 // ByName looks up repository by name
 func (collection *LocalRepoCollection) ByName(name string) (*LocalRepo, error) {
-	collection.loadList()
-
-	for _, r := range collection.list {
-		if r.Name == name {
-			return r, nil
-		}
+	result := collection.search(func(r *LocalRepo) bool { return r.Name == name }, true)
+	if len(result) == 0 {
+		return nil, fmt.Errorf("local repo with name %s not found", name)
 	}
-	return nil, fmt.Errorf("local repo with name %s not found", name)
+
+	return result[0], nil
 }
 
 // ByUUID looks up repository by uuid
 func (collection *LocalRepoCollection) ByUUID(uuid string) (*LocalRepo, error) {
-	collection.loadList()
-
-	for _, r := range collection.list {
-		if r.UUID == uuid {
-			return r, nil
-		}
+	if r, ok := collection.cache[uuid]; ok {
+		return r, nil
 	}
-	return nil, fmt.Errorf("local repo with uuid %s not found", uuid)
+
+	key := (&LocalRepo{UUID: uuid}).Key()
+
+	value, err := collection.db.Get(key)
+	if err == database.ErrNotFound {
+		return nil, fmt.Errorf("local repo with uuid %s not found", uuid)
+	}
+
+	if err != nil {
+		return nil, err
+	}
+
+	r := &LocalRepo{}
+	err = r.Decode(value)
+
+	if err == nil {
+		collection.cache[r.UUID] = r
+	}
+
+	return r, err
 }
 
 // ForEach runs method for each repository
@@ -212,30 +240,16 @@ func (collection *LocalRepoCollection) ForEach(handler func(*LocalRepo) error) e
 
 // Len returns number of remote repos
 func (collection *LocalRepoCollection) Len() int {
-	collection.loadList()
-
-	return len(collection.list)
+	return len(collection.db.KeysByPrefix([]byte("L")))
 }
 
 // Drop removes remote repo from collection
 func (collection *LocalRepoCollection) Drop(repo *LocalRepo) error {
-	collection.loadList()
-
-	repoPosition := -1
-
-	for i, r := range collection.list {
-		if r == repo {
-			repoPosition = i
-			break
-		}
-	}
-
-	if repoPosition == -1 {
+	if _, err := collection.db.Get(repo.Key()); err == database.ErrNotFound {
 		panic("local repo not found!")
 	}
 
-	collection.list[len(collection.list)-1], collection.list[repoPosition], collection.list =
-		nil, collection.list[len(collection.list)-1], collection.list[:len(collection.list)-1]
+	delete(collection.cache, repo.UUID)
 
 	err := collection.db.Delete(repo.Key())
 	if err != nil {

deb/local_test.go

@@ -124,6 +124,11 @@ func (s *LocalRepoCollectionSuite) TestByUUID(c *C) {
 
 	r, err := s.collection.ByUUID(repo.UUID)
 	c.Assert(err, IsNil)
+	c.Assert(r, Equals, repo)
+
+	collection := NewLocalRepoCollection(s.db)
+	r, err = collection.ByUUID(repo.UUID)
+	c.Assert(err, IsNil)
 	c.Assert(r.String(), Equals, repo.String())
 }
 

deb/remote.go

@@ -3,6 +3,7 @@ package deb
 import (
 	"bytes"
 	gocontext "context"
+	"errors"
 	"fmt"
 	"log"
 	"net/url"
@@ -654,8 +655,8 @@ func (repo *RemoteRepo) RefKey() []byte {
 // RemoteRepoCollection does listing, updating/adding/deleting of RemoteRepos
 type RemoteRepoCollection struct {
 	*sync.RWMutex
-	db   database.Storage
-	list []*RemoteRepo
+	db    database.Storage
+	cache map[string]*RemoteRepo
 }
 
 // NewRemoteRepoCollection loads RemoteRepos from DB and makes up collection
@@ -663,43 +664,59 @@ func NewRemoteRepoCollection(db database.Storage) *RemoteRepoCollection {
 	return &RemoteRepoCollection{
 		RWMutex: &sync.RWMutex{},
 		db:      db,
+		cache:   make(map[string]*RemoteRepo),
 	}
 }
 
-func (collection *RemoteRepoCollection) loadList() {
-	if collection.list != nil {
-		return
-	}
-
-	blobs := collection.db.FetchByPrefix([]byte("R"))
-	collection.list = make([]*RemoteRepo, 0, len(blobs))
-
-	for _, blob := range blobs {
-		r := &RemoteRepo{}
-		if err := r.Decode(blob); err != nil {
-			log.Printf("Error decoding mirror: %s\n", err)
-		} else {
-			collection.list = append(collection.list, r)
+func (collection *RemoteRepoCollection) search(filter func(*RemoteRepo) bool, unique bool) []*RemoteRepo {
+	result := []*RemoteRepo(nil)
+	for _, r := range collection.cache {
+		if filter(r) {
+			result = append(result, r)
 		}
 	}
+
+	if unique && len(result) > 0 {
+		return result
+	}
+
+	collection.db.ProcessByPrefix([]byte("R"), func(key, blob []byte) error {
+		r := &RemoteRepo{}
+		if err := r.Decode(blob); err != nil {
+			log.Printf("Error decoding remote repo: %s\n", err)
+			return nil
+		}
+
+		if filter(r) {
+			if _, exists := collection.cache[r.UUID]; !exists {
+				collection.cache[r.UUID] = r
+				result = append(result, r)
+				if unique {
+					return errors.New("abort")
+				}
+			}
+		}
+
+		return nil
+	})
+
+	return result
 }
 
 // Add appends new repo to collection and saves it
 func (collection *RemoteRepoCollection) Add(repo *RemoteRepo) error {
-	collection.loadList()
+	_, err := collection.ByName(repo.Name)
 
-	for _, r := range collection.list {
-		if r.Name == repo.Name {
-			return fmt.Errorf("mirror with name %s already exists", repo.Name)
-		}
+	if err == nil {
+		return fmt.Errorf("mirror with name %s already exists", repo.Name)
 	}
 
-	err := collection.Update(repo)
+	err = collection.Update(repo)
 	if err != nil {
 		return err
 	}
 
-	collection.list = append(collection.list, repo)
+	collection.cache[repo.UUID] = repo
 	return nil
 }
 
@@ -734,26 +751,38 @@ func (collection *RemoteRepoCollection) LoadComplete(repo *RemoteRepo) error {
 
 // ByName looks up repository by name
 func (collection *RemoteRepoCollection) ByName(name string) (*RemoteRepo, error) {
-	collection.loadList()
-
-	for _, r := range collection.list {
-		if r.Name == name {
-			return r, nil
-		}
+	result := collection.search(func(r *RemoteRepo) bool { return r.Name == name }, true)
+	if len(result) == 0 {
+		return nil, fmt.Errorf("mirror with name %s not found", name)
 	}
-	return nil, fmt.Errorf("mirror with name %s not found", name)
+
+	return result[0], nil
 }
 
 // ByUUID looks up repository by uuid
 func (collection *RemoteRepoCollection) ByUUID(uuid string) (*RemoteRepo, error) {
-	collection.loadList()
-
-	for _, r := range collection.list {
-		if r.UUID == uuid {
-			return r, nil
-		}
+	if r, ok := collection.cache[uuid]; ok {
+		return r, nil
 	}
-	return nil, fmt.Errorf("mirror with uuid %s not found", uuid)
+
+	key := (&RemoteRepo{UUID: uuid}).Key()
+
+	value, err := collection.db.Get(key)
+	if err == database.ErrNotFound {
+		return nil, fmt.Errorf("mirror with uuid %s not found", uuid)
+	}
+	if err != nil {
+		return nil, err
+	}
+
+	r := &RemoteRepo{}
+	err = r.Decode(value)
+
+	if err == nil {
+		collection.cache[r.UUID] = r
+	}
+
+	return r, err
 }
 
 // ForEach runs method for each repository
@@ -771,30 +800,16 @@ func (collection *RemoteRepoCollection) ForEach(handler func(*RemoteRepo) error)
 
 // Len returns number of remote repos
 func (collection *RemoteRepoCollection) Len() int {
-	collection.loadList()
-
-	return len(collection.list)
+	return len(collection.db.KeysByPrefix([]byte("R")))
 }
 
 // Drop removes remote repo from collection
 func (collection *RemoteRepoCollection) Drop(repo *RemoteRepo) error {
-	collection.loadList()
-
-	repoPosition := -1
-
-	for i, r := range collection.list {
-		if r == repo {
-			repoPosition = i
-			break
-		}
-	}
-
-	if repoPosition == -1 {
+	if _, err := collection.db.Get(repo.Key()); err == database.ErrNotFound {
 		panic("repo not found!")
 	}
 
-	collection.list[len(collection.list)-1], collection.list[repoPosition], collection.list =
-		nil, collection.list[len(collection.list)-1], collection.list[:len(collection.list)-1]
+	delete(collection.cache, repo.UUID)
 
 	err := collection.db.Delete(repo.Key())
 	if err != nil {

deb/remote_test.go

@@ -651,6 +651,11 @@ func (s *RemoteRepoCollectionSuite) TestByUUID(c *C) {
 
 	r, err := s.collection.ByUUID(repo.UUID)
 	c.Assert(err, IsNil)
+	c.Assert(r, Equals, repo)
+
+	collection := NewRemoteRepoCollection(s.db)
+	r, err = collection.ByUUID(repo.UUID)
+	c.Assert(err, IsNil)
 	c.Assert(r.String(), Equals, repo.String())
 }
 

deb/snapshot.go

@@ -173,8 +173,8 @@ func (s *Snapshot) Decode(input []byte) error {
 // SnapshotCollection does listing, updating/adding/deleting of Snapshots
 type SnapshotCollection struct {
 	*sync.RWMutex
-	db   database.Storage
-	list []*Snapshot
+	db    database.Storage
+	cache map[string]*Snapshot
 }
 
 // NewSnapshotCollection loads Snapshots from DB and makes up collection
@@ -182,43 +182,23 @@ func NewSnapshotCollection(db database.Storage) *SnapshotCollection {
 	return &SnapshotCollection{
 		RWMutex: &sync.RWMutex{},
 		db:      db,
-	}
-}
-
-func (collection *SnapshotCollection) loadList() {
-	if collection.list != nil {
-		return
-	}
-
-	blobs := collection.db.FetchByPrefix([]byte("S"))
-	collection.list = make([]*Snapshot, 0, len(blobs))
-
-	for _, blob := range blobs {
-		s := &Snapshot{}
-		if err := s.Decode(blob); err != nil {
-			log.Printf("Error decoding snapshot: %s\n", err)
-		} else {
-			collection.list = append(collection.list, s)
-		}
+		cache:   map[string]*Snapshot{},
 	}
 }
 
 // Add appends new repo to collection and saves it
 func (collection *SnapshotCollection) Add(snapshot *Snapshot) error {
-	collection.loadList()
-
-	for _, s := range collection.list {
-		if s.Name == snapshot.Name {
-			return fmt.Errorf("snapshot with name %s already exists", snapshot.Name)
-		}
+	_, err := collection.ByName(snapshot.Name)
+	if err == nil {
+		return fmt.Errorf("snapshot with name %s already exists", snapshot.Name)
 	}
 
-	err := collection.Update(snapshot)
+	err = collection.Update(snapshot)
 	if err != nil {
 		return err
 	}
 
-	collection.list = append(collection.list, snapshot)
+	collection.cache[snapshot.UUID] = snapshot
 	return nil
 }
 
@@ -245,70 +225,96 @@ func (collection *SnapshotCollection) LoadComplete(snapshot *Snapshot) error {
 	return snapshot.packageRefs.Decode(encoded)
 }
 
-// ByName looks up snapshot by name
-func (collection *SnapshotCollection) ByName(name string) (*Snapshot, error) {
-	collection.loadList()
-
-	for _, s := range collection.list {
-		if s.Name == name {
-			return s, nil
+func (collection *SnapshotCollection) search(filter func(*Snapshot) bool, unique bool) []*Snapshot {
+	result := []*Snapshot(nil)
+	for _, s := range collection.cache {
+		if filter(s) {
+			result = append(result, s)
 		}
 	}
+
+	if unique && len(result) > 0 {
+		return result
+	}
+
+	collection.db.ProcessByPrefix([]byte("S"), func(key, blob []byte) error {
+		s := &Snapshot{}
+		if err := s.Decode(blob); err != nil {
+			log.Printf("Error decoding snapshot: %s\n", err)
+			return nil
+		}
+
+		if filter(s) {
+			if _, exists := collection.cache[s.UUID]; !exists {
+				collection.cache[s.UUID] = s
+				result = append(result, s)
+				if unique {
+					return errors.New("abort")
+				}
+			}
+		}
+
+		return nil
+	})
+
+	return result
+}
+
+// ByName looks up snapshot by name
+func (collection *SnapshotCollection) ByName(name string) (*Snapshot, error) {
+	result := collection.search(func(s *Snapshot) bool { return s.Name == name }, true)
+	if len(result) > 0 {
+		return result[0], nil
+	}
+
 	return nil, fmt.Errorf("snapshot with name %s not found", name)
 }
 
 // ByUUID looks up snapshot by UUID
 func (collection *SnapshotCollection) ByUUID(uuid string) (*Snapshot, error) {
-	collection.loadList()
-
-	for _, s := range collection.list {
-		if s.UUID == uuid {
-			return s, nil
-		}
+	if s, ok := collection.cache[uuid]; ok {
+		return s, nil
 	}
-	return nil, fmt.Errorf("snapshot with uuid %s not found", uuid)
+
+	key := (&Snapshot{UUID: uuid}).Key()
+
+	value, err := collection.db.Get(key)
+	if err == database.ErrNotFound {
+		return nil, fmt.Errorf("snapshot with uuid %s not found", uuid)
+	}
+	if err != nil {
+		return nil, err
+	}
+
+	s := &Snapshot{}
+	err = s.Decode(value)
+
+	if err == nil {
+		collection.cache[s.UUID] = s
+	}
+
+	return s, err
 }
 
 // ByRemoteRepoSource looks up snapshots that have specified RemoteRepo as a source
 func (collection *SnapshotCollection) ByRemoteRepoSource(repo *RemoteRepo) []*Snapshot {
-	collection.loadList()
-
-	var result []*Snapshot
-
-	for _, s := range collection.list {
-		if s.SourceKind == SourceRemoteRepo && utils.StrSliceHasItem(s.SourceIDs, repo.UUID) {
-			result = append(result, s)
-		}
-	}
-	return result
+	return collection.search(func(s *Snapshot) bool {
+		return s.SourceKind == SourceRemoteRepo && utils.StrSliceHasItem(s.SourceIDs, repo.UUID)
+	}, false)
 }
 
 // ByLocalRepoSource looks up snapshots that have specified LocalRepo as a source
 func (collection *SnapshotCollection) ByLocalRepoSource(repo *LocalRepo) []*Snapshot {
-	collection.loadList()
-
-	var result []*Snapshot
-
-	for _, s := range collection.list {
-		if s.SourceKind == SourceLocalRepo && utils.StrSliceHasItem(s.SourceIDs, repo.UUID) {
-			result = append(result, s)
-		}
-	}
-	return result
+	return collection.search(func(s *Snapshot) bool {
+		return s.SourceKind == SourceLocalRepo && utils.StrSliceHasItem(s.SourceIDs, repo.UUID)
+	}, false)
 }
 
 // BySnapshotSource looks up snapshots that have specified snapshot as a source
 func (collection *SnapshotCollection) BySnapshotSource(snapshot *Snapshot) []*Snapshot {
-	collection.loadList()
-
-	var result []*Snapshot
-
-	for _, s := range collection.list {
-		if s.SourceKind == "snapshot" && utils.StrSliceHasItem(s.SourceIDs, snapshot.UUID) {
-			result = append(result, s)
-		}
-	}
-	return result
+	return collection.search(func(s *Snapshot) bool {
+		return s.SourceKind == "snapshot" && utils.StrSliceHasItem(s.SourceIDs, snapshot.UUID)
+	}, false)
 }
 
 // ForEach runs method for each snapshot
@@ -326,15 +332,25 @@ func (collection *SnapshotCollection) ForEach(handler func(*Snapshot) error) err
 
 // ForEachSorted runs method for each snapshot following some sort order
 func (collection *SnapshotCollection) ForEachSorted(sortMethod string, handler func(*Snapshot) error) error {
-	collection.loadList()
+	blobs := collection.db.FetchByPrefix([]byte("S"))
+	list := make([]*Snapshot, 0, len(blobs))
 
-	sorter, err := newSnapshotSorter(sortMethod, collection)
+	for _, blob := range blobs {
+		s := &Snapshot{}
+		if err := s.Decode(blob); err != nil {
+			log.Printf("Error decoding snapshot: %s\n", err)
+		} else {
+			list = append(list, s)
+		}
+	}
+
+	sorter, err := newSnapshotSorter(sortMethod, list)
 	if err != nil {
 		return err
 	}
 
-	for _, i := range sorter.list {
-		err = handler(collection.list[i])
+	for _, s := range sorter.list {
+		err = handler(s)
 		if err != nil {
 			return err
 		}
@@ -346,30 +362,16 @@ func (collection *SnapshotCollection) ForEachSorted(sortMethod string, handler f
 // Len returns number of snapshots in collection
 // ForEach runs method for each snapshot
 func (collection *SnapshotCollection) Len() int {
-	collection.loadList()
-
-	return len(collection.list)
+	return len(collection.db.KeysByPrefix([]byte("S")))
 }
 
 // Drop removes snapshot from collection
 func (collection *SnapshotCollection) Drop(snapshot *Snapshot) error {
-	collection.loadList()
-
-	snapshotPosition := -1
-
-	for i, s := range collection.list {
-		if s == snapshot {
-			snapshotPosition = i
-			break
-		}
-	}
-
-	if snapshotPosition == -1 {
+	if _, err := collection.db.Get(snapshot.Key()); err == database.ErrNotFound {
 		panic("snapshot not found!")
 	}
 
-	collection.list[len(collection.list)-1], collection.list[snapshotPosition], collection.list =
-		nil, collection.list[len(collection.list)-1], collection.list[:len(collection.list)-1]
+	delete(collection.cache, snapshot.UUID)
 
 	err := collection.db.Delete(snapshot.Key())
 	if err != nil {
@@ -386,13 +388,12 @@ const (
 )
 
 type snapshotSorter struct {
-	list       []int
-	collection *SnapshotCollection
+	list       []*Snapshot
 	sortMethod int
 }
 
-func newSnapshotSorter(sortMethod string, collection *SnapshotCollection) (*snapshotSorter, error) {
-	s := &snapshotSorter{collection: collection}
+func newSnapshotSorter(sortMethod string, list []*Snapshot) (*snapshotSorter, error) {
+	s := &snapshotSorter{list: list}
 
 	switch sortMethod {
 	case "time", "Time":
@@ -403,11 +404,6 @@ func newSnapshotSorter(sortMethod string, collection *SnapshotCollection) (*snap
 		return nil, fmt.Errorf("sorting method \"%s\" unknown", sortMethod)
 	}
 
-	s.list = make([]int, len(collection.list))
-	for i := range s.list {
-		s.list[i] = i
-	}
-
 	sort.Sort(s)
 
 	return s, nil
@@ -420,9 +416,9 @@ func (s *snapshotSorter) Swap(i, j int) {
 func (s *snapshotSorter) Less(i, j int) bool {
 	switch s.sortMethod {
 	case SortName:
-		return s.collection.list[s.list[i]].Name < s.collection.list[s.list[j]].Name
+		return s.list[i].Name < s.list[j].Name
 	case SortTime:
-		return s.collection.list[s.list[i]].CreatedAt.Before(s.collection.list[s.list[j]].CreatedAt)
+		return s.list[i].CreatedAt.Before(s.list[j].CreatedAt)
 	}
 	panic("unknown sort method")
 }

deb/snapshot_bench_test.go

@@ -36,3 +36,63 @@ func BenchmarkSnapshotCollectionForEach(b *testing.B) {
 		})
 	}
 }
+
+func BenchmarkSnapshotCollectionByUUID(b *testing.B) {
+	const count = 1024
+
+	tmpDir := os.TempDir()
+	defer os.RemoveAll(tmpDir)
+
+	db, _ := database.NewOpenDB(tmpDir)
+	defer db.Close()
+
+	collection := NewSnapshotCollection(db)
+
+	uuids := []string{}
+	for i := 0; i < count; i++ {
+		snapshot := NewSnapshotFromRefList(fmt.Sprintf("snapshot%d", i), nil, NewPackageRefList(), fmt.Sprintf("Snapshot number %d", i))
+		if collection.Add(snapshot) != nil {
+			b.FailNow()
+		}
+		uuids = append(uuids, snapshot.UUID)
+	}
+
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		collection = NewSnapshotCollection(db)
+
+		if _, err := collection.ByUUID(uuids[i%len(uuids)]); err != nil {
+			b.FailNow()
+		}
+	}
+}
+
+func BenchmarkSnapshotCollectionByName(b *testing.B) {
+	const count = 1024
+
+	tmpDir := os.TempDir()
+	defer os.RemoveAll(tmpDir)
+
+	db, _ := database.NewOpenDB(tmpDir)
+	defer db.Close()
+
+	collection := NewSnapshotCollection(db)
+
+	for i := 0; i < count; i++ {
+		snapshot := NewSnapshotFromRefList(fmt.Sprintf("snapshot%d", i), nil, NewPackageRefList(), fmt.Sprintf("Snapshot number %d", i))
+		if collection.Add(snapshot) != nil {
+			b.FailNow()
+		}
+	}
+
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		collection = NewSnapshotCollection(db)
+
+		if _, err := collection.ByName(fmt.Sprintf("snapshot%d", i%count)); err != nil {
+			b.FailNow()
+		}
+	}
+}

deb/snapshot_test.go

@@ -2,6 +2,7 @@ package deb
 
 import (
 	"errors"
+	"sort"
 
 	"github.com/aptly-dev/aptly/database"
 
@@ -158,6 +159,10 @@ func (s *SnapshotCollectionSuite) TestAddByNameByUUID(c *C) {
 	snapshot, err = collection.ByUUID(s.snapshot1.UUID)
 	c.Assert(err, IsNil)
 	c.Assert(snapshot.String(), Equals, s.snapshot1.String())
+
+	snapshot, err = collection.ByUUID(s.snapshot2.UUID)
+	c.Assert(err, IsNil)
+	c.Assert(snapshot.String(), Equals, s.snapshot2.String())
 }
 
 func (s *SnapshotCollectionSuite) TestUpdateLoadComplete(c *C) {
@@ -193,6 +198,23 @@ func (s *SnapshotCollectionSuite) TestForEachAndLen(c *C) {
 	c.Assert(err, Equals, e)
 }
 
+func (s *SnapshotCollectionSuite) TestForEachSorted(c *C) {
+	s.collection.Add(s.snapshot2)
+	s.collection.Add(s.snapshot1)
+	s.collection.Add(s.snapshot4)
+	s.collection.Add(s.snapshot3)
+
+	names := []string{}
+
+	err := s.collection.ForEachSorted("name", func(snapshot *Snapshot) error {
+		names = append(names, snapshot.Name)
+		return nil
+	})
+	c.Assert(err, IsNil)
+
+	c.Check(sort.StringsAreSorted(names), Equals, true)
+}
+
 func (s *SnapshotCollectionSuite) TestFindByRemoteRepoSource(c *C) {
 	c.Assert(s.collection.Add(s.snapshot1), IsNil)
 	c.Assert(s.collection.Add(s.snapshot2), IsNil)
@@ -230,7 +252,11 @@ func (s *SnapshotCollectionSuite) TestFindSnapshotSource(c *C) {
 	c.Assert(s.collection.Add(snapshot4), IsNil)
 	c.Assert(s.collection.Add(snapshot5), IsNil)
 
-	c.Check(s.collection.BySnapshotSource(s.snapshot1), DeepEquals, []*Snapshot{snapshot3, snapshot4})
+	list := s.collection.BySnapshotSource(s.snapshot1)
+	sorter, _ := newSnapshotSorter("name", list)
+	sort.Sort(sorter)
+
+	c.Check(sorter.list, DeepEquals, []*Snapshot{snapshot3, snapshot4})
 	c.Check(s.collection.BySnapshotSource(s.snapshot2), DeepEquals, []*Snapshot{snapshot3})
 	c.Check(s.collection.BySnapshotSource(snapshot5), DeepEquals, []*Snapshot(nil))
 }