-
Notifications
You must be signed in to change notification settings - Fork 661
Improves git object cache #4685
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
Conversation
|
@pvanbuijtene: Thank you for your contribution and the work you have done. I have following remarks/suggestions:
|
That was my idea to move the functionality from the GitCache class back into the GitRepository as I think the repository wrapper should handle the caching mechanism as it's the abstraction of the database which maintains the collections and the cache, but we can reconsider that |
|
Happy to help :) @arturcic as I understand you will implement the suggestions, feel free to ping me in case you want to have some changes tested against a repository causing issues. |
|
@pvanbuijtene thank you, yeah, I implemented the suggestions so far in this PR, but it's a draft for now, I want to test caching the other collections as well before having this merged |
7393ace to
ec03709
Compare
468ac8d to
ac0e28b
Compare
|
@pvanbuijtene do you have the time to test this PR on the big repository you have and compare? |
|
@arturcic sure, the changes seem to have introduced some additional memory usage: |
btw what are you using for the memory usage analysis? |
|
I will also do a check with the anonymized repo to see if the object counts are similar, this way there might still be a way to verify things with that repo. |
30f83ff to
d5b92bf
Compare
|
@pvanbuijtene mind to have a new run on memory usage for main and PR branch against your original big repo? |
d5b92bf to
bfcf3ec
Compare
bfcf3ec to
d6b6483
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.
Pull Request Overview
This PR refactors the git object caching mechanism by introducing a dedicated GitRepositoryCache class to centralize object wrapping and caching. The changes improve code organization and maintainability by extracting caching logic from the GitRepository class into a specialized cache component.
Key Changes:
- Introduces a new
GitRepositoryCacheclass to centralize caching of git objects (branches, commits, tags, remotes, references, refspecs) - Removes the
IGitObjectinterface and moves its properties directly toICommit - Renames
Refsproperty toReferencesthroughout the codebase for consistency - Updates collection classes to use the new caching mechanism instead of creating objects directly
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| GitRepositoryCache.cs | New cache class implementing centralized caching logic for git objects |
| GitRepository.cs | Refactored to use GitRepositoryCache and lazy initialization for collections |
| Commit.cs | No longer inherits from GitObject; implements ICommit directly with Id/Sha properties |
| IGitObject.cs | Removed interface; properties moved to ICommit |
| ICommit.cs | Now contains Id and Sha properties directly instead of inheriting from IGitObject |
| IGitRepository.cs | Renamed Refs property to References |
| Branch.cs, BranchCollection.cs | Updated to use GitRepositoryCache instead of GitRepository |
| Tag.cs, TagCollection.cs | Updated to use GitRepositoryCache instead of GitRepository |
| CommitCollection.cs | Updated to use GitRepositoryCache instead of GitRepository |
| Remote.cs, RemoteCollection.cs | Updated to use GitRepositoryCache instead of GitRepository |
| Reference.cs, ReferenceCollection.cs | Updated to use GitRepositoryCache instead of GitRepository |
| RefSpec.cs, RefSpecCollection.cs | Marked RefSpec as sealed; updated to use GitRepositoryCache |
| RepositoryStore.cs | Updated method signature to use ICommit instead of IGitObject |
| GitPreparer.cs | Updated all Refs references to References |
| PublicAPI files | Updated shipped/unshipped API surfaces |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d6b6483 to
9eafb6e
Compare
Introduces caching mechanisms for remotes, references, and ref specs in `GitRepository`. Refactors related collections to utilize the new cache for improved efficiency and consistency.
Replaces caching logic within `GitRepository` with a new dedicated `GitRepositoryCache` class. Updates related classes and collections to utilize the cache through `GetOrWrap` methods, improving maintainability and reducing duplication.
9eafb6e to
681782b
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.
Pull Request Overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@pvanbuijtene mind to have a review as well?, please also attach the new memory consumption data you see on your big repo. |
thanks @pvanbuijtene. I think I will merge this one then. Thanks once again for the initial work you've done. |
|
Thank you @arturcic for your contribution! |

















Follow up for #4681