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 built-in rebase perf regression #72
Conversation
/submit |
Submitted as pull.72.git.gitgitgadget@gmail.com |
The same clean-up code is repeated quite a few times; Let's DRY up the code some. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Currently, we only accept the flag indicating whether the HEAD should be detached not. In the next commit, we want to introduce another flag: to toggle between emulating `reset --hard` vs `checkout -q`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When we converted a `git checkout -q $onto^0` call to use `reset_head()`, we inadvertently incurred a change from a twoway_merge to a oneway_merge, as if we wanted a `git reset --hard` instead. This has performance ramifications under certain, though, as the oneway_merge needs to lstat() every single index entry whereas twoway_merge does not. So let's go back to the old behavior. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
070092b
to
a7360b8
Compare
/submit |
Submitted as pull.72.v2.git.gitgitgadget@gmail.com |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This branch is now known as |
This patch series was integrated into pu via git@bb6dd0e. |
This patch series was integrated into next via git@bb6dd0e. |
This patch series was integrated into master via git@bb6dd0e. |
This patch series was integrated into maint via git@bb6dd0e. |
Closed via bb6dd0e. |
In our tests with large repositories, we noticed a serious regression of the performance of
git rebase
when using the built-in vs the shell script version. It boils down to an incorrect conversion of agit checkout -q
: instead of using atwoway_merge
asgit checkout
does, we used aoneway_merge
asgit reset
does. The latter, however, callslstat()
on all files listed in the index, while the former essentially looks only at the files that are different between the given two revisions.Let's reinstate the original behavior by introducing a flag to the
reset_head()
function to indicate whether we want to emulatereset --hard
(in which case we use theoneway_merge
, otherwise we usetwoway_merge
).Changes vs v1:
goto leave_reset_head
.desc
array is not initialized to all-zero, to avoid bogus addresses being passed tofree()
.detach_head
andreset_hard
parameters have been consolidated into aflags
parameter.reset_head()
function once again only initializeshead_oid
when needed.