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

Cache the Git remote URL to speed up rendering hyperlinks #1940

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

charlievieth
Copy link

@charlievieth charlievieth commented Jan 11, 2025

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

This commit speeds up the rendering of hyperlinks by ~55x by caching the
Git repo's remote URL instead of fetching it each time a hyperlink is
rendered.

Fixes dandavison#1939
@th1000s
Copy link
Collaborator

th1000s commented Jan 12, 2025

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.

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.

🐛 Hyperlinks make delta 55x slower when used as the pager for git log
2 participants