bundle: Update bundle roots conflict detection algorithm.#8664
Merged
philipaconrad merged 1 commit intoMay 18, 2026
Merged
Conversation
This commit is a change of algorithm for how we detect conflicts between the roots of multiple bundles. The old algorithm was an O(N^2) all-to-all root paths comparison. The new algorithm changes this to a string sorting and scanning process, which results in O(N log N) comparisons, and fewer string split operations. The benchmarks included with this commit indicate a meaningful improvement across all cases tested, with even the pathological cases showing massive improvement in runtime and memory usage/allocs. Signed-off-by: Philip Conrad <philip@chariot-chaser.net>
Benchmark Regression DetectedCommit
This comment was automatically generated by the benchmarks workflow. |
Contributor
|
Still too noisy/brittle. But at least we know the other benchmarks are all within the threshold, which is nice! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What code changed in this PR?
This commit is a change of algorithm for how we detect conflicts between the roots of multiple bundles. The old algorithm was an O(N^2) all-to-all root paths comparison. The new algorithm changes this to a string sorting and scanning process, which results in O(N log N) string comparisons, and fewer string split operations.
The benchmarks included with this commit indicate a meaningful improvement across nearly all cases tested, with even the pathological cases showing massive improvement in runtime and memory usage/allocs.
The algorithm here is a backport of the one I figured out for Swift OPA, over in open-policy-agent/swift-opa#110
Notes for Reviewers
v1/bundle/store.goare comments explaining what's going on for each step, so that it's hopefully clear why the new algorithm works the same as the previous one, despite doing the actual conflict detection work very differently.How to test?
v1/bundle/store_bench_test.goto exercise the conflict detection algorithm. The differences are stark (some cases as much as a 95%+ speedup over the original algorithm), and scales as bundle counts loaded on the OPA instance increase.Benchmark Results
benchstatresults (Click to expand)On a 2024 M4 MacBook Pro this is what I got from running
benchstaton the results. The benchmarking commands were of the form:go test -tags=opa_wasm ./v1/bundle/ -run '^$' -bench '^(BenchmarkHasRootsOverlap|BenchmarkHasRootsOverlapWithStore)$' -benchmem -benchtime=2s -count=15 > run.txtSummary: