[pull] master from microsoft:master

GVFS/GVFS.Common/Git/GitRepo.cs

@@ -113,6 +113,21 @@ public virtual bool TryGetBlobLength(string blobSha, out long size)
             return this.GetLooseBlobState(blobSha, null, out size) == LooseBlobState.Exists;
         }
 
+        /// <summary>
+        /// Try to find the SHAs of subtrees missing from the given tree.
+        /// </summary>
+        /// <param name="treeSha">Tree to look up</param>
+        /// <param name="subtrees">SHAs of subtrees of this tree which are not downloaded yet.</param>
+        /// <returns></returns>
+        public virtual bool TryGetMissingSubTrees(string treeSha, out string[] subtrees)
+        {
+            string[] missingSubtrees = null;
+            var succeeded = this.libgit2RepoInvoker.TryInvoke(repo =>
+                repo.GetMissingSubTrees(treeSha), out missingSubtrees);
+            subtrees = missingSubtrees;
+            return succeeded;
+        }
+
         public void Dispose()
         {
             if (this.libgit2RepoInvoker != null)

GVFS/GVFS.Common/Git/LibGit2Repo.cs

@@ -1,5 +1,6 @@
 using GVFS.Common.Tracing;
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Runtime.InteropServices;
 
@@ -147,6 +148,82 @@ public virtual bool TryCopyBlob(string sha, Action<Stream, long> writeAction)
             return true;
         }
 
+        /// <summary>
+        /// Get the list of missing subtrees for the given treeSha.
+        /// </summary>
+        /// <param name="treeSha">Tree to look up</param>
+        /// <param name="missingSubtrees">SHAs of subtrees of this tree which are not downloaded yet.</param>
+        public virtual string[] GetMissingSubTrees(string treeSha)
+        {
+            List<string> missingSubtreesList = new List<string>();
+            IntPtr treeHandle;
+            if (Native.RevParseSingle(out treeHandle, this.RepoHandle, treeSha) != Native.SuccessCode
+                || treeHandle == IntPtr.Zero)
+            {
+                return Array.Empty<string>();
+            }
+
+            try
+            {
+                if (Native.Object.GetType(treeHandle) != Native.ObjectTypes.Tree)
+                {
+                    return Array.Empty<string>();
+                }
+
+                uint entryCount = Native.Tree.GetEntryCount(treeHandle);
+                for (uint i = 0; i < entryCount; i++)
+                {
+                    if (this.IsMissingSubtree(treeHandle, i, out string entrySha))
+                    {
+                        missingSubtreesList.Add(entrySha);
+                    }
+                }
+            }
+            finally
+            {
+                Native.Object.Free(treeHandle);
+            }
+
+            return missingSubtreesList.ToArray();
+        }
+
+        /// <summary>
+        /// Determine if the given index of a tree is a subtree and if it is missing.
+        /// If it is a missing subtree, return the SHA of the subtree.
+        /// </summary>
+        private bool IsMissingSubtree(IntPtr treeHandle, uint i, out string entrySha)
+        {
+            entrySha = null;
+            IntPtr entryHandle = Native.Tree.GetEntryByIndex(treeHandle, i);
+            if (entryHandle == IntPtr.Zero)
+            {
+                return false;
+            }
+
+            var entryMode = Native.Tree.GetEntryFileMode(entryHandle);
+            if (entryMode != Native.Tree.TreeEntryFileModeDirectory)
+            {
+                return false;
+            }
+
+            var entryId = Native.Tree.GetEntryId(entryHandle);
+            if (entryId == IntPtr.Zero)
+            {
+                return false;
+            }
+
+            var rawEntrySha = Native.IntPtrToGitOid(entryId);
+            entrySha = rawEntrySha.ToString();
+
+            if (this.ObjectExists(entrySha))
+            {
+                return false;
+            }
+            return true;
+            /* Both the entryHandle and the entryId handle are owned by the treeHandle, so we shouldn't free them or it will lead to corruption of the later entries */
+        }
+
+
         public void Dispose()
         {
             this.Dispose(true);
@@ -247,6 +324,26 @@ public static class Blob
                 [DllImport(Git2NativeLibName, EntryPoint = "git_blob_rawcontent")]
                 public static unsafe extern byte* GetRawContent(IntPtr objectHandle);
             }
+
+            public static class Tree
+            {
+                [DllImport(Git2NativeLibName, EntryPoint = "git_tree_entrycount")]
+                public static extern uint GetEntryCount(IntPtr treeHandle);
+
+                [DllImport(Git2NativeLibName, EntryPoint = "git_tree_entry_byindex")]
+                public static extern IntPtr GetEntryByIndex(IntPtr treeHandle, uint index);
+
+                [DllImport(Git2NativeLibName, EntryPoint = "git_tree_entry_id")]
+                public static extern IntPtr GetEntryId(IntPtr entryHandle);
+
+                /* git_tree_entry_type requires the object to exist, so we can't use it to check if
+                 * a missing entry is a tree. Instead, we can use the file mode to determine if it is a tree. */
+                [DllImport(Git2NativeLibName, EntryPoint = "git_tree_entry_filemode")]
+                public static extern uint GetEntryFileMode(IntPtr entryHandle);
+
+                public const uint TreeEntryFileModeDirectory = 0x4000;
+
+            }
         }
     }
 }

