Re: [PATCH 0/2] [RFC] Revert/delay performance regression in 'git checkout -b'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Wed, Aug 21, 2019 at 12:21 PM Derrick Stolee via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> As we were integrating Git 2.23.0 into VFS for Git, we discovered that "git
> checkout -b new-branch" went from 0.3s to 10+s on the Windows OS repo. This
> was an intentional change when writing the "git switch" builtin. Here is the
> commit message for 65f099b ("switch: no worktree status unless real branch
> switch happens" 2019-03-29):
>
> When we switch from one branch to another, it makes sense to show a
> summary of local changes since there could be conflicts, or some files
> left modified.... When switch is used solely for creating a new
> branch (and "switch" to the same commit) or detaching, we don't really
> need to show anything.
>
> "git checkout" does it anyway for historical reasons. But we can start
> with a clean slate with switch and don't have to.
>
> This essentially reverts fa655d8411 (checkout: optimize "git checkout
> -b <new_branch>" - 2018-08-16) and make it default for switch,
> but also for -B and --detach. Users of big repos are encouraged to
> move to switch.
>
> I was considering doing a full, long-term revert of this change to get the
> performance back to normal, but I also saw this feedback on the list for
> this patch:
>
> I like this last bit. The skip_merge_working_tree() function which
> this removes was ugly, difficult to maintain, and difficult to get
> just right (and easy to break -- even by changing parts of the system
> which one might not expect to impact it).

Instead of restoring this easy-to-break code, could we instead
simplify it and make it more robust?  As per the original commit
message, the whole point of the patch is that when you have a huge
index, operations take a while.  But in the special case of "git
checkout -b <new_branch>", there's no need to even check the index.
So, we could:

  * Check if the user is running "git checkout -b <new_branch>"
  * If so, use the performance hack to skip touching the index at all.

This would be much better than what the patch currently does:

  * Check if the user has specified -m, if so they clearly didn't just
specify "git checkout -b <new_branch>"
  * Check if the user has specified -f, if so they clearly didn't just
specify "git checkout -b <new_branch>"
  * Check if the user has specified --detach, if so they clearly
didn't just specify "git checkout -b <new_branch>"
  * ...<lots of other similar steps>...
  * If we got here, since we've checked all other cases (assuming
other people who have touched checkout remembered to add the necessary
checks for each and every new flag), then by deduction the user must
have specified "git checkout -b <new_branch>", so...
  * Use the performance hack to skip touching the index at all.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux