Cache Git commands related to submodules

* Cache commands with sha input and no variable input like Git Notes Only fetch Git Notes for commands that are not cached * Cache GetCommitRangeDiffCount (for rev-diff) * Increase Git command cache to 50
gitextensions · Feb 24, 2021 · f7c94da · f7c94da
1 parent eea10a7
commit f7c94da
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 26 deletions.
diff --git a/GitCommands/CommitDataManager.cs b/GitCommands/CommitDataManager.cs
@@ -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"/>.
@@ -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;
@@ -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;
             }
@@ -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;
         }
@@ -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())
             {
@@ -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))
             {

diff --git a/GitCommands/Git/CommandCache.cs b/GitCommands/Git/CommandCache.cs
@@ -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);
         }

diff --git a/GitCommands/Git/GitModule.cs b/GitCommands/Git/GitModule.cs
@@ -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))
             {
@@ -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))
@@ -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
@@ -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)
@@ -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)

diff --git a/GitCommands/Git/SubmoduleHelpers.cs b/GitCommands/Git/SubmoduleHelpers.cs
@@ -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);
                 }
             }
 

diff --git a/GitUI/Editor/FileViewer.cs b/GitUI/Editor/FileViewer.cs
@@ -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()
@@ -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()

diff --git a/ResourceManager/LocalizationHelpers.cs b/ResourceManager/LocalizationHelpers.cs
@@ -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);
@@ -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);
@@ -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)
@@ -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)