[upstream handoff] fix: stop find_nearest_bookmarked_ancestors re-walking merge ancestors combinatorially - #8
Draft
rigelbuild-bot wants to merge 1 commit into
Conversation
…king merge ancestors combinatorially This branch is based on upstream v0.5.4 (0c03418), not on this fork's main. Matt pulls it and pushes it to the upstream Codeberg repo (codeberg.org/abrenneke/jj-vine). The two sections below are the text to open the upstream PR with; they are the deliverable, not this fork PR's own description. ## Upstream PR title fix: stop find_nearest_bookmarked_ancestors re-walking merge ancestors combinatorially ## Upstream PR description `find_nearest_bookmarked_ancestors` recurses into a commit's ancestry with no record of what it has already visited. On a linear history that is fine, but a history with merge commits reaches a shared ancestor by more than one path, and each path re-expands that ancestor's whole subtree from scratch. Every expansion is a `jj log` subprocess, so the number of `jj` invocations grows exponentially with the number of merges. On a repository with 136 commits and 26 merges outside `trunk()` this did not terminate within 200 seconds. A 12-rung ladder of merge diamonds spawns 8194 `jj` invocations. This threads a `visited` set (keyed on commit id) through the recursion, so each commit's ancestry is expanded at most once. The set of boundary ancestors the function returns is unchanged: a boundary reached on the first visit already bubbles up to the caller, which deduplicates. The walk goes from exponential to linear. Adds a regression test that builds a ladder of merge diamonds and asserts the `jj` invocation count stays linear (a small test-only subprocess counter on `Jujutsu` backs the assertion). Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
upstream-vcs/upstream-rewalk-dedup
branch
from
September 11, 2026 23:25
5f9dfca to
f5a17ce
Compare
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.
This branch is based on upstream v0.5.4 (0c03418), not on this fork's main. Matt pulls it and pushes it to the upstream Codeberg repo (codeberg.org/abrenneke/jj-vine). The two sections below are the text to open the upstream PR with; they are the deliverable, not this fork PR's own description.
Upstream PR title
fix: stop find_nearest_bookmarked_ancestors re-walking merge ancestors combinatorially
Upstream PR description
find_nearest_bookmarked_ancestorsrecurses into a commit's ancestry with no record of what it has already visited. On a linear history that is fine, but a history with merge commits reaches a shared ancestor by more than one path, and each path re-expands that ancestor's whole subtree from scratch. Every expansion is ajj logsubprocess, so the number ofjjinvocations grows exponentially with the number of merges.On a repository with 136 commits and 26 merges outside
trunk()this did not terminate within 200 seconds. An 8-rung ladder of merge diamonds spawns 514jjinvocations where the fixed walk spawns 20.This threads a
visitedset (keyed on commit id) through the recursion, so each commit's ancestry is expanded at most once. The set of boundary ancestors the function returns is unchanged: a boundary reached on the first visit already bubbles up to the caller, which deduplicates. The walk goes from exponential to linear.Adds a regression test that builds a ladder of merge diamonds and asserts the
jjinvocation count stays linear, well under a ceiling the pre-fix walk blows past (a small test-only subprocess counter onJujutsubacks the assertion).Co-authored-by: Matt Wilkinson matt@rigel.build