-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Cache last commit to accelerate the repository directory page visit #10069
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 last commit to accelerate the repository directory page visit #10069
Conversation
@lunny instead of a hard lim, could we paginate it somehow? |
can you add some tests? |
Why the separate options? Is the new cache expected to go big? |
I like the added Enable option! |
I don't think we need separate cache for last commit, just use cache we already have in place |
Yeah, if there is no compelling reason to make it a separate cache, I'd prefer it to be included in the regular cache. |
2b05749
to
1f0edd1
Compare
That will be another PR. @lafriks @silverwind I have sent another commit that will use the original cache config defautly. I think the default cache time |
@6543 I will try to add a test but since it's a page performance optimization for big repository but the test repositories are small. The test will only confirm the results are the same before enable the cache or after. |
Codecov Report
@@ Coverage Diff @@
## master #10069 +/- ##
==========================================
+ Coverage 43.48% 43.49% +0.01%
==========================================
Files 567 567
Lines 79067 79104 +37
==========================================
+ Hits 34381 34409 +28
- Misses 40440 40447 +7
- Partials 4246 4248 +2
Continue to review full report at Codecov.
|
i think it is enouth if you add test to test if cache works at all ... (set cache-size to low value .. ; open repo & test if it is in the cash) ? |
1f0edd1
to
e23901c
Compare
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.
@lunny only following settings are needed for [cache.last_commit]
section
- edit: settingENABLED
: true: Enable the cache.ITEM_TTL
to 0 will disable cache so no need for this option alsoITEM_TTL
: 16h: Time to keep items in cache if not used, Setting it to 0 disables caching.COMMITS_COUNT
: 1000: Only enable the cache when repository's commits count great than.
You can reuse same cache and set specific TTL for last_commit
keys.
58633a0
to
b5ed5bd
Compare
@lafriks done. |
without
with
|
Maybe pre-populate cache on startup and push? |
I think the endresult is just more load on the system ... if we implement this i like to be able to disable this @silverwind |
@silverwind I will send another PR to do that. Because I think it's need a queue that will make this PR more complicated. |
@lafriks need your approval. |
Will replace #6045, resolve #491
On my local test (macOS ) with default cache setting(memory):
aports (home directory): 4010ms -> 225ms
vdsm (home directory): 1510ms -> 112ms
BTW: aports /main has many files, so I haven't tested it. We need a limitation on files listed. That will be another PR.