-
-
Notifications
You must be signed in to change notification settings - Fork 201
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
perf(changelog): cache commit retain checks (258 times faster generation) #772
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #772 +/- ##
==========================================
+ Coverage 37.03% 39.69% +2.66%
==========================================
Files 20 20
Lines 1542 1600 +58
==========================================
+ Hits 571 635 +64
+ Misses 971 965 -6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
d1e09db
to
202aeb6
Compare
Only a measly 87× faster? Pshaw nothing to see here moving along now… |
Challenge accepted. Now 258 times faster. 😄 |
ddf3e78
to
ed47005
Compare
I updated the cache implementation to be more useful by caching the changed files of each commit. This slows down the retain_commit check by ~2 times but allows reusing the cache in case the include/exclude patterns change. This is beneficial for running in monorepos where git-cliff runs multiple times to generate changelog for different subsets of the repo. This is a good trade-off in my opinion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
I fixed some bugs in the last three commits to improve the include/exclude behaviour:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, can you resolve conflicts caused by #709 ? 😇
This caches the commit retain checks whenever include/exclude paths are specified. This speeds up changelog generation by `87` times in a big repository I have: ``` Now: 0.237 s Before: 20.633 s ```
Sled fails to open the cache if multiple git-cliff processes run on the same repository. This fixes the issue by using cacache which stores the cache locally under .git/git-cliff
This slows down the retain_commit check by ~2 times, but allows reusing the cache in case the include/exclude patterns change. This is beneficial for running in monorepos where git-cliff runs multiple times to generate changelog for different subsets of the repo
The slices are normalized, so an owned pattern is needed to avoid type mismatch between new normalized/unchanged patterns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks!
Description
This caches the commit retain checks whenever include/exclude paths are specified.
This speeds up changelog generation by
258
times in a big repository I have:Fixes #767
It also improves handling including/excluding patterns, considering the first commit, and various other improvements.
How Has This Been Tested?
I tested this on the repository mentioned in #767
Screenshots / Logs (if applicable)
Here are the profiling flamegraphs created via #768. The bottleneck is removed after this fix is applied as seen in the above image (4366 samples to only 16 samples). After the fix, other parts of the code finally show up in the profiler.
After - Fully Cached:
After - Cached + new commit:
Before:
Types of Changes
Checklist: