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

From: Justin Donnelly <justinrdonnelly@gmail.com>
Date: Aug 16, 2022, 04:20 UTC
Message-ID: <CAGTqyRyuV42-NRW_OKzy9F+LbFcZp-QZkM73LqrNkmOi60oU+w@mail.gmail.com>
In-Reply-To: <xmqqilmt4huq.fsf@gitster.g>

On Mon, Aug 15, 2022 at 12:00 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 22 quoted lines

>
> 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

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`.

Show 10 quoted lines

> 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