Skip to content

perf: speed up path conflict detection #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lczyk wants to merge 2 commits into
base: performance-tree
Choose a base branch
from
Open

perf: speed up path conflict detection #29

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f449d7a
perf: reuse conflict traversal queues
lczyk Jul 10, 2026
3c097a7
perf: exact-match child lookup in traversal
lczyk Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 41 additions & 6 deletions internal/setup/conflict.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ type node struct {
Segment segment
SegmentSlices []*segmentSlice
Children map[string]*node
// GlobChildren lists the children whose segment contains a wildcard, so

@letFunny letFunny Jul 13, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very interesting. I didn't expect such a simple change to have 25% improvement (my machine). I had done another more complex optimization for looking for candidates that had the same % improvement but the implementation was far worse.

I think we should go for this one.

// that a literal segment can find its exact match in Children and only
// scan these.
GlobChildren []*node
}

// pathConflictTree uses a custom trie to find conflicts that might arise from
Expand All @@ -52,6 +56,10 @@ type node struct {
type pathConflictTree struct {
Root *node
PathToSlices map[string][]*Slice
// currentQueue and nextQueue are kept across pathHasConflict calls so

@letFunny letFunny Jul 13, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When writing the original PR I cached the queues, then I removed it, then added it back, etc. and in the end I don't have a strong opinion on whether to do it or not. It obviously increases performance (around 20% on my machine, so 20ms out of 100ms). The only reason I didn't add it from the get go was a similar case in the past where there was an optimization by caching the results and Gustavo argued against it as it increased the complexity.

After seeing you came up with the same idea I think we should start the debate again.

// that the traversal does not have to grow fresh queues for every path.
currentQueue []*node
nextQueue []*node
}

var rootSegment = segment{"/", false, false}
Expand Down Expand Up @@ -100,12 +108,19 @@ func (g *pathConflictTree) pathHasConflict(newSegments []segment, newSegmentSlic
return fmt.Errorf("slices %s and %s conflict on %s and %s", oldSlice, newSlice, oldPath, newPath)
}

var currentQueue []*node
var nextQueue []*node
currentQueue := g.currentQueue[:0]
nextQueue := g.nextQueue[:0]
defer func() {
// Keep the grown queues for the next call.
g.currentQueue, g.nextQueue = currentQueue, nextQueue
}()

// Skip "/".
currentQueue = slices.Collect(maps.Values(g.Root.Children))
newSegments = newSegments[1:]
if len(newSegments) == 0 {
return nil
}
currentQueue = appendCandidates(currentQueue, g.Root, newSegments[0])

// If we run out of segments from the graph or the path there cannot be a
// conflict.
Expand Down Expand Up @@ -152,8 +167,8 @@ func (g *pathConflictTree) pathHasConflict(newSegments []segment, newSegmentSlic
// If we are at the terminal node of both paths we found a conflict.
return conflictErrMsg(oldSegmentSlice, newSegmentSlice)
}
for _, child := range oldNode.Children {
nextQueue = append(nextQueue, child)
if len(newSegments) > 1 {
nextQueue = appendCandidates(nextQueue, oldNode, newSegments[1])
}
break oldNodeLoop
} else {
Expand All @@ -174,6 +189,23 @@ func (g *pathConflictTree) pathHasConflict(newSegments []segment, newSegmentSlic
return nil
}

// appendCandidates appends the children of parent that can possibly match
// seg. A segment with a wildcard can match any child, but a literal segment
// can only match the child with the exact same text or children with
// wildcards.
func appendCandidates(queue []*node, parent *node, seg segment) []*node {
if seg.HasGlob {
for _, child := range parent.Children {
queue = append(queue, child)
}
return queue
}
if child, ok := parent.Children[seg.Text]; ok {
queue = append(queue, child)
}
return append(queue, parent.GlobChildren...)
}

// insertSegments inserts the path's segments blindly in the graph without
// looking at conflicts.
func (g *pathConflictTree) insertSegments(segments []segment, segmentSlices []*segmentSlice) {
Expand All @@ -188,9 +220,12 @@ func (g *pathConflictTree) insertSegments(segments []segment, segmentSlices []*s
Segment: segment,
Children: map[string]*node{},
}
parent.Children[segment.Text] = current
if segment.HasGlob {
parent.GlobChildren = append(parent.GlobChildren, current)
}
}
current.SegmentSlices = append(current.SegmentSlices, segmentSlices...)
parent.Children[segment.Text] = current
parent = current
}
}
Expand Down