Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

diff: reuse precomputed hash values #4917

Merged
merged 3 commits into from
Nov 20, 2024
Merged

Conversation

yuja
Copy link
Contributor

@yuja yuja commented Nov 19, 2024

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

yuja added 3 commits November 19, 2024 21:56
Since patience diff is recursive, it makes some sense to reuse precomputed
hash values. This patch migrates Histogram to remembering hashed values. The
precomputed values will be cached globally by DiffSource.

Technically, Histogram doesn't have to keep a separate copy of hash values, but
this appears to give better perf than slicing text and hash value from two Vecs.
… index

I'm going to add a Vec of precomputed hashes, and the Vec will be owned by
DiffSource.
This isn't always fast because it increases the chance of cache miss, but in
practice, it makes "jj file annotate" faster. It's still slower than
"git blame", though.

Maybe we should also change the hash function.

```
group                             new                     old
-----                             ---                     ---
bench_diff_git_git_read_tree_c    1.00     45.2±0.38µs    1.29     58.4±0.32µs
bench_diff_lines/modified/10k     1.00     32.7±0.24ms    1.05     34.4±0.17ms
bench_diff_lines/modified/1k      1.00      2.9±0.00ms    1.06      3.1±0.01ms
bench_diff_lines/reversed/10k     1.00     22.7±0.18ms    1.02     23.2±0.29ms
bench_diff_lines/reversed/1k      1.00    439.0±9.46µs    1.19    523.9±6.05µs
bench_diff_lines/unchanged/10k    1.00      2.9±0.06ms    1.20      3.5±0.02ms
bench_diff_lines/unchanged/1k     1.00    240.8±1.03µs    1.30    312.1±1.05µs
```

```
% hyperfine --sort command --warmup 3 --runs 10 -L bin jj-0,jj-1 \
  'target/release-with-debug/{bin} --ignore-working-copy file annotate lib/src/revset.rs'
Benchmark 1: target/release-with-debug/jj-0 ..
  Time (mean ± σ):      1.604 s ±  0.259 s    [User: 1.543 s, System: 0.057 s]
  Range (min … max):    1.348 s …  1.917 s    10 runs

Benchmark 2: target/release-with-debug/jj-1 ..
  Time (mean ± σ):      1.183 s ±  0.026 s    [User: 1.118 s, System: 0.062 s]
  Range (min … max):    1.155 s …  1.237 s    10 runs
```
@yuja yuja merged commit 05c7da3 into jj-vcs:main Nov 20, 2024
18 checks passed
@yuja yuja deleted the push-rquppqumnonk branch November 20, 2024 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants