Skip to content
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.

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

Fix a performance regression in the built-in rebase #1983

Merged
merged 2 commits into from
Dec 13, 2018

Conversation

dscho
Copy link
Member

@dscho dscho commented Dec 13, 2018

In the VFSforGit project, it was detected that the patches to call the --am backend directly (i.e the builtin-rebase--am topic branch) introduced a performance regression.

The root cause was that we translated the move_to_original_branch to C by using the reset_head() function which always updated the worktree.

But in the case of move_to_original_branch, we want to update the original branch to point at the current revision, and not change the worktree at all.

To work around that performance issue, let's teach reset_head() to update only the refs.

This is what the legacy (scripted) rebase does in
`move_to_original_branch`, and we will need this functionality in the
next commit.

Signed-off-by: Johannes Schindelin <[email protected]>
In the scripted version, we call `move_to_original_branch`, which
updates `HEAD` to point to the original branch again, after updating
that branch to point at the current commit. This does not require any
worktree update.

In the built-in version, we tried to reuse the `reset_head()` function,
which however does try to update the worktree. Of course, as there are
no changes, no actual worktree updates are performed. However, with an
insanely large repository, even just looking whether we need to write
anything takes a noticeable time. So let's avoid that in the built-in
version, too.

Signed-off-by: Johannes Schindelin <[email protected]>
Copy link

@derrickstolee derrickstolee left a comment

Choose a reason for hiding this comment

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

Perf tests have confirmed this is the right fix.

@dscho dscho merged commit 33e4ee3 into git-for-windows:master Dec 13, 2018
@dscho dscho deleted the builtin-rebase-perf-fix branch December 13, 2018 19:22
dscho added a commit to dscho/git that referenced this pull request Dec 15, 2018
…f-fix

Fix a performance regression in the built-in rebase
@dscho dscho added this to the v2.20.1 milestone Dec 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants