From c220b09257d6682d4d1aab8f5794f51bef347833 Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Fri, 27 Jun 2025 10:42:22 -0700 Subject: [PATCH 1/2] Download all trees for non-prefetched commits when root tree is downloaded --- GVFS/GVFS.Common/Git/GitRepo.cs | 11 +++--- GVFS/GVFS.Common/Git/LibGit2Repo.cs | 17 +++------ GVFS/GVFS.Mount/InProcessMount.cs | 36 ++++++++++++++++--- .../Mock/Git/MockLibGit2Repo.cs | 3 +- GVFS/GVFS/CommandLine/GVFSVerb.cs | 2 +- 5 files changed, 47 insertions(+), 22 deletions(-) diff --git a/GVFS/GVFS.Common/Git/GitRepo.cs b/GVFS/GVFS.Common/Git/GitRepo.cs index 4aa4b945a7..ee9d8b96d2 100644 --- a/GVFS/GVFS.Common/Git/GitRepo.cs +++ b/GVFS/GVFS.Common/Git/GitRepo.cs @@ -4,6 +4,7 @@ using System.IO; using System.IO.Compression; using System.Linq; +using static GVFS.Common.Git.LibGit2Repo; namespace GVFS.Common.Git { @@ -60,9 +61,9 @@ public void OpenRepo() this.libgit2RepoInvoker?.InitializeSharedRepo(); } - public bool TryGetIsBlob(string sha, out bool isBlob) + public bool TryGetObjectType(string sha, out Native.ObjectTypes? objectType) { - return this.libgit2RepoInvoker.TryInvoke(repo => repo.IsBlob(sha), out isBlob); + return this.libgit2RepoInvoker.TryInvoke(repo => repo.GetObjectType(sha), out objectType); } public virtual bool TryCopyBlobContentStream(string blobSha, Action writeAction) @@ -86,10 +87,12 @@ public virtual bool TryCopyBlobContentStream(string blobSha, Action repo.CommitAndRootTreeExists(commitSha), out output); + string treeShaLocal = null; + this.libgit2RepoInvoker.TryInvoke(repo => repo.CommitAndRootTreeExists(commitSha, out treeShaLocal), out output); + rootTreeSha = treeShaLocal; return output; } diff --git a/GVFS/GVFS.Common/Git/LibGit2Repo.cs b/GVFS/GVFS.Common/Git/LibGit2Repo.cs index c1056def32..0849dd6b7f 100644 --- a/GVFS/GVFS.Common/Git/LibGit2Repo.cs +++ b/GVFS/GVFS.Common/Git/LibGit2Repo.cs @@ -41,24 +41,17 @@ protected LibGit2Repo() protected ITracer Tracer { get; } protected IntPtr RepoHandle { get; private set; } - public bool IsBlob(string sha) + public Native.ObjectTypes? GetObjectType(string sha) { IntPtr objHandle; if (Native.RevParseSingle(out objHandle, this.RepoHandle, sha) != Native.SuccessCode) { - return false; + return null; } try { - switch (Native.Object.GetType(objHandle)) - { - case Native.ObjectTypes.Blob: - return true; - - default: - return false; - } + return Native.Object.GetType(objHandle); } finally { @@ -91,9 +84,9 @@ public virtual string GetTreeSha(string commitish) return null; } - public virtual bool CommitAndRootTreeExists(string commitish) + public virtual bool CommitAndRootTreeExists(string commitish, out string treeSha) { - string treeSha = this.GetTreeSha(commitish); + treeSha = this.GetTreeSha(commitish); if (treeSha == null) { return false; diff --git a/GVFS/GVFS.Mount/InProcessMount.cs b/GVFS/GVFS.Mount/InProcessMount.cs index a6f9710a86..79fa29e9e7 100644 --- a/GVFS/GVFS.Mount/InProcessMount.cs +++ b/GVFS/GVFS.Mount/InProcessMount.cs @@ -16,6 +16,7 @@ using System.IO; using System.Text; using System.Threading; +using static GVFS.Common.Git.LibGit2Repo; namespace GVFS.Mount { @@ -44,6 +45,8 @@ public class InProcessMount private HeartbeatThread heartbeat; private ManualResetEvent unmountEvent; + private readonly Dictionary treesWithRecentlyDownloadedCommits = new Dictionary(); + // True if InProcessMount is calling git reset as part of processing // a folder dehydrate request private volatile bool resetForDehydrateInProgress; @@ -504,7 +507,21 @@ private void HandleDownloadObjectRequest(NamedPipeMessages.Message message, Name else { Stopwatch downloadTime = Stopwatch.StartNew(); - if (this.gitObjects.TryDownloadAndSaveObject(objectSha, GVFSGitObjects.RequestSource.NamedPipeMessage) == GitObjects.DownloadAndSaveObjectResult.Success) + + /* If this is the root tree for a commit that was was just downloaded, assume that more + * trees will be needed soon and download them as well by using the download commit API. + * + * Otherwise, or as a fallback if the commit download fails, download the object directly. + */ + if (this.treesWithRecentlyDownloadedCommits.TryGetValue(objectSha, out string commitSha) + && this.gitObjects.TryDownloadCommit(commitSha)) + { + this.treesWithRecentlyDownloadedCommits.Remove(objectSha); + response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.SuccessResult); + // FUTURE: Should the stats be updated to reflect all the trees in the pack? + // FUTURE: Should we try to clean up duplicate trees or increase depth of the commit download? + } + else if (this.gitObjects.TryDownloadAndSaveObject(objectSha, GVFSGitObjects.RequestSource.NamedPipeMessage) == GitObjects.DownloadAndSaveObjectResult.Success) { response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.SuccessResult); } @@ -513,9 +530,20 @@ private void HandleDownloadObjectRequest(NamedPipeMessages.Message message, Name response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.DownloadFailed); } - bool isBlob; - this.context.Repository.TryGetIsBlob(objectSha, out isBlob); - this.context.Repository.GVFSLock.Stats.RecordObjectDownload(isBlob, downloadTime.ElapsedMilliseconds); + Native.ObjectTypes? objectType; + this.context.Repository.TryGetObjectType(objectSha, out objectType); + this.context.Repository.GVFSLock.Stats.RecordObjectDownload(objectType == Native.ObjectTypes.Blob, downloadTime.ElapsedMilliseconds); + + if (objectType == Native.ObjectTypes.Commit + && !this.context.Repository.CommitAndRootTreeExists(objectSha, out var treeSha) + && !string.IsNullOrEmpty(treeSha)) + { + /* If a commit is downloaded but its tree doesn't exist, it means the commit hadn't been prefetched and all its trees + * may be needed soon depending on the context. e.g. git log (without a pathspec) doesn't need trees, but git checkout does. + * save the tree/commit so if the tree is requested soon we can download all the trees for the commit in a batch. + */ + this.treesWithRecentlyDownloadedCommits[treeSha] = objectSha; + } } } diff --git a/GVFS/GVFS.UnitTests/Mock/Git/MockLibGit2Repo.cs b/GVFS/GVFS.UnitTests/Mock/Git/MockLibGit2Repo.cs index c2494ba3f9..9901995617 100644 --- a/GVFS/GVFS.UnitTests/Mock/Git/MockLibGit2Repo.cs +++ b/GVFS/GVFS.UnitTests/Mock/Git/MockLibGit2Repo.cs @@ -12,8 +12,9 @@ public MockLibGit2Repo(ITracer tracer) { } - public override bool CommitAndRootTreeExists(string commitish) + public override bool CommitAndRootTreeExists(string commitish, out string treeSha) { + treeSha = string.Empty; return false; } diff --git a/GVFS/GVFS/CommandLine/GVFSVerb.cs b/GVFS/GVFS/CommandLine/GVFSVerb.cs index cdf9e920e6..a9fcb95876 100644 --- a/GVFS/GVFS/CommandLine/GVFSVerb.cs +++ b/GVFS/GVFS/CommandLine/GVFSVerb.cs @@ -663,7 +663,7 @@ protected bool TryDownloadCommit( out string error, bool checkLocalObjectCache = true) { - if (!checkLocalObjectCache || !repo.CommitAndRootTreeExists(commitId)) + if (!checkLocalObjectCache || !repo.CommitAndRootTreeExists(commitId, out _)) { if (!gitObjects.TryDownloadCommit(commitId)) { From 8ac9ad03b73fa1ea9a4ae669c3ab5770b017dfa4 Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Thu, 10 Jul 2025 11:04:03 -0700 Subject: [PATCH 2/2] Revise commit batch - limit to once per 5 minutes - disable if any prefetch of commit graph has already succeeded --- GVFS/GVFS.Mount/InProcessMount.cs | 49 ++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/GVFS/GVFS.Mount/InProcessMount.cs b/GVFS/GVFS.Mount/InProcessMount.cs index 79fa29e9e7..a9dd95b708 100644 --- a/GVFS/GVFS.Mount/InProcessMount.cs +++ b/GVFS/GVFS.Mount/InProcessMount.cs @@ -14,6 +14,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.Linq; using System.Text; using System.Threading; using static GVFS.Common.Git.LibGit2Repo; @@ -45,7 +46,8 @@ public class InProcessMount private HeartbeatThread heartbeat; private ManualResetEvent unmountEvent; - private readonly Dictionary treesWithRecentlyDownloadedCommits = new Dictionary(); + private readonly Dictionary treesWithDownloadedCommits = new Dictionary(); + private DateTime lastCommitPackDownloadTime = DateTime.MinValue; // True if InProcessMount is calling git reset as part of processing // a folder dehydrate request @@ -513,10 +515,10 @@ private void HandleDownloadObjectRequest(NamedPipeMessages.Message message, Name * * Otherwise, or as a fallback if the commit download fails, download the object directly. */ - if (this.treesWithRecentlyDownloadedCommits.TryGetValue(objectSha, out string commitSha) + if (this.ShouldDownloadCommitPack(objectSha, out string commitSha) && this.gitObjects.TryDownloadCommit(commitSha)) { - this.treesWithRecentlyDownloadedCommits.Remove(objectSha); + this.DownloadedCommitPack(objectSha: objectSha, commitSha: commitSha); response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.SuccessResult); // FUTURE: Should the stats be updated to reflect all the trees in the pack? // FUTURE: Should we try to clean up duplicate trees or increase depth of the commit download? @@ -530,19 +532,25 @@ private void HandleDownloadObjectRequest(NamedPipeMessages.Message message, Name response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.DownloadFailed); } + Native.ObjectTypes? objectType; this.context.Repository.TryGetObjectType(objectSha, out objectType); this.context.Repository.GVFSLock.Stats.RecordObjectDownload(objectType == Native.ObjectTypes.Blob, downloadTime.ElapsedMilliseconds); if (objectType == Native.ObjectTypes.Commit + && !this.PrefetchHasBeenDone() && !this.context.Repository.CommitAndRootTreeExists(objectSha, out var treeSha) && !string.IsNullOrEmpty(treeSha)) { - /* If a commit is downloaded but its tree doesn't exist, it means the commit hadn't been prefetched and all its trees - * may be needed soon depending on the context. e.g. git log (without a pathspec) doesn't need trees, but git checkout does. - * save the tree/commit so if the tree is requested soon we can download all the trees for the commit in a batch. + /* If a commit is downloaded, it wasn't prefetched. + * If any prefetch has been done, there is probably a commit in the prefetch packs that is close enough that + * loose object download of missing trees will be faster than downloading a pack of all the trees for the commit. + * Otherwise, the trees for the commit may be needed soon depending on the context. + * e.g. git log (without a pathspec) doesn't need trees, but git checkout does. + * + * Save the tree/commit so if the tree is requested soon we can download all the trees for the commit in a batch. */ - this.treesWithRecentlyDownloadedCommits[treeSha] = objectSha; + this.treesWithDownloadedCommits[treeSha] = objectSha; } } } @@ -550,6 +558,33 @@ private void HandleDownloadObjectRequest(NamedPipeMessages.Message message, Name connection.TrySendResponse(response.CreateMessage()); } + private bool PrefetchHasBeenDone() + { + var prefetchPacks = this.gitObjects.ReadPackFileNames(this.enlistment.GitPackRoot, GVFSConstants.PrefetchPackPrefix); + return prefetchPacks.Length > 0; + } + + private bool ShouldDownloadCommitPack(string objectSha, out string commitSha) + { + + if (!this.treesWithDownloadedCommits.TryGetValue(objectSha, out commitSha) + || this.PrefetchHasBeenDone()) + { + return false; + } + + /* This is a heuristic to prevent downloading multiple packs related to git history commands, + * since commits downloaded close together likely have similar trees. */ + var timePassed = DateTime.UtcNow - this.lastCommitPackDownloadTime; + return (timePassed > TimeSpan.FromMinutes(5)); + } + + private void DownloadedCommitPack(string objectSha, string commitSha) + { + this.lastCommitPackDownloadTime = DateTime.UtcNow; + this.treesWithDownloadedCommits.Remove(objectSha); + } + private void HandlePostFetchJobRequest(NamedPipeMessages.Message message, NamedPipeServer.Connection connection) { NamedPipeMessages.RunPostFetchJob.Request request = new NamedPipeMessages.RunPostFetchJob.Request(message);