GVFS/GVFS.Mount/InProcessMount.cs

@@ -27,6 +27,12 @@ public class InProcessMount
         private const int MaxPipeNameLength = 250;
         private const int MutexMaxWaitTimeMS = 500;
 
+        // This is value chosen based on tested scenarios to limit the required download time for
+        // all the trees. This is approximately the amount of trees that can be downloaded in 1 second.
+        // Downloading an entire commit pack also takes around 1 second, so this should limit downloading
+        // all the trees in a commit to ~2-3 seconds.
+        private const int MissingTreeThresholdForDownloadingCommitPack = 200;
+
         private readonly bool showDebugWindow;
 
         private FileSystemCallbacks fileSystemCallbacks;
@@ -47,7 +53,6 @@ public class InProcessMount
         private ManualResetEvent unmountEvent;
 
         private readonly Dictionary<string, string> treesWithDownloadedCommits = new Dictionary<string,string>();
-        private DateTime lastCommitPackDownloadTime = DateTime.MinValue;
 
         // True if InProcessMount is calling git reset as part of processing
         // a folder dehydrate request
@@ -518,13 +523,14 @@ private void HandleDownloadObjectRequest(NamedPipeMessages.Message message, Name
                     if (this.ShouldDownloadCommitPack(objectSha, out string commitSha)
                         && this.gitObjects.TryDownloadCommit(commitSha))
                     {
-                        this.DownloadedCommitPack(objectSha: objectSha, commitSha: commitSha);
+                        this.DownloadedCommitPack(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?
                     }
                     else if (this.gitObjects.TryDownloadAndSaveObject(objectSha, GVFSGitObjects.RequestSource.NamedPipeMessage) == GitObjects.DownloadAndSaveObjectResult.Success)
                     {
+                        this.UpdateTreesForDownloadedCommits(objectSha);
                         response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.SuccessResult);
                     }
                     else
@@ -548,7 +554,7 @@ private void HandleDownloadObjectRequest(NamedPipeMessages.Message message, Name
                          * 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.
+                         * Save the tree/commit so if more trees are requested we can download all the trees for the commit in a batch.
                          */
                         this.treesWithDownloadedCommits[treeSha] = objectSha;
                     }
@@ -561,28 +567,67 @@ private void HandleDownloadObjectRequest(NamedPipeMessages.Message message, Name
         private bool PrefetchHasBeenDone()
         {
             var prefetchPacks = this.gitObjects.ReadPackFileNames(this.enlistment.GitPackRoot, GVFSConstants.PrefetchPackPrefix);
-            return prefetchPacks.Length > 0;
+            var result = prefetchPacks.Length > 0;
+            if (result)
+            {
+                this.treesWithDownloadedCommits.Clear();
+            }
+            return result;
         }
 
         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));
+            /* This is a heuristic to prevent downloading multiple packs related to git history commands.
+             * Closely related commits are likely to have similar trees, so we'll find fewer missing trees in them.
+             * Conversely, if we know (from previously downloaded missing trees) that a commit has a lot of missing
+             * trees left, we'll probably need to download many more trees for the commit so we should download the pack.
+             */
+            var commitShaLocal = commitSha; // can't use out parameter in lambda
+            int missingTreeCount = this.treesWithDownloadedCommits.Where(x => x.Value == commitShaLocal).Count();
+            return missingTreeCount > MissingTreeThresholdForDownloadingCommitPack;
+        }
+
+        private void UpdateTreesForDownloadedCommits(string objectSha)
+        {
+            /* If we are downloading missing trees, we probably are missing more trees for the commit.
+             * Update our list of trees associated with the commit so we can use the # of missing trees 
+             * as a heuristic to decide whether to batch download all the trees for the commit the
+             * next time a missing one is requested.
+             */
+            if (!this.treesWithDownloadedCommits.TryGetValue(objectSha, out var commitSha)
+                || this.PrefetchHasBeenDone())
+            {
+                return;
+            }
+
+            if (!this.context.Repository.TryGetObjectType(objectSha, out var objectType)
+                || objectType != Native.ObjectTypes.Tree)
+            {
+                return;
+            }
+
+            if (this.context.Repository.TryGetMissingSubTrees(objectSha, out var missingSubTrees))
+            {
+                foreach (var missingSubTree in missingSubTrees)
+                {
+                    this.treesWithDownloadedCommits[missingSubTree] = commitSha;
+                }
+            }
         }
 
-        private void DownloadedCommitPack(string objectSha, string commitSha)
+        private void DownloadedCommitPack(string commitSha)
         {
-            this.lastCommitPackDownloadTime = DateTime.UtcNow;
-            this.treesWithDownloadedCommits.Remove(objectSha);
+            var toRemove = this.treesWithDownloadedCommits.Where(x => x.Value == commitSha).ToList();
+            foreach (var tree in toRemove)
+            {
+                this.treesWithDownloadedCommits.Remove(tree.Key);
+            }
         }
 
         private void HandlePostFetchJobRequest(NamedPipeMessages.Message message, NamedPipeServer.Connection connection)