-
Notifications
You must be signed in to change notification settings - Fork 660
Reduce memory usage by caching commits on their sha hash #4681
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,3 +1,4 @@ | ||||||
using System.Collections.Concurrent; | ||||||
using GitVersion.Extensions; | ||||||
using GitVersion.Helpers; | ||||||
using LibGit2Sharp; | ||||||
|
@@ -8,6 +9,10 @@ internal sealed partial class GitRepository | |||||
{ | ||||||
private Lazy<IRepository>? repositoryLazy; | ||||||
|
||||||
private readonly ConcurrentDictionary<string, Branch> cachedBranches = new(); | ||||||
private readonly ConcurrentDictionary<string, Commit> cachedCommits = new(); | ||||||
private readonly ConcurrentDictionary<string, Tag> cachedTags = new(); | ||||||
|
||||||
private IRepository RepositoryInstance | ||||||
{ | ||||||
get | ||||||
|
@@ -20,12 +25,12 @@ private IRepository RepositoryInstance | |||||
public string WorkingDirectory => RepositoryInstance.Info.WorkingDirectory; | ||||||
public bool IsHeadDetached => RepositoryInstance.Info.IsHeadDetached; | ||||||
public bool IsShallow => RepositoryInstance.Info.IsShallow; | ||||||
public IBranch Head => new Branch(RepositoryInstance.Head, RepositoryInstance.Diff); | ||||||
public IBranch Head => GetOrCreate(RepositoryInstance.Head, RepositoryInstance.Diff); | ||||||
|
||||||
public ITagCollection Tags => new TagCollection(RepositoryInstance.Tags, RepositoryInstance.Diff); | ||||||
public ITagCollection Tags => new TagCollection(RepositoryInstance.Tags, RepositoryInstance.Diff, this); | ||||||
public IReferenceCollection Refs => new ReferenceCollection(RepositoryInstance.Refs); | ||||||
public IBranchCollection Branches => new BranchCollection(RepositoryInstance.Branches, RepositoryInstance.Diff); | ||||||
public ICommitCollection Commits => new CommitCollection(RepositoryInstance.Commits, RepositoryInstance.Diff); | ||||||
public IBranchCollection Branches => new BranchCollection(RepositoryInstance.Branches, RepositoryInstance.Diff, this); | ||||||
public ICommitCollection Commits => new CommitCollection(RepositoryInstance.Commits, RepositoryInstance.Diff, this); | ||||||
public IRemoteCollection Remotes => new RemoteCollection(RepositoryInstance.Network.Remotes); | ||||||
|
||||||
public void DiscoverRepository(string? gitDirectory) | ||||||
|
@@ -48,7 +53,7 @@ public void DiscoverRepository(string? gitDirectory) | |||||
var first = (Commit)commit; | ||||||
var second = (Commit)otherCommit; | ||||||
var mergeBase = RepositoryInstance.ObjectDatabase.FindMergeBase(first, second); | ||||||
return mergeBase == null ? null : new Commit(mergeBase, RepositoryInstance.Diff); | ||||||
return mergeBase == null ? null : GetOrCreate(mergeBase, RepositoryInstance.Diff); | ||||||
}); | ||||||
} | ||||||
|
||||||
|
@@ -58,6 +63,26 @@ public int UncommittedChangesCount() | |||||
return retryAction.Execute(GetUncommittedChangesCountInternal); | ||||||
} | ||||||
|
||||||
public Branch GetOrCreate(LibGit2Sharp.Branch innerBranch, Diff repoDiff) | ||||||
{ | ||||||
if (innerBranch.Tip is null) | ||||||
{ | ||||||
return new Branch(innerBranch, repoDiff, this); | ||||||
} | ||||||
|
||||||
var cacheKey = $"{innerBranch.CanonicalName}|{innerBranch.Tip.Sha}|{innerBranch.RemoteName}"; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry for not discovering this earlier. It's not very important, but will improve debugging if the format of the key matches what one would expect to find used in Git and GitHub a bit more than this.
Suggested change
|
||||||
return cachedBranches.GetOrAdd(cacheKey, new Branch(innerBranch, repoDiff, this)); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't lazy initialization here be even more optimal, avoiding the construction of a
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree |
||||||
} | ||||||
|
||||||
public Commit GetOrCreate(LibGit2Sharp.Commit innerCommit, Diff repoDiff) => | ||||||
cachedCommits.GetOrAdd(innerCommit.Sha, new Commit(innerCommit, repoDiff, this)); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
public Tag GetOrCreate(LibGit2Sharp.Tag innerTag, Diff repoDiff) | ||||||
{ | ||||||
var cacheKey = $"{innerTag.CanonicalName}|{innerTag.Target.Sha}"; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
return cachedTags.GetOrAdd(cacheKey, new Tag(innerTag, repoDiff, this)); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
|
||||||
public void Dispose() | ||||||
{ | ||||||
if (this.repositoryLazy is { IsValueCreated: true }) RepositoryInstance.Dispose(); | ||||||
|
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.
Why the rename from
GitCache
toGitRepository
? I don't feel like the provided functionality is quite that of a repository, which is often used as an abstraction over a database. 🤔