git/diffcore-pickaxe.c, branch main

global: mark code units that generate warnings with `-Wsign-compare`

2024-12-06T11:20:02Z

Mark code units that generate warnings with `-Wsign-compare`. This allows for a structured approach to get rid of all such warnings over time in a way that can be easily measured. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

diff: improve lifecycle management of diff queues

2024-09-30T18:23:05Z

The lifecycle management of diff queues is somewhat confusing: - For most of the part this can be attributed to `DIFF_QUEUE_CLEAR()`, which does not release any memory but rather initializes the queue, only. This is in contrast to our common naming schema, where "clearing" means that we release underlying memory and then re-initialize the data structure such that it is ready to use. - A second offender is `diff_free_queue()`, which does not free the queue structure itself. It is rather a release-style function. Refactor the code to make things less confusing. `DIFF_QUEUE_CLEAR()` is replaced by `DIFF_QUEUE_INIT` and `diff_queue_init()`, while `diff_free_queue()` is replaced by `diff_queue_release()`. While on it, adapt callsites where we call `DIFF_QUEUE_CLEAR()` with the intent to release underlying memory to instead call `diff_queue_clear()` to fix memory leaks. This memory leak is exposed by t4211, but plugging it alone does not make the whole test suite pass. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

diff.h: remove unnecessary include of oidset.h

2023-06-21T20:39:53Z

This also made it clear that several .c files depended upon various things that oidset included, but had omitted the direct #include for those headers. Add those now. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

pretty.h: move has_non_ascii() declaration from commit.h

2023-02-24T01:25:29Z

The function is defined in pretty.c, so this moves the declaration to a more logical place. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

diffcore-pickaxe: mark unused parameters in pickaxe functions

2022-10-18T04:24:04Z

We have a virtual pickaxe_fn for handling -G versus -S pickaxe options. They need to take the same set of parameters, but of course they care about different ones (e.g., a regex -G will never use a kwset). Mark the unused ones to appease -Wunused-parameter. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

xdiff-interface: replace discard_hunk_line() with a flag

2021-05-11T03:47:31Z

Remove the dummy discard_hunk_line() function added in 3b40a090fd4 (diff: avoid generating unused hunk header lines, 2018-11-02) in favor of having a new XDL_EMIT_NO_HUNK_HDR flag, for use along with the two existing and similar XDL_EMIT_* flags. Unlike the recently amended xdiff_emit_line_fn interface which'll be called in a loop in xdl_emit_diff(), the hunk header is only emitted once. It makes more sense to pass this as a flag than provide a dummy callback because that function may be able to skip doing certain work if it knows the caller is doing nothing with the hunk header. It would be possible to do so in the case of -U0 now, but the benefit of doing so is so small that I haven't bothered. But this leaves the door open to that, and more importantly makes the API use more intuitive. The reason we're putting a flag in the gap between 1<<0 and 1<<2 is that the old 1<<1 flag was removed in 907681e940d (xdiff: drop XDL_EMIT_COMMON, 2016-02-23) without re-ordering the remaining flags. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

pickaxe -G: don't special-case create/delete

2021-05-11T03:47:31Z

Instead of special-casing creations and deletions let's just generate a diff for them. This logic of not running a diff under -G if we don't have both sides dates back to the original implementation of -S in 52e9578985f ([PATCH] Introducing software archaeologist's tool "pickaxe"., 2005-05-21). In the case of -S we were not working with the xdiff interface and needed to do this, but when -G was implemented in f506b8e8b5f (git log/diff: add -G that greps in the patch text, 2010-08-23) this logic was diligently copied over. But as the performance test added earlier in this series shows, this does not make much of a difference. With: time GIT_TEST_LONG= GIT_PERF_REPEAT_COUNT=10 GIT_PERF_MAKE_OPTS='-j8 CFLAGS=-O3' ./run origin/next HEAD~ HEAD -- p4209-pickaxe.sh With the HEAD~ commit being the preceding "pickaxe -G: terminate early on matching lines" we get these results. Note that it's only the -G codepaths that are relevant to this change: Test origin/next HEAD~ HEAD ----------------------------------------------------------------------------------------------------------------------------------------- 4209.1: git log -S'int main' .. 0.35(0.32+0.03) 0.35(0.33+0.02) +0.0% 0.35(0.30+0.05) +0.0% 4209.2: git log -S'æ' .. 0.46(0.42+0.04) 0.46(0.41+0.05) +0.0% 0.46(0.42+0.04) +0.0% 4209.3: git log --pickaxe-regex -S'(int|void|null)' .. 0.65(0.62+0.02) 0.64(0.61+0.02) -1.5% 0.64(0.60+0.04) -1.5% 4209.4: git log --pickaxe-regex -S'if *\([^ ]+ & ' .. 0.52(0.45+0.06) 0.52(0.50+0.01) +0.0% 0.54(0.47+0.04) +3.8% 4209.5: git log --pickaxe-regex -S'[àáâãäåæñøùúûüýþ]' .. 0.39(0.34+0.05) 0.39(0.34+0.04) +0.0% 0.39(0.36+0.03) +0.0% 4209.6: git log -G'(int|void|null)' .. 0.60(0.55+0.04) 0.58(0.54+0.03) -3.3% 0.58(0.49+0.08) -3.3% 4209.7: git log -G'if *\([^ ]+ & ' .. 0.61(0.52+0.06) 0.59(0.53+0.05) -3.3% 0.59(0.54+0.05) -3.3% 4209.8: git log -G'[àáâãäåæñøùúûüýþ]' .. 0.61(0.51+0.07) 0.58(0.54+0.04) -4.9% 0.57(0.51+0.06) -6.6% 4209.9: git log -i -S'int main' .. 0.36(0.31+0.04) 0.36(0.34+0.02) +0.0% 0.35(0.32+0.03) -2.8% 4209.10: git log -i -S'æ' .. 0.36(0.33+0.03) 0.39(0.34+0.01) +8.3% 0.36(0.32+0.03) +0.0% 4209.11: git log -i --pickaxe-regex -S'(int|void|null)' .. 0.83(0.77+0.05) 0.82(0.77+0.05) -1.2% 0.80(0.75+0.04) -3.6% 4209.12: git log -i --pickaxe-regex -S'if *\([^ ]+ & ' .. 0.67(0.61+0.03) 0.64(0.61+0.03) -4.5% 0.63(0.61+0.02) -6.0% 4209.13: git log -i --pickaxe-regex -S'[àáâãäåæñøùúûüýþ]' .. 0.40(0.37+0.02) 0.40(0.37+0.03) +0.0% 0.40(0.36+0.04) +0.0% 4209.14: git log -i -G'(int|void|null)' .. 0.58(0.51+0.07) 0.59(0.52+0.06) +1.7% 0.58(0.52+0.05) +0.0% 4209.15: git log -i -G'if *\([^ ]+ & ' .. 0.60(0.54+0.05) 0.60(0.54+0.06) +0.0% 0.60(0.56+0.03) +0.0% 4209.16: git log -i -G'[àáâãäåæñøùúûüýþ]' .. 0.58(0.51+0.06) 0.57(0.52+0.05) -1.7% 0.60(0.48+0.09) +3.4% This small simplification really doesn't buy us much now, but I've got plans to both convert the pickaxe code to using a PCREv2 backend[1] and to implement additional pickaxe modes to do custom searches through the diff[2]. Always having the diff available under -G is going to help to simplify both of those changes. 1. https://lore.kernel.org/git/20210203032811.14979-22-avarab@gmail.com/ 2. https://lore.kernel.org/git/20190424152215.16251-3-avarab@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

pickaxe -G: terminate early on matching lines

2021-05-11T03:47:31Z

Solve a long-standing item for "git log -Grx" of us e.g. finding "+ str" in the diff context and noting that we had a "hit", but xdiff diligently continuing to generate and spew the rest of the diff at us. This makes use of a new "early return" xdiff interface added by preceding commits. The TODO item (or, the NEEDSWORK comment) has been there since "git log -G" was implemented. See f506b8e8b5f (git log/diff: add -G that greps in the patch text, 2010-08-23). But now with the support added in the preceding changes to the xdiff-interface we can return early. Let's assert the behavior of that new early-return xdiff-interface by having a BUG() call here to die if it ever starts handing us needless work again. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

xdiff-interface: prepare for allowing early return

2021-05-11T03:47:31Z

Change the function prototype of xdiff_emit_line_fn to return an "int" instead of "void". Change all of those functions to "return 0", nothing checks those return values yet, and no behavior is being changed. In subsequent commits the interface will be changed to allow early return via this new return value. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

pickaxe -S: slightly optimize contains()

2021-05-11T03:47:31Z

When the "log -S" switch counts occurrences of on the pre-image and post-image of a change. As soon as we know we had e.g. 1 before and 2 now we can stop, we don't need to keep counting past 2. With this change a diff between A and B may have different performance characteristics than between B and A. That's OK in this case, since we'll emit the same output, and the effect is to make one of them better. I'm picking a check of "one" first on the assumption that it's a more common case to have files grow over time than not. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano