Cache the Git remote URL to speed up rendering hyperlinks
This commit speeds up the rendering of hyperlinks by ~47x by caching the Git repos remote URL instead of fetching it each time a hyperlink is rendered.
Fixes #1939
Note
I am very new to Rust so I am very open to feedback and completely understand if you want to rewrite or abandon this entire PR (especially if for some reason the remote URL should not be cached). Additionally, I think the GitConfig should be responsible for storing the cached remote, but struggled to make that work due to borrowing, lifetimes, mutability, etc.
Benchmarks
The below benchmarks were ran in the root of Linux v6.12. We use head -c $((128 * 1024 * 1024)) to limit the amount of Git log data since otherwise benchmarking delta before this change takes a very long time.
TLDR: This change makes processing 128Mib of Git log output take 3.7 seconds instead of 176.1 seconds.
Without git remote caching
$ DELTA_PAGER='bash -c "time head -c $((128 * 1024 * 1024)) | pv --stats --output=/dev/null"' \
git -c 'pager.log=delta --hyperlinks' log
128MiB 0:02:56 [ 744KiB/s]
rate min/avg/max/mdev = 654979.457/764202.554/1108112.678/35667.893 B/s
real 2m56.063s
user 0m0.736s
sys 0m5.530s
With git remote caching (this change)
$ PATH="{PATH_TO_NEW_DELTA}:${PATH}"
$ DELTA_PAGER='bash -c "time head -c $((128 * 1024 * 1024)) | pv --stats --output=/dev/null"' \
git -c 'pager.log=delta --hyperlinks' log
128MiB 0:00:03 [34.6MiB/s]
rate min/avg/max/mdev = 35979500.033/36287504.626/36646720.334/263641.477 B/s
real 0m3.700s
user 0m0.205s
sys 0m1.690s
Thanks for reporting this!
Caching is fine, the slowdown is from libgit2 being overly correct and assuming that the remote url could change while the program is running.
The PR can indeed be made simpler by using a RefCell to add interior mutability to the GitConfig, i.e. cached_remote: RefCell<Option<Option<String>>> with an not-yet-cached option layer, and another for the result of .find_remote(..).ok() - or maybe a custum enum is clearer.
@th1000s Took a while until I had some spare cycles to look at this again, but following your guidance I was able to use a OnceCell to embed the GitRemoteRepo struct into the GitConfig struct. Thank you again for your suggestions and would love another review if you have the time.
@dandavison 1) Thank you for delta - it's incredible 2) Any chance you could enable the tests?
Hi @charlievieth -- tests are running! I'm sorry that I personally am finding so little time to work on delta at the moment, but thank you very much for your work here.
@dandavison Thank you and I completely understand (I've only recently had the cycles to address my own OSS commits and projects).
The clippy failure that I get on this branch/PR are identical to the one I get when running make lint (aka cargo clippy) against the main branch (d5e0565cbfa47acde98d41d8777ace5d1bc4d690). I'm fairly new to Rust so happy to look into the failure myself and include it into this branch or create a preceding PR (to merge before this one) that addresses the issue.
For context my rust toolchain is:
- rustc
1.89.0(29483883e 2025-08-04) (Homebrew) - clippy
0.1.89
Script used to check the delta of cargo clippy
#!/usr/bin/env bash
set -euo pipefail
TMP=$(mktemp -d)
TMP_MAIN="${TMP}/clippy.main.txt"
TMP_HYPER="${TMP}/clippy.hyperlink.txt"
if ! git remote -v | grep -q '^charlievieth'; then
git remote add charlievieth https://github.com/charlievieth/delta.git
fi
git fetch --all
git checkout main
cargo clippy &> "${TMP_MAIN}" || true
git checkout cev/hyperlink
cargo clippy &> "${TMP_HYPER}" || true
(
cd "${TMP}"
git diff --no-index "$(basename "${TMP_MAIN}")" "$(basename "${TMP_HYPER}")"
)
Output (nothing applicable AFAIK):
diff --git a/clippy.main.txt b/clippy.hyperlink.txt
index 65e5421..71ee65e 100644
--- a/clippy.main.txt
+++ b/clippy.hyperlink.txt
@@ -71,4 +71,4 @@ help: use `'_` for type paths
| ++++
warning: `git-delta` (bin "delta") generated 5 warnings
- Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.68s
+ Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.70s
If you'd like to deal with the clippy complaints then that would be great! I think the best way would be to open a PR for the clippy-related fixes against main and then rebase this on that.
If you'd like to deal with the clippy complaints then that would be great! I think the best way would be to open a PR for the clippy-related fixes against
mainand then rebase this on that.
Created https://github.com/dandavison/delta/pull/2038 to address the clippy complaints.
If you'd like to deal with the clippy complaints then that would be great! I think the best way would be to open a PR for the clippy-related fixes against
mainand then rebase this on that.Created #2038 to address the clippy complaints.
@dandavison now that https://github.com/dandavison/delta/pull/2038 is merged any chance you can take a look at this PR?
The clippy error reported here also occurs on the main branch.
Thank you for this improvement, and apologies for the delays. I also fixed the new v1.91 (which was released in the meantime) clippy warning.
@th1000s thank you for the review and for merging this PR!