From: Phillip Wood <phillip.wood123@gmail•com>
To: "D. Ben Knoble" <ben.knoble@gmail•com>, "René Scharfe" <l.s.r@web•de>
Cc: Git <git@vger•kernel.org>,
Phillip Wood <phillip.wood@dunelm•org.uk>,
Junio C Hamano <gitster@pobox•com>, Jeff King <peff@peff•net>
Subject: Re: [PATCH v2] diff: disable rename detection with --quiet
Date: Mon, 10 Nov 2025 09:42:53 +0000 [thread overview]
Message-ID: <61e6b077-26ef-49ed-83cf-fa5b7888429c@gmail.com> (raw)
In-Reply-To: <CALnO6CC+ke1L7T+dO13B0FSjLyJqihKHKZaa-B4dh9guxk7z0Q@mail.gmail.com>
Hi Ben
On 09/11/2025 17:34, D. Ben Knoble wrote:
> On Sun, Nov 9, 2025 at 11:43 AM René Scharfe <l.s.r@web•de> wrote:
>>
> λ hyperfine -NiP v 1 2 ~/code/git/'buildv{v}/git diff --cached --quiet
> --no-ext-diff --find-copies-harder'
> Benchmark 1: /home/benknoble/code/git/buildv1/git diff --cached
> --quiet --no-ext-diff --find-copies-harder
> Time (mean ± σ): 72.0 ms ± 3.3 ms [User: 45.2 ms, System: 26.2 ms]
> Range (min … max): 67.6 ms … 79.6 ms 42 runs
>
> Warning: Ignoring non-zero exit code.
>
> Benchmark 2: /home/benknoble/code/git/buildv2/git diff --cached
> --quiet --no-ext-diff --find-copies-harder
> Time (mean ± σ): 19.9 ms ± 1.5 ms [User: 8.9 ms, System: 10.6 ms]
> Range (min … max): 16.1 ms … 24.0 ms 151 runs
>
> Warning: Ignoring non-zero exit code.
>
> Summary
> /home/benknoble/code/git/buildv2/git diff --cached --quiet
> --no-ext-diff --find-copies-harder ran
> 3.61 ± 0.31 times faster than /home/benknoble/code/git/buildv1/git
> diff --cached --quiet --no-ext-diff --find-copies-harder
That's a nice speedup. Thanks for sharing that - I knew in an abstract
way that "--find-copies-harder" slowed things down but seeing some
concrete numbers really brings it home.
Best Wishes
Phillip
>> This actually fixes the error code when using the options --cached,
>> --find-copies-harder, --no-ext-diff and --quiet together:
>> run_diff_index() indirectly calls diff-lib.c::show_modified(), which
>> queues even non-modified entries using diff_change() because we need
>> them for copy detection. diff_change() sets flags.has_changes, though,
>> which causes diff_can_quit_early() to declare we're done after seeing
>> only the very first entry -- way too soon.
>
> This does describe the behavior I saw, but it seems to me that, if we
> have changes, then we ought to be able to quit early for --quiet, no?
>
> So there's some other knock-on effect that causes quitting early to be
> wrong here, and I'm not exactly sure what it is (other than the diff
> queues being different sizes when we hit relevant parts of
> diffcore_std, though it's the working case that has the larger queue).
> So I'm having a hard time tying this paragraph to the actual issue
> (mostly due to my complete unfamiliarity with the diffing subsystem).
>
>> Using --cached, --find-copies-harder and --quiet together without
>> --no-ext-diff was not affected even before, as it causes the flag
>> flags.diff_from_contents to be set, which disables the optimization
>> in a different way.
>>
>> Reported-by: D. Ben Knoble <ben.knoble@gmail•com>
>> Suggested-by: Phillip Wood <phillip.wood@dunelm•org.uk>
>> Signed-off-by: René Scharfe <l.s.r@web•de>
>> ---
>> diff.c | 2 ++
>> t/t4007-rename-3.sh | 10 ++++++++++
>> 2 files changed, 12 insertions(+)
>>
>> diff --git a/diff.c b/diff.c
>> index a1961526c0..efa8d9773c 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -4987,6 +4987,8 @@ void diff_setup_done(struct diff_options *options)
>> if (options->flags.quick) {
>> options->output_format = DIFF_FORMAT_NO_OUTPUT;
>> options->flags.exit_with_status = 1;
>> + options->detect_rename = 0;
>> + options->flags.find_copies_harder = 0;
>> }
>>
>> /*
>> diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh
>> index e8faf0dd2e..3fc81bcd76 100755
>> --- a/t/t4007-rename-3.sh
>> +++ b/t/t4007-rename-3.sh
>> @@ -41,6 +41,16 @@ test_expect_success 'copy detection, cached' '
>> compare_diff_raw current expected
>> '
>>
>> +test_expect_success 'exit code of quiet copy detection' '
>> + test_expect_code 1 \
>> + git diff --quiet --cached --find-copies-harder $tree
>> +'
>> +
>> +test_expect_success 'exit code of quiet copy detection with --no-ext-diff' '
>> + test_expect_code 1 \
>> + git diff --quiet --cached --find-copies-harder --no-ext-diff $tree
>> +'
>> +
>> # In the tree, there is only path0/COPYING. In the cache, path0 and
>> # path1 both have COPYING and the latter is a copy of path0/COPYING.
>> # However when we say we care only about path1, we should just see
>> --
>> 2.51.2
>
> Covering both seems like the right move to me, thanks!
>
> --
> D. Ben Knoble
>
next prev parent reply other threads:[~2025-11-10 9:43 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-08 19:05 diff --cached --no-ext-diff --find-copies-harder --quiet exits with wrong status code D. Ben Knoble
2025-11-08 19:08 ` D. Ben Knoble
2025-11-08 19:12 ` D. Ben Knoble
2025-11-09 12:11 ` [PATCH] diff: disabled quick optimization with --find-copies-harder René Scharfe
2025-11-09 14:18 ` Phillip Wood
2025-11-09 16:43 ` René Scharfe
2025-11-09 16:43 ` [PATCH v2] diff: disable rename detection with --quiet René Scharfe
2025-11-09 17:34 ` D. Ben Knoble
2025-11-09 18:35 ` René Scharfe
2025-11-10 23:58 ` D. Ben Knoble
2025-11-10 9:42 ` Phillip Wood [this message]
2025-11-10 17:54 ` Jeff King
2025-11-10 19:13 ` Junio C Hamano
2025-11-22 21:44 ` René Scharfe
2025-11-23 7:09 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=61e6b077-26ef-49ed-83cf-fa5b7888429c@gmail.com \
--to=phillip.wood123@gmail$(echo .)com \
--cc=ben.knoble@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=l.s.r@web$(echo .)de \
--cc=peff@peff$(echo .)net \
--cc=phillip.wood@dunelm$(echo .)org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox