On Thu, Jul 17, 2025 at 08:32:23PM +0000, Ezekiel Newren via GitGitGadget wrote:
> // desktop
> // CPU: 8-core Intel Core i7-9700 (-MCP-) speed/min/max: 800/800/4700 MHz
> $ hyperfine --warmup 3 -L exe $BASE/build_release/git,$BASE/build_v2.49.0/git '{exe} log --oneline --shortstat v6.8..v6.9 >/dev/null'
> Benchmark 1: /home/steamuser/dev/git/build_release/git log --oneline --shortstat v6.8..v6.9 >/dev/null
> Time (mean ± σ): 6.823 s ± 0.020 s [User: 6.624 s, System: 0.180 s]
> Range (min … max): 6.801 s … 6.858 s 10 runs
>
> Benchmark 2: /home/steamuser/dev/git/build_v2.49.0/git log --oneline --shortstat v6.8..v6.9 >/dev/null
> Time (mean ± σ): 8.151 s ± 0.024 s [User: 7.928 s, System: 0.198 s]
> Range (min … max): 8.105 s … 8.184 s 10 runs
>
> Summary
> /home/steamuser/dev/git/build_release/git log --oneline --shortstat v6.8..v6.9 >/dev/null ran
> 1.19 ± 0.01 times faster than /home/steamuser/dev/git/build_v2.49.0/git log --oneline --shortstat v6.8..v6.9 >/dev/null
Very cool!
> Signed-off-by: Ezekiel Newren <ezekielnewren@xxxxxxxxx>
> ---
> rust/Cargo.lock | 7 +++++++
> rust/xdiff/Cargo.toml | 1 +
> rust/xdiff/src/lib.rs | 7 +++++++
> xdiff/xprepare.c | 19 +++++++++++++++++--
> 4 files changed, 32 insertions(+), 2 deletions(-)
This patch is delightfully simple. Thank you for carefully preparing the
previous five patches to make this one as tiny as it is.
> @@ -175,14 +178,26 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_
>
> xdl_parse_lines(mf, narec, xdf);
>
> + if ((xpp->flags & XDF_WHITESPACE_FLAGS) == 0) {
It may be worth adding a comment here to explain why we aren't using
xdl_hash_record() when xpp->flags lacks XDF_WHITESPACE_FLAGS.
(As a meta-note for reviewing this series, there are a handful of style
nits that I haven't mentioned, e.g., if (... == 0) instead of if (!...).
But since the xdiff code doesn't match the project's style conventions,
I have avoided mentioning it in my review.)
> + for (usize i = 0; i < (usize) xdf->nrec; i++) {
> + xrecord_t *rec = xdf->recs[i];
> + rec->ha = xxh3_64(rec->ptr, rec->size);
> + }
> + } else {
> + for (usize i = 0; i < (usize) xdf->nrec; i++) {
> + xrecord_t *rec = xdf->recs[i];
> + char const* dump = (char const*) rec->ptr;
> + rec->ha = xdl_hash_record(&dump, (char const*) (rec->ptr + rec->size), xpp->flags);
> + }
> + }
> +
> for (usize i = 0; i < (usize) xdf->nrec; i++) {
> xrecord_t *rec = xdf->recs[i];
> - char const* dump = (char const*) rec->ptr;
> - rec->ha = xdl_hash_record(&dump, (char const*) (rec->ptr + rec->size), xpp->flags);
> xdl_classify_record(pass, cf, rec);
I am curious why you are calling xdl_classify_record() here as a
post-processing step rather than inline with the hash calculation above.
Thanks,
Taylor
