Skip to content
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 Git commands related to submodules-3.5 #8870

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions GitCommands/CommitDataManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@ public interface ICommitDataManager
CommitData CreateFromRevision(GitRevision revision, IReadOnlyList<ObjectId> children);

/// <summary>
/// Gets <see cref="CommitData"/> for the specified <paramref name="sha1"/>.
/// Gets <see cref="CommitData"/> for the specified <paramref name="commitId"/>.
/// </summary>
CommitData? GetCommitData(string sha1, out string? error);
/// <param name="commitId">The sha or Git reference.</param>
/// <param name="error">error message for the Git command</param>
/// <param name="cache">Allow caching of the Git command, should only be used if commitId is a sha and Notes are not used.</param>
CommitData? GetCommitData(string commitId, out string? error, bool cache = false);

/// <summary>
/// Updates the <see cref="CommitData.Body"/> (commit message) property of <paramref name="commitData"/>.
Expand All @@ -36,7 +39,8 @@ public interface ICommitDataManager

public sealed class CommitDataManager : ICommitDataManager
{
private const string CommitDataFormat = "%H%n%T%n%P%n%aN <%aE>%n%at%n%cN <%cE>%n%ct%n%e%n%B%nNotes:%n%-N";
private const string CommitDataFormat = "%H%n%T%n%P%n%aN <%aE>%n%at%n%cN <%cE>%n%ct%n%e%n%B";
private const string CommitDataWithNotesFormat = "%H%n%T%n%P%n%aN <%aE>%n%at%n%cN <%cE>%n%ct%n%e%n%B%nNotes:%n%-N";
private const string BodyAndNotesFormat = "%H%n%e%n%B%nNotes:%n%-N";

private readonly Func<IGitModule> _getModule;
Expand All @@ -49,7 +53,7 @@ public CommitDataManager(Func<IGitModule> getModule)
/// <inheritdoc />
public void UpdateBody(CommitData commitData, out string? error)
{
if (!TryGetCommitLog(commitData.ObjectId.ToString(), BodyAndNotesFormat, out error, out var data))
if (!TryGetCommitLog(commitData.ObjectId.ToString(), BodyAndNotesFormat, out error, out var data, cache: false))
{
return;
}
Expand Down Expand Up @@ -82,9 +86,9 @@ public void UpdateBody(CommitData commitData, out string? error)
}

/// <inheritdoc />
public CommitData? GetCommitData(string sha1, out string? error)
public CommitData? GetCommitData(string commitId, out string? error, bool cache = false)
{
return TryGetCommitLog(sha1, CommitDataFormat, out error, out var info)
return TryGetCommitLog(commitId, cache ? CommitDataFormat : CommitDataWithNotesFormat, out error, out var info, cache)
? CreateFromFormattedData(info)
: null;
}
Expand Down Expand Up @@ -183,7 +187,7 @@ private IGitModule GetModule()
return module;
}

private bool TryGetCommitLog(string commitId, string format, [NotNullWhen(returnValue: false)] out string? error, [NotNullWhen(returnValue: true)] out string? data)
private bool TryGetCommitLog(string commitId, string format, [NotNullWhen(returnValue: false)] out string? error, [NotNullWhen(returnValue: true)] out string? data, bool cache)
{
if (commitId.IsArtificial())
{
Expand All @@ -199,8 +203,12 @@ private bool TryGetCommitLog(string commitId, string format, [NotNullWhen(return
commitId
};

// Do not cache this command, since notes can be added
data = GetModule().GitExecutable.GetOutput(arguments, outputEncoding: GitModule.LosslessEncoding);
// This command can be cached if commitId is a git sha and Notes are ignored
Debug.Assert(!cache || ObjectId.TryParse(commitId, out _), $"git-log cache should be used only for sha ({commitId})");

data = GetModule().GitExecutable.GetOutput(arguments,
outputEncoding: GitModule.LosslessEncoding,
cache: cache ? GitModule.GitCommandCache : null);

if (GitModule.IsGitErrorMessage(data))
{
Expand Down
2 changes: 1 addition & 1 deletion GitCommands/Git/CommandCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public sealed class CommandCache
/// Initialises a new instance of <see cref="CommandCache"/> with specified <paramref name="capacity"/>.
/// </summary>
/// <param name="capacity">The maximum number of commands to cache.</param>
public CommandCache(int capacity = 40)
public CommandCache(int capacity = 50)
{
_cache = new MruCache<string, (byte[] output, byte[] error)>(capacity: capacity);
}
Expand Down
16 changes: 8 additions & 8 deletions GitCommands/Git/GitModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -790,15 +790,15 @@ public async Task<List<ConflictData>> GetConflictsAsync(string filename = "")
return _gitTreeParser.ParseSingle(output);
}

public int? GetCommitCount(string parentHash, string childHash)
public int? GetCommitCount(string parent, string child, bool cache = false)
{
var args = new GitArgumentBuilder("rev-list")
{
parentHash,
$"^{childHash}",
parent,
$"^{child}",
"--count"
};
var output = _gitExecutable.GetOutput(args);
var output = _gitExecutable.GetOutput(args, cache: cache ? GitCommandCache : null);

if (int.TryParse(output, out var commitCount))
{
Expand Down Expand Up @@ -833,7 +833,7 @@ public async Task<List<ConflictData>> GetConflictsAsync(string filename = "")
"--count",
"--left-right"
};
var output = _gitExecutable.GetOutput(args);
var output = _gitExecutable.GetOutput(args, cache: GitCommandCache);

var counts = output.Split('\t');
if (counts.Length == 2 && int.TryParse(counts[0], out var first) && int.TryParse(counts[1], out var second))
Expand Down Expand Up @@ -3659,7 +3659,7 @@ public string OpenWithDifftool(string? filename, string oldFileName = "", string
a,
b
};
var output = _gitExecutable.GetOutput(args);
var output = _gitExecutable.GetOutput(args, cache: GitCommandCache);

return ObjectId.TryParse(output, offset: 0, out var objectId)
? objectId
Expand Down Expand Up @@ -3703,7 +3703,7 @@ public SubmoduleStatus CheckSubmoduleStatus(ObjectId? commit, ObjectId? oldCommi

if (loadData)
{
oldData = _commitDataManager.GetCommitData(oldCommit.ToString(), out _);
oldData = _commitDataManager.GetCommitData(oldCommit.ToString(), out _, cache: true);
}

if (oldData is null)
Expand All @@ -3713,7 +3713,7 @@ public SubmoduleStatus CheckSubmoduleStatus(ObjectId? commit, ObjectId? oldCommi

if (loadData)
{
data = _commitDataManager.GetCommitData(commit.ToString(), out _);
data = _commitDataManager.GetCommitData(commit.ToString(), out _, cache: true);
}

if (data is null)
Expand Down
4 changes: 2 additions & 2 deletions GitCommands/Git/SubmoduleHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ public static class SubmoduleHelpers
else
{
var submodule = module.GetSubmodule(fileName);
addedCommits = submodule.GetCommitCount(commitId.ToString(), oldCommitId.ToString());
removedCommits = submodule.GetCommitCount(oldCommitId.ToString(), commitId.ToString());
addedCommits = submodule.GetCommitCount(commitId.ToString(), oldCommitId.ToString(), cache: true);
removedCommits = submodule.GetCommitCount(oldCommitId.ToString(), commitId.ToString(), cache: true);
}
}

Expand Down
4 changes: 2 additions & 2 deletions GitUI/Editor/FileViewer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ public Task ViewGitItemAsync(GitItemStatus file, [CanBeNull] Action openWithDiff
isSubmodule,
getImage: GetImage,
getFileText: GetFileTextIfBlobExists,
getSubmoduleText: () => LocalizationHelpers.GetSubmoduleText(Module, file.Name.TrimEnd('/'), sha),
getSubmoduleText: () => LocalizationHelpers.GetSubmoduleText(Module, file.Name.TrimEnd('/'), sha, cache: true),
openWithDifftool: openWithDifftool);

string GetFileTextIfBlobExists()
Expand Down Expand Up @@ -599,7 +599,7 @@ public Task ViewFileAsync(string fileName, bool isSubmodule = false, [CanBeNull]
isSubmodule,
getImage: GetImage,
getFileText: GetFileText,
getSubmoduleText: () => LocalizationHelpers.GetSubmoduleText(Module, fileName.TrimEnd('/'), ""),
getSubmoduleText: () => LocalizationHelpers.GetSubmoduleText(Module, fileName.TrimEnd('/'), "", cache: false),
openWithDifftool));

Image GetImage()
Expand Down
8 changes: 4 additions & 4 deletions ResourceManager/LocalizationHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public static string GetFullDateString(DateTimeOffset datetime)
return datetime.LocalDateTime.ToString("G");
}

public static string GetSubmoduleText(GitModule superproject, string name, string hash)
public static string GetSubmoduleText(GitModule superproject, string name, string hash, bool cache)
{
var sb = new StringBuilder();
sb.AppendLine("Submodule " + name);
Expand All @@ -91,7 +91,7 @@ public static string GetSubmoduleText(GitModule superproject, string name, strin
// TEMP, will be moved in the follow up refactor
ICommitDataManager commitDataManager = new CommitDataManager(() => module);

CommitData data = commitDataManager.GetCommitData(hash, out _);
CommitData data = commitDataManager.GetCommitData(hash, out _, cache);
if (data is null)
{
sb.AppendLine("Commit hash:\t" + hash);
Expand Down Expand Up @@ -152,7 +152,7 @@ public static string ProcessSubmoduleStatus([NotNull] GitModule module, [NotNull
{
if (status.OldCommit is not null)
{
oldCommitData = commitDataManager.GetCommitData(status.OldCommit.ToString(), out _);
oldCommitData = commitDataManager.GetCommitData(status.OldCommit.ToString(), out _, cache: true);
}

if (oldCommitData is not null)
Expand Down Expand Up @@ -184,7 +184,7 @@ public static string ProcessSubmoduleStatus([NotNull] GitModule module, [NotNull
{
if (status.Commit is not null)
{
commitData = commitDataManager.GetCommitData(status.Commit.ToString(), out _);
commitData = commitDataManager.GetCommitData(status.Commit.ToString(), out _, cache: true);
}

if (commitData is not null)
Expand Down