Cache various Repository Git operations
Related comment: gitlab-com/operations#42 (comment 3609898)
cc @joshfng
-
Master
TODO:
-
Flush the cache for
Repository#has_visible_content?
whenever a user deletes the last remaining branch or creates a new one. - Pushing commits currently flushes all caches, which does more harm than good. On a busy project it would make the caching changes introduced in this MR pretty much useless.
- Measure if this has any impact on any other pages.
-
Flush the cache for
-
Master
Worth mentioning: our Redis cache expiration time is 2 weeks, this means that after 2 weeks of the initial caching certain pages might be slower the first until the cache is built. For now this should be good enough.
-
-
44 44 end 45 45 46 46 def empty? 47 raw_repository.empty? 47 return @empty unless @empty.nil? -
Master
The use of instance variables here (and below) ensures we don't end up loading data from Redis every time these methods are called on the same Repository instance within the same request.
-
-
Added 1 commit:
- f96c6ff5 - Cache various Repository Git operations
Toggle commit list -
Master
I tested the following pages to see how these changes impact performance elsewhere:
- GitLab CE dashboard
- GitLab CE commits list
- GitLab CE issues list
The timings are as following (here before/after indicates before/after the changes in this MR):
Before
- Dashboard: 13 seconds
- Commits: 28 seconds
- Issues: 5.5 seconds
After
- Dashboard: 5 seconds (= 2.6x faster)
- Commits: 10 seconds (= 2.8x faster)
- Issues: 1.8 seconds (= 3x faster)
-
Title changed from WIP: Cache various Repository Git operations to Cache various Repository Git operations
Toggle commit list -
Master
To spare reading of gitlab-com/operations#42 (closed), this basically reduces the loading time of issue #1 (closed) from around 21 seconds (locally) to around 3 seconds (also locally).
-
-
-
-
Master
Yay build finally green, comments welcome!
-
Owner
@yorickpeterse
👍 LGTM.My only question is whether we need to add these new cache keys to
cache_keys
so they can be cleared byexpire_cache
, or if they'll only ever need to be cleared manually, as they currently are.Anyway, you can merge at your discretion.
-
-
Master
@rspeicher I think because of:
Pushing commits currently flushes all caches, which does more harm than good. On a busy project it would make the caching changes introduced in this MR pretty much useless.
it's better to not expire these new cache keys?
-
Master
@rspeicher @rymai Rémy is correct here, the methods are not added to
cache_keys
as that would result in the cache being flushed upon every push even when not needed. -
Status changed to merged
Toggle commit list -
-
Owner
I really need to remember to read @yorickpeterse's commit messages since they're actually useful.
😀 -
-
-