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

refactor: use http-cache for caching, optimize network calls #304

Merged
merged 9 commits into from
Dec 16, 2024

Conversation

woodruffw
Copy link
Owner

@woodruffw woodruffw commented Dec 15, 2024

I'm not 100% sure if this makes sense yet. Need to do some performance measurements.

TODOs:

  • Use a cache location other than ./http-cache (should probably be ~/.cache/zizmor by default or similar)
  • See if we can better take advantage of async Rust here

Fixes #52.

@woodruffw woodruffw added performance refactor Refactoring tasks labels Dec 15, 2024
@woodruffw woodruffw self-assigned this Dec 15, 2024
@woodruffw
Copy link
Owner Author

Very rough benchmark:

export GH_TOKEN=$(gh auth token)
time zizmor pypa/pip-audit

With a release build of zizmor 0.9.2 that averages about 14s on my local machine.

Versus this branch, release build:

time ./target/release/zizmor pypa/pip-audit

...averages about 16s on a cold cache. However, once warmed up, it takes ~700ms to run.

So, slightly slower on a cold run, but significantly faster on subsequent runs.

Not sure this matters, given that it's all essentially
serialized to the same thread anyways.
@woodruffw
Copy link
Owner Author

2f1b6ef (#304) separately optimizes the impostor-commit audit's tag/branch lookup pattern, making it perform better on real-world actions (in the real world, hash-pinned tags are more common than hash-pinned branches, so we can search those first to get a performance boost).

With that, the runs on this branch are now around 12.5s for me locally. So that "pays" for the change here, even if it's not directly caused by it.

@woodruffw
Copy link
Owner Author

14b859b (#304) separately optimizes ref-confusion to use per-branch/tag APIs, rather than listing all branches/tags in each target repo. This also shaves ~2s off my local runs against pypa/pip-audit, bringing this branch down to around 10.5s on cold runs.

@woodruffw woodruffw changed the title refactor: use http-cache for caching refactor: use http-cache for caching, optimize network calls Dec 15, 2024
@woodruffw
Copy link
Owner Author

Final savings here, with a cold cache:

0.15user 0.18system 0:10.66elapsed 3%CPU (0avgtext+0avgdata 11648maxresident)k
0inputs+2488outputs (0major+972minor)pagefaults 0swaps

versus current release (0.9.2):

0.05user 0.04system 0:14.31elapsed 0%CPU (0avgtext+0avgdata 10752maxresident)k
0inputs+0outputs (0major+845minor)pagefaults 0swaps

...so about 40% faster in the cold case. In the hot case, we're 90% faster (although most users won't fully see this, since they won't be running zizmor in a tight loop over the same repo repeatedly).

@woodruffw woodruffw merged commit 6a6e0ca into main Dec 16, 2024
20 checks passed
@woodruffw woodruffw deleted the ww/http-cache branch December 16, 2024 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance refactor Refactoring tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Caching: persist between runs
1 participant