From a461ee4347dbf29e2fb5725e0101f31a58f7ed3a Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Tue, 31 Mar 2026 17:25:41 +0200 Subject: [PATCH 1/2] perf: improve release validation performance Avoid expensive calls to look for conflicts when the prefixes do not match. --- internal/setup/setup.go | 39 +++++++++++++++++++++++++++++++++--- internal/setup/setup_test.go | 20 ++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 6ed24abe..6b12e433 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -3,6 +3,7 @@ package setup import ( "errors" "fmt" + "maps" "os" "path/filepath" "slices" @@ -273,14 +274,43 @@ func (r *Release) validate() error { } // Check for glob and generate conflicts. + // + // This is the most time-consuming part of every command because checking + // each pair of paths uses an n^2 algorithm; specifically due to glob + // handling. We can speed up the process by exploiting the fact that + // conflicts are not allowed which means that in valid releases globs will + // appear at only the specific parts of the paths. There will always be a + // non-trivial prefix of the path that doesn't contain globs. + // + // We will speed up the search by only looking for conflicts in paths that + // share this same prefix. By collecting all paths and doing binary search + // we can find the start of the prefix. This is much simpler than other + // data structures like tries or radix trees which have similar algorithmic + // complexities. + allPaths := slices.Collect(maps.Keys(paths)) + slices.Sort(allPaths) for oldPath, oldSlices := range paths { for _, old := range oldSlices { oldInfo := old.Contents[oldPath] if oldInfo.Kind != GeneratePath && oldInfo.Kind != GlobPath { break } - for newPath, newSlices := range paths { - if oldPath == newPath { + + prefixLen := strings.IndexAny(oldPath, "*?") + searchKey := oldPath[:prefixLen] + // startIndex is the position of the prefix or the position where + // the prefix would have been found. + startIndex, _ := slices.BinarySearch(allPaths, searchKey) + for i := startIndex; i < len(allPaths); i++ { + newPath := allPaths[i] + if !strings.HasPrefix(newPath, searchKey) { + // Iterate until the prefix no longer matches, which means + // all other paths will fail the comparison below. + break + } + newSlices := paths[newPath] + // We know prefixes match, we can skip that part of the string. + if oldPath[len(searchKey)-1:] == newPath[len(searchKey)-1:] { // Identical paths have been filtered earlier. continue } @@ -291,13 +321,16 @@ func (r *Release) validate() error { continue } } - if strdist.GlobPath(newPath, oldPath) { + // We know prefixes match, we can skip that part of the string. + if strdist.GlobPath(newPath[len(searchKey)-1:], oldPath[len(searchKey)-1:]) { if (old.Package > new.Package) || (old.Package == new.Package && old.Name > new.Name) || (old.Package == new.Package && old.Name == new.Name && oldPath > newPath) { old, new = new, old oldPath, newPath = newPath, oldPath } return fmt.Errorf("slices %s and %s conflict on %s and %s", old, new, oldPath, newPath) + } else { + break } } } diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index d669943f..23f9a83d 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -518,6 +518,26 @@ var setupTests = []setupTest{{ `, }, relerror: "slices mypkg1_myslice1 and mypkg2_myslice1 conflict on /path1", +}, { + summary: "When multiple prefixes match all are tested", + input: map[string]string{ + "slices/mydir/mypkg1.yaml": ` + package: mypkg1 + slices: + myslice1: + contents: + /path/*/foo: + `, + "slices/mydir/mypkg2.yaml": ` + package: mypkg2 + slices: + myslice1: + contents: + /path/bar/foo1: + /path/bar/f*: + `, + }, + relerror: "slices mypkg1_myslice1 and mypkg2_myslice1 conflict on /path/\\*/foo and /path/bar/f\\*", }, { summary: "Directories must be suffixed with /", input: map[string]string{ From c2c1a9c831f05501ec7c65b4016c35947540e0c6 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Wed, 1 Apr 2026 13:10:53 +0200 Subject: [PATCH 2/2] internal error for invariant --- internal/setup/setup.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 6b12e433..23d0746f 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -297,6 +297,9 @@ func (r *Release) validate() error { } prefixLen := strings.IndexAny(oldPath, "*?") + if prefixLen == -1 { + return fmt.Errorf("internal error: invalid path: generate or glob path does not contain ' ?' or '*': %q", oldPath) + } searchKey := oldPath[:prefixLen] // startIndex is the position of the prefix or the position where // the prefix would have been found.