From b5c28a7ba2b2dc133b40d9d13778b001531c8b2e Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Mon, 30 Sep 2024 22:43:04 +0200 Subject: [PATCH] internal/restic: Use IDSet.Clone + use maps package One place where IDSet.Clone is useful was reinventing it, using a conversion to list, a sort, and a conversion back to map. Also, use the stdlib "maps" package to implement as much of IDSet as possible. This requires changing one caller, which assumed that cloning nil would return a non-nil IDSet. --- internal/repository/index/master_index.go | 3 +++ internal/restic/idset.go | 33 +++++------------------ internal/restic/lock.go | 2 +- 3 files changed, 11 insertions(+), 27 deletions(-) diff --git a/internal/repository/index/master_index.go b/internal/repository/index/master_index.go index 2600fe350..ce9afcde4 100644 --- a/internal/repository/index/master_index.go +++ b/internal/repository/index/master_index.go @@ -347,6 +347,9 @@ func (mi *MasterIndex) Rewrite(ctx context.Context, repo restic.Unpacked, exclud // copy excludePacks to prevent unintended sideeffects excludePacks = excludePacks.Clone() + if excludePacks == nil { + excludePacks = restic.NewIDSet() + } debug.Log("start rebuilding index of %d indexes, excludePacks: %v", len(indexes), excludePacks) wg, wgCtx := errgroup.WithContext(ctx) diff --git a/internal/restic/idset.go b/internal/restic/idset.go index 9e6e3c6fd..7d98b487c 100644 --- a/internal/restic/idset.go +++ b/internal/restic/idset.go @@ -1,6 +1,9 @@ package restic -import "sort" +import ( + "maps" + "sort" +) // IDSet is a set of IDs. type IDSet map[ID]struct{} @@ -44,28 +47,10 @@ func (s IDSet) List() IDs { } // Equals returns true iff s equals other. -func (s IDSet) Equals(other IDSet) bool { - if len(s) != len(other) { - return false - } - - for id := range s { - if _, ok := other[id]; !ok { - return false - } - } - - // length + one-way comparison is sufficient implication of equality - - return true -} +func (s IDSet) Equals(other IDSet) bool { return maps.Equal(s, other) } // Merge adds the blobs in other to the current set. -func (s IDSet) Merge(other IDSet) { - for id := range other { - s.Insert(id) - } -} +func (s IDSet) Merge(other IDSet) { maps.Copy(s, other) } // Intersect returns a new set containing the IDs that are present in both sets. func (s IDSet) Intersect(other IDSet) (result IDSet) { @@ -106,8 +91,4 @@ func (s IDSet) String() string { return "{" + str[1:len(str)-1] + "}" } -func (s IDSet) Clone() IDSet { - c := NewIDSet() - c.Merge(s) - return c -} +func (s IDSet) Clone() IDSet { return maps.Clone(s) } diff --git a/internal/restic/lock.go b/internal/restic/lock.go index 969d0593d..8ad84091a 100644 --- a/internal/restic/lock.go +++ b/internal/restic/lock.go @@ -187,7 +187,7 @@ func (l *Lock) checkForOtherLocks(ctx context.Context) error { // Store updates in new IDSet to prevent data races var m sync.Mutex - newCheckedIDs := NewIDSet(checkedIDs.List()...) + newCheckedIDs := checkedIDs.Clone() err = ForAllLocks(ctx, l.repo, checkedIDs, func(id ID, lock *Lock, err error) error { if err != nil { // if we cannot load a lock then it is unclear whether it can be ignored