[Vsintegration] Cut UI-thread marshalling and code model database lock contention

src/VisualStudio/ProjectBase/Automation/VSProject/OAProjectReference.cs

@@ -62,28 +62,59 @@ public override EnvDTE.Project SourceProject
         {
             get
             {
-                if (Guid.Empty == BaseReferenceNode.ReferencedProjectGuid)
+                var referencedGuid = BaseReferenceNode.ReferencedProjectGuid;
+                if (Guid.Empty == referencedGuid)
                 {
                     return null;
                 }
                 if (BaseReferenceNode.ProjectMgr == null)
                 {
                     return null;
                 }
+                // The ProjectInfo is shared by every project reference that points at this
+                // project, and it is dropped or cleared when that project closes, unloads or
+                // reloads. So anything found here is both current and worth reusing, which
+                // keeps this resolution at once per project instead of once per reference.
+                // Register the entry when it is missing: the build dependency pass that
+                // normally creates it can run after this property is first read, and without
+                // an entry there is nowhere to cache and every caller resolves again.
+                var projectInfo = ProjectInfo.GetOrCreate(referencedGuid, BaseReferenceNode.Url);
+                var cached = projectInfo?.DteProject;
+                if (cached != null)
+                {
+                    return cached;
+                }
                 return ThreadHelper.JoinableTaskFactory.Run(async delegate
                 {
                     await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();
-                    IVsHierarchy hierarchy = VsShellUtilities.GetHierarchy(BaseReferenceNode.ProjectMgr.Site, BaseReferenceNode.ReferencedProjectGuid);
-                    Logger.Information($"OAProjectReference: GetHierarchy for project reference {BaseReferenceNode.ReferencedProjectGuid} returned {(hierarchy != null ? "a hierarchy" : "null")}");
+                    IVsHierarchy hierarchy = projectInfo?.Hierarchy;
+                    if (hierarchy == null)
+                    {
+                        hierarchy = VsShellUtilities.GetHierarchy(BaseReferenceNode.ProjectMgr.Site, referencedGuid);
+                        Logger.Information($"OAProjectReference: Resolved hierarchy for project reference {referencedGuid} through the shell");
+                    }
+                    Logger.Information($"OAProjectReference: GetHierarchy for project reference {referencedGuid} returned {(hierarchy != null ? "a hierarchy" : "null")}");
                     if (null == hierarchy)
                     {
                         return null;
                     }
+                    // Cache the hierarchy even when the automation object below cannot be
+                    // obtained: the hierarchy is what costs a solution wide lookup, and
+                    // leaving it unstored made every later caller pay for it again.
+                    if (projectInfo != null)
+                    {
+                        projectInfo.Hierarchy = hierarchy;
+                    }
                     object extObject;
                     if (Microsoft.VisualStudio.ErrorHandler.Succeeded(
-                            hierarchy.GetProperty(VSConstants.VSITEMID_ROOT, (int)__VSHPROPID.VSHPROPID_ExtObject, out extObject)))
+                            hierarchy.GetProperty(VSConstants.VSITEMID_ROOT, (int)__VSHPROPID.VSHPROPID_ExtObject, out extObject))
+                        && extObject is EnvDTE.Project project)
                     {
-                        return extObject as EnvDTE.Project;
+                        if (projectInfo != null)
+                        {
+                            projectInfo.DteProject = project;
+                        }
+                        return project;
                     }
                     return null;
                 });

src/VisualStudio/ProjectBase/ProjectInfo.cs

@@ -17,7 +17,50 @@ public class ProjectInfo
     {
         public string Url { get; private set; }
         public Guid Id { get; private set; }
-        public IVsHierarchy Hierarchy { get; set; } = null;
+
+        private IVsHierarchy _hierarchy = null;
+        private EnvDTE.Project _dteProject = null;
+
+        /// <summary>
+        /// The hierarchy of this project, once somebody has resolved it.
+        /// </summary>
+        /// <remarks>
+        /// Resolving a hierarchy from a guid enumerates the whole solution, so it is well
+        /// worth caching. Setting this to null is how close, unload and reload invalidate
+        /// the entry - see the ClearHierarchy methods.
+        /// </remarks>
+        public IVsHierarchy Hierarchy
+        {
+            get { return _hierarchy; }
+            set
+            {
+                _hierarchy = value;
+                if (value == null)
+                {
+                    // Invalidation: the automation object was resolved from the hierarchy
+                    // we are dropping, so it has to go too. Replacing it with another non
+                    // null hierarchy is NOT an invalidation: the shell can hand out a
+                    // different runtime wrapper for the very same project, and treating
+                    // that as a change threw the cache away on almost every write.
+                    _dteProject = null;
+                }
+            }
+        }
+
+        /// <summary>
+        /// The automation object of this project, resolved from <see cref="Hierarchy"/>.
+        /// </summary>
+        /// <remarks>
+        /// Shared by every project reference that points at this project, so the resolution
+        /// happens once per project instead of once per reference node - on a solution with
+        /// 226 projects that is 88 resolutions instead of 3283. It is dropped whenever
+        /// <see cref="Hierarchy"/> is cleared, so it cannot outlive the project it belongs to.
+        /// </remarks>
+        public EnvDTE.Project DteProject
+        {
+            get { return _hierarchy == null ? null : _dteProject; }
+            set { _dteProject = value; }
+        }
 #if DEBUG
         public string Name => System.IO.Path.GetFileNameWithoutExtension(Url);
 
@@ -64,6 +107,71 @@ public static ProjectInfo GetProjectInfo(string url, Guid guid)
             return result;
         }
 
