Re: [PATCH v2] git-prompt: show presence of unresolved conflicts at command prompt

[Justin Donnelly <justinrdonnelly@gmail.com>]

On Tue, Aug 16, 2022 at 12:20 AM Justin Donnelly
<justinrdonnelly@gmail.com> wrote:
>
> On Mon, Aug 15, 2022 at 12:00 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Phillip Wood <phillip.wood123@gmail.com> writes:
> >
> > > I had not commented as I don't use the prompt. I have just had a quick
> > > read and I wonder if it would be more efficient to use
> > >     git diff --cached --quiet --diff-filter=U
> > > rather than
> > >     git ls-files --unmerged 2>/dev/null
> > > to check if there are unmerged entries,
> >
> > The former reads the on-disk index into in-core index, and reads
> > tree objects (recursively for subdirectories) referenced by the
> > HEAD, walks both in parallel to find differences and filters out the
> > result to unmerged (I am not sure how well diff-filter works with
> > unmerged paths, though).
> >
> > The latter rads the on-disk index into in-core index, scans the
> > entries and finds unmerged entries.
> >
> > So if we compare the overhead to run either command standalone, I am
> > reasonably sure that the latter would be a lot more efficient.
> >
>
> Here's how I tested performance. The setup and test execution code are
> below. I tested each technique (`git ls-files --unmerged 2>/dev/null`
> and `git diff --cached --quiet --diff-filter=U`) 3 times and listed
> the results. Please let me know if you see any problems with the
> methodology.
>
> Setup:
> mkdir -p /tmp/perf/base && cd /tmp/perf/base
> git clone https://github.com/torvalds/linux.git
>
> Each test:
> cd /tmp/perf
> rm -rf test
> cp -r base test
> cd test/linux/drivers/watchdog
> git switch --detach v6.0-rc1 # pick some commit for consistency
> for f in *; do echo "/* a */" >> $f; done # 182 files
> git stash
> for f in *; do echo "/* b */" >> $f; done
> git commit -am "adding to end of files in watchdog directory"
> git stash pop
> time [[ $(git ls-files --unmerged 2>/dev/null) ]]
> # OR run the next one instead
> # time ! git diff --cached --quiet --diff-filter=U 2>/dev/null
>
> Results (hopefully this text lines up better for others than it does for me):
> time [[ $(git ls-files --unmerged 2>/dev/null) ]]
>       run 1     run 2     run3
> real  0m0.008s  0m0.009s  0m0.008s
> user  0m0.005s  0m0.001s  0m0.004s
> sys   0m0.004s  0m0.008s  0m0.004s
>
> time ! git diff --cached --quiet --diff-filter=U 2>/dev/null
>       run 1     run 2     run3
> real  0m0.009s  0m0.009s  0m0.007s
> user  0m0.004s  0m0.009s  0m0.007s
> sys   0m0.004s  0m0.000s  0m0.000s
>

Actually, what's probably more important is how long the commands take
when there is no conflict (that will be a far more common situation).
I tested that today, and the numbers were about the same.

> As you can see, the results are basically the same. I'm not sure if
> real world usage would yield different results. So for now, I'll defer
> to Junio's analysis and stick with `ls-files --unmerged`.
>
> > But if the shell prompt code already needs to run the diff-index for
> > other reasons (e.g. to show if there is any modification added to
> > the index), that may change the equation.  Instead of adding a
> > separate and extra call to "ls-files -u", it might be more efficient
> > if you can somehow piggy-back on an existing diff-index call.  For
> > example, you may be running "git diff --cached --quiet" for exit code
> > to show if any change has been added, but you can instead run "git
> > diff --no-ext-diff --no-renames --cached --name-status" and (1) if
> > there is any output, then the index is dirty, and (2) if there is a
> > line that begins with "U", you have an unmerged path right there.
>
> It had not occurred to me to consolidate and piggy-back off of other
> commands. I find the idea intriguing, but am not sure it makes sense
> to do it for only a single feature (especially this feature since the
> time to determine the conflict is short). I think it would make the
> code more complex, and it might be better to take a holistic approach
> to such an effort. Let me know if you strongly disagree.
>
> Thanks,
> Justin