+        /// <summary>
+        /// Forget the cached hierarchy of every ProjectInfo that points to it.
+        /// </summary>
+        /// <remarks>
+        /// Readers of <see cref="Hierarchy"/> take a non null value as proof that the project is
+        /// still loaded, so it has to be dropped as soon as that project is closed, unloaded or
+        /// reloaded. Removing the whole ProjectInfo only happens for our own project nodes
+        /// (ProjectNode.Close()), so foreign projects need this. Clearing too eagerly costs
+        /// nothing: the next reader resolves the hierarchy through the shell and caches it again.
+        /// </remarks>
+        public static void ClearHierarchy(IVsHierarchy hierarchy)
+        {
+            if (hierarchy == null)
+            {
+                return;
+            }
+            foreach (var projectInfo in _projectsByUrl.Values)
+            {
+                if (ReferenceEquals(projectInfo.Hierarchy, hierarchy))
+                {
+                    Logger.Information($"Dropping cached hierarchy for {projectInfo.Url} with guid {projectInfo.Id}");
+                    projectInfo.Hierarchy = null;
+                }
+            }
+        }
+
+        /// <summary>
+        /// Forget all cached hierarchies, for when the whole solution goes away.
+        /// </summary>
+        public static void ClearHierarchies()
+        {
+            Logger.Information("Dropping all cached project hierarchies");
+            foreach (var projectInfo in _projectsByUrl.Values)
+            {
+                projectInfo.Hierarchy = null;
+            }
+        }
+
+        /// <summary>
+        /// Find the entry for a project, registering one when it does not exist yet.
+        /// </summary>
+        /// <remarks>
+        /// ProjectNode.CreateBuildDependencies normally registers these, but it can run
+        /// after the automation layer has already started reading project references.
+        /// Anything that wants to cache per project needs an entry to cache on, so it
+        /// creates one here instead of giving up and resolving again on every call: one
+        /// session logged 253 solution wide hierarchy lookups for each of 88 projects
+        /// purely because the entries did not exist yet.
+        /// Two threads racing here end up with two equivalent entries and the last one
+        /// wins, which costs at most one extra resolution.
+        /// </remarks>
+        public static ProjectInfo GetOrCreate(Guid id, string url)
+        {
+            if (id == Guid.Empty || string.IsNullOrEmpty(url))
+            {
+                return null;
+            }
+            var result = GetProjectInfo(url, id);
+            if (result == null)
+            {
+                result = new ProjectInfo(id, url);
+            }
+            return result;
+        }
+
         public static void RemoveProjectInfo(string url, Guid id)
         {
             var projectInfo = GetProjectInfo(url, id);

src/VisualStudio/ProjectBase/SolutionListenerForProjectReferenceUpdate.cs

@@ -44,6 +44,9 @@ public override int OnBeforeCloseProject(IVsHierarchy hierarchy, int removed)
             ThreadHelper.ThrowIfNotOnUIThread();
             Logger.Information($"OnBeforeCloseProject: Project {hierarchy} is being closed. Is it being removed from the solution? {(removed != 0 ? "Yes" : "No")}");
 
+            // This also fires for projects that are not ours, which never run through ProjectNode.Close()
+            ProjectInfo.ClearHierarchy(hierarchy);
+
             if (removed != 0)
             {
                 List<ProjectReferenceNode> projectReferences = this.GetProjectReferencesContainingThisProject(hierarchy);
@@ -71,6 +74,9 @@ public override int OnAfterLoadProject(IVsHierarchy stubHierarchy, IVsHierarchy
         {
             ThreadHelper.ThrowIfNotOnUIThread();
 
+            // The stub is replaced by realHierarchy, so anything cached for it is stale now
+            ProjectInfo.ClearHierarchy(stubHierarchy);
+
             List<ProjectReferenceNode> projectReferences = this.GetProjectReferencesContainingThisProject(realHierarchy);
             Logger.Information($"OnAfterLoadProject:Project {realHierarchy} is being loaded. Updating {projectReferences.Count} project references that point to it.");
 
@@ -145,10 +151,21 @@ public override int OnAfterRenameProject(IVsHierarchy hierarchy)
         }
 
 
+        public override int OnAfterCloseSolution(object reserved)
+        {
+            // Nothing that was cached while the solution was open can be trusted anymore
+            ProjectInfo.ClearHierarchies();
+            return VSConstants.S_OK;
+        }
+
+
         public override int OnBeforeUnloadProject(IVsHierarchy realHierarchy, IVsHierarchy stubHierarchy)
         {
             ThreadHelper.ThrowIfNotOnUIThread();
 
+            // realHierarchy is going away, and this also fires for projects that are not ours
+            ProjectInfo.ClearHierarchy(realHierarchy);
+
             List<ProjectReferenceNode> projectReferences = this.GetProjectReferencesContainingThisProject(realHierarchy);
 
             Logger.Information($"OnBeforeUnloadProject: Project {realHierarchy} is being unloaded. Updating {projectReferences.Count} project references that point to it.");