From 0386f67fcfaae48aa1326c1195018bc326f82c6c Mon Sep 17 00:00:00 2001 From: Hansjoerg Petriffer Date: Fri, 18 Sep 2026 16:17:59 +0200 Subject: [PATCH 1/6] [Vsintegration] Cache resolved project reference hierarchies and automation objects OAProjectReference.SourceProject resolved its hierarchy through VsShellUtilities.GetHierarchy on every single read, with no caching, and each read also entered a JoinableTaskFactory.Run that marshals to the UI thread. BuildDependency.GetReferencedHierarchy already cached the same lookup on ProjectInfo.Hierarchy; SourceProject simply ignored it. The intellisense logs show how much that cost: 2,388,244 of these lookups across ~150 logged sessions, 234,360 in a single 11 minute session and 8,928 in one second. Every one is a UI thread marshal, which is what makes the IDE unresponsive while a solution loads. SourceProject now reuses ProjectInfo.Hierarchy and stores the resolved EnvDTE project on the new ProjectInfo.DteProject, shared by every reference node pointing at the same project. Invalidation goes through the existing ProjectInfo lifetime: ClearHierarchy/ClearHierarchies are called from SolutionListenerForProjectReferenceUpdate on project close, unload, reload and solution close. That listener sees every project in the solution, unlike SolutionListenerForProjectEvents which filters to our own hierarchies, so foreign project references are covered too. This also closes a pre-existing hole where BuildDependency could keep using the hierarchy of a foreign project that had been unloaded. Measured on RadixWf.sln (226 projects, 3283 project reference nodes, 88 distinct referenced projects), same scenario before and after, with more editor activity in the after run: OAProjectReference: GetHierarchy 13,183 -> 88 calls (-99.3%) solution wide shell lookups all of them -> 0 load burst wall clock 6.54s -> 2.23s 88 calls over 88 distinct projects, at most one per project, which is the floor. Note that reaching it depends on the ProjectInfo entries already existing when SourceProject is first read; when the build dependency pass runs later instead, this commit alone degrades to repeated lookups. A follow-up commit removes that ordering dependency by registering the entry on demand. Two things to keep in mind when touching this code: - Do not use ReferenceEquals on the hierarchy to decide invalidation. The shell hands out a different runtime callable wrapper for the same project, so comparing wrappers treats almost every write as a change and throws the cache away. DteProject is therefore cleared only when Hierarchy is set to null, which is what the ClearHierarchy methods do. - Store the hierarchy as soon as it is resolved, not only in the branch where the automation object comes back, otherwise later callers pay for the solution wide lookup again. The unload and reload invalidation paths were exercised afterwards by unloading and reloading two heavily referenced projects (AcsDef, 89 referencing projects; AcsLib, 86): ClearHierarchy fires before the rest of the unload handling, OnAfterLoadProject fires on reload, and no COM or other exceptions appear. All measurements are from VS 2022 Experimental. Co-Authored-By: Claude Opus 5 --- .../VSProject/OAProjectReference.cs | 38 +++++++-- src/VisualStudio/ProjectBase/ProjectInfo.cs | 83 ++++++++++++++++++- ...lutionListenerForProjectReferenceUpdate.cs | 17 ++++ 3 files changed, 132 insertions(+), 6 deletions(-) diff --git a/src/VisualStudio/ProjectBase/Automation/VSProject/OAProjectReference.cs b/src/VisualStudio/ProjectBase/Automation/VSProject/OAProjectReference.cs index 9f6578362e..0d2b96ea59 100644 --- a/src/VisualStudio/ProjectBase/Automation/VSProject/OAProjectReference.cs +++ b/src/VisualStudio/ProjectBase/Automation/VSProject/OAProjectReference.cs @@ -62,7 +62,8 @@ public override EnvDTE.Project SourceProject { get { - if (Guid.Empty == BaseReferenceNode.ReferencedProjectGuid) + var referencedGuid = BaseReferenceNode.ReferencedProjectGuid; + if (Guid.Empty == referencedGuid) { return null; } @@ -70,20 +71,47 @@ public override EnvDTE.Project SourceProject { 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. + var projectInfo = ProjectInfo.GetProjectInfo(referencedGuid); + 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; }); diff --git a/src/VisualStudio/ProjectBase/ProjectInfo.cs b/src/VisualStudio/ProjectBase/ProjectInfo.cs index 3264aa34b7..914afb4c62 100644 --- a/src/VisualStudio/ProjectBase/ProjectInfo.cs +++ b/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; + + /// + /// The hierarchy of this project, once somebody has resolved it. + /// + /// + /// 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. + /// + 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; + } + } + } + + /// + /// The automation object of this project, resolved from . + /// + /// + /// 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 + /// is cleared, so it cannot outlive the project it belongs to. + /// + 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,44 @@ public static ProjectInfo GetProjectInfo(string url, Guid guid) return result; } + /// + /// Forget the cached hierarchy of every ProjectInfo that points to it. + /// + /// + /// Readers of 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. + /// + 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; + } + } + } + + /// + /// Forget all cached hierarchies, for when the whole solution goes away. + /// + public static void ClearHierarchies() + { + Logger.Information("Dropping all cached project hierarchies"); + foreach (var projectInfo in _projectsByUrl.Values) + { + projectInfo.Hierarchy = null; + } + } + public static void RemoveProjectInfo(string url, Guid id) { var projectInfo = GetProjectInfo(url, id); diff --git a/src/VisualStudio/ProjectBase/SolutionListenerForProjectReferenceUpdate.cs b/src/VisualStudio/ProjectBase/SolutionListenerForProjectReferenceUpdate.cs index 0c4d35f73b..3a4e872e44 100644 --- a/src/VisualStudio/ProjectBase/SolutionListenerForProjectReferenceUpdate.cs +++ b/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 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 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 projectReferences = this.GetProjectReferencesContainingThisProject(realHierarchy); Logger.Information($"OnBeforeUnloadProject: Project {realHierarchy} is being unloaded. Updating {projectReferences.Count} project references that point to it."); From eb02d7e5a1d44dfba24f3c3d71f6bc8b160f5b67 Mon Sep 17 00:00:00 2001 From: Hansjoerg Petriffer Date: Fri, 18 Sep 2026 15:30:09 +0200 Subject: [PATCH 2/6] [Vsintegration] Stop the code model database backup from blocking everything The 5 minute backup of the in memory code model database held the global lock on the connection for its whole duration, and nothing stopped several backups from queueing up behind each other. CommitWhenNeeded is called from every write path in XDatabase, and lastWritten was only updated once SaveToDisk had finished. So every caller inside the 5 minute window still saw a stale timestamp, started its own BackgroundWorker, and they all serialized on the lock. A cold model walk in August shows the result: five backups running back to back from 10:00:03 to 10:00:35, half a minute in which no parse or lookup could touch the code model. Changes: - CommitWhenNeeded claims the interval when it schedules the backup instead of when the backup completes, and an Interlocked guard admits only one at a time. The guard is the second line of defence for the race where two threads pass the time check together; the timestamp handles the common case. - SaveToDisk now takes the lock on oConn only around BackupDatabase, which is the one call that touches the in memory connection. Deleting the old file, opening the disk database and the VACUUM all work on the disk side and no longer block the code model. The lock moved into SaveToDisk so that all four callers behave the same way; the other three never locked at all. - SafeFileDelete slept tries*100 ms before even the first attempt, costing 100 ms on every backup for nothing. It now only backs off between retries. - TimeSpan.Minutes is the minutes component, so "Minutes >= 5 .or. Hours > 0" was a workaround for the wrong property. Use TotalMinutes. Measured on a cold walk of RadixWf.sln that rebuilt the whole database (157,628 parses, 39,387 database writes, 2.6x the write volume of the August baseline it is compared against): backups 6 -> 3, no two adjacent (gaps of exactly 300.00s) total time spent backing up 43.5s -> 12.3s longest single backup 13.6s -> 5.2s chained backups 5 in a row -> none UpdateFileContents lock hold p50 83 -> 58ms, p95 163 -> 105ms, p99 359 -> 173ms, max 32.5s -> 5.2s calls holding the lock > 1s 17 -> 3 Note that the comparison is against a historical log from VS 18 rather than a controlled run of the old code on the same machine, so treat the percentages as indicative. The elimination of chaining is structural rather than statistical. Still open: three unrelated writes held the lock for ~5s simultaneously during the walk without any backup running, so the lock itself is worth a second look. Co-Authored-By: Claude Opus 5 --- .../XSharpCodeModelXs/Database/XDatabase.prg | 65 +++++++++++++------ 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/src/VisualStudio/XSharpCodeModelXs/Database/XDatabase.prg b/src/VisualStudio/XSharpCodeModelXs/Database/XDatabase.prg index 4aea94747f..5a63abaa07 100644 --- a/src/VisualStudio/XSharpCodeModelXs/Database/XDatabase.prg +++ b/src/VisualStudio/XSharpCodeModelXs/Database/XDatabase.prg @@ -24,6 +24,7 @@ STATIC CLASS XDatabase STATIC PRIVATE oConn AS DbConnection // In memory database ! STATIC PRIVATE lastWritten := DateTime.MinValue AS DateTime + STATIC PRIVATE backupRunning := 0 AS LONG // guards against overlapping backups STATIC PRIVATE currentFile AS STRING STATIC PROPERTY FileName as STRING GET currentFile STATIC PROPERTY DeleteOnClose as LOGIC AUTO @@ -173,12 +174,17 @@ STATIC METHOD SafeFileDelete(cFile as STRING) AS VOID var deleted := false do while tries < 4 .and. !deleted try - System.Threading.Thread.Sleep(tries * 100) File.Delete(cFile) deleted := true catch as IOException Log(i"Failed to delete file {cFile}, attempts {tries}") tries++ + // Only back off when we are actually going to try again. This used to + // sleep before the first attempt as well, which cost 100 ms on every + // single backup for nothing. + if tries < 4 + System.Threading.Thread.Sleep(tries * 100) + endif end try enddo if ! deleted @@ -188,11 +194,18 @@ STATIC METHOD SafeFileDelete(cFile as STRING) AS VOID STATIC METHOD SaveToDisk(oConn AS DbConnection, cFile AS STRING) AS VOID CHECKIFOPEN + // Only the BackupDatabase call below touches the in memory connection. Deleting + // the old file, opening the disk database and vacuuming it all work on the disk + // side, so they must stay outside the lock on oConn: that lock serializes the + // whole code model, and holding it for a full backup plus a VACUUM blocked every + // parse and every lookup for seconds at a time. Log(i"SafeDelete file {cFile}") SafeFileDelete(cFile) USING VAR diskdb := OpenFile(cFile) Log(i"Save DB to disk {cFile}") - oConn:BackupDatabase(diskdb, "main") + BEGIN LOCK oConn + oConn:BackupDatabase(diskdb, "main") + END LOCK USING VAR oCmd := CreateCommand("VACUUM", diskdb) Log(i"Execute VACUUM command") oCmd:ExecuteNonQuery() @@ -211,27 +224,39 @@ STATIC METHOD CommitWhenNeeded() AS VOID VAR ts := DateTime.Now - lastWritten // Save to disk every 5 minutes Log(i"Time since last backup {ts}") - IF ts:Minutes >= 5 .OR. ts:Hours > 0 - LOCAL oBW AS BackgroundWorker - oBW := BackgroundWorker{} - oBW:DoWork += BackupInBackground - oBW:RunWorkerAsync() - + IF ts:TotalMinutes < 5 + RETURN ENDIF + // Let one backup run at a time. This is called from every write to the database, + // so without the guard each caller inside the 5 minute window starts its own + // BackgroundWorker and they all queue up behind each other: the logs show runs of + // 5 and 6 backups back to back, 32 seconds of the code model being unavailable. + IF System.Threading.Interlocked.CompareExchange(REF backupRunning, 1, 0) != 0 + Log("A backup is already running, skipping this one") + RETURN + ENDIF + // Claim the interval right away. SaveToDisk sets it again when it has finished, + // but until then everybody else must already see this interval as taken care of. + lastWritten := DateTime.Now + LOCAL oBW AS BackgroundWorker + oBW := BackgroundWorker{} + oBW:DoWork += BackupInBackground + oBW:RunWorkerAsync() STATIC METHOD BackupInBackground(sender AS OBJECT , args AS DoWorkEventArgs ) AS VOID - CHECKIFOPEN - BEGIN LOCK oConn - TRY - Log(i"Starting backup to {currentFile}") - SaveToDisk(oConn, currentFile ) - CATCH e AS Exception - Log(i"Error backing up to {currentFile}") - XSettings.Exception(e) - FINALLY - Log(i"Completed backup to {currentFile}") - END TRY - END LOCK + TRY + CHECKIFOPEN + Log(i"Starting backup to {currentFile}") + // SaveToDisk takes the lock on oConn itself, and only around the part that + // actually needs it. + SaveToDisk(oConn, currentFile ) + CATCH e AS Exception + Log(i"Error backing up to {currentFile}") + XSettings.Exception(e) + FINALLY + Log(i"Completed backup to {currentFile}") + System.Threading.Interlocked.Exchange(REF backupRunning, 0) + END TRY RETURN STATIC METHOD CreateSchema(Connection AS DbConnection) AS VOID From 92f7c80ec9e12a5435cb92a1c0ad8de74a0f85e3 Mon Sep 17 00:00:00 2001 From: Hansjoerg Petriffer Date: Fri, 18 Sep 2026 16:17:35 +0200 Subject: [PATCH 3/6] [Vsintegration] Register the ProjectInfo when a project reference is first read The cache added in the previous commit only works when a ProjectInfo already exists for the referenced project. ProjectNode.CreateBuildDependencies normally registers those, but it can run after the automation layer has started reading SourceProject. When it does there is nowhere to cache: GetProjectInfo returns null, nothing is stored, and every caller resolves the hierarchy through the shell again. That is a race, not a rare case. Two sessions, same solution, same build: ProjectInfos registered before the reads: 88 calls, 0 shell lookups ProjectInfos registered after the reads: 4256 calls, 4170 shell lookups (253 calls for each of 88 projects) The previous commit was measured on the first of those, so its numbers were a best case rather than the norm. ProjectInfo.GetOrCreate registers the entry when it is missing, using the same (guid, url) pair CreateBuildDependencies would use, so whichever side gets there first produces an equivalent entry. Guid.Empty is rejected, matching the rule in ProjectNode that avoids registering a url keyed entry before the guid is known. Two threads racing produce two equivalent entries and the last one wins, costing at most one extra resolution. Verified over three open / unload / reload / close cycles in one session: 86 of 88 projects resolved exactly once per cycle shell lookups 88, 92, 92 - stable, no ordering dependency left no growth across cycles (268, 268 calls; ProjectInfo create/remove 78/78) no cycle below 88 resolutions, so entries really are invalidated on solution close rather than carried into the next solution 0 errors The calls above 88 in later cycles all belong to the two projects that were unloaded: while a project is unloaded its hierarchy is a stub whose VSHPROPID_ExtObject is not an EnvDTE.Project, so there is nothing to cache and each referencing node re-enters once. The expensive part stays cached - shell lookups rose by 4, not by 180 - and it lasts only as long as the project is unloaded. Co-Authored-By: Claude Opus 5 --- .../VSProject/OAProjectReference.cs | 5 +++- src/VisualStudio/ProjectBase/ProjectInfo.cs | 27 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/VisualStudio/ProjectBase/Automation/VSProject/OAProjectReference.cs b/src/VisualStudio/ProjectBase/Automation/VSProject/OAProjectReference.cs index 0d2b96ea59..984e6fd594 100644 --- a/src/VisualStudio/ProjectBase/Automation/VSProject/OAProjectReference.cs +++ b/src/VisualStudio/ProjectBase/Automation/VSProject/OAProjectReference.cs @@ -75,7 +75,10 @@ public override EnvDTE.Project SourceProject // 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. - var projectInfo = ProjectInfo.GetProjectInfo(referencedGuid); + // 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) { diff --git a/src/VisualStudio/ProjectBase/ProjectInfo.cs b/src/VisualStudio/ProjectBase/ProjectInfo.cs index 914afb4c62..e47a84b408 100644 --- a/src/VisualStudio/ProjectBase/ProjectInfo.cs +++ b/src/VisualStudio/ProjectBase/ProjectInfo.cs @@ -145,6 +145,33 @@ public static void ClearHierarchies() } } + /// + /// Find the entry for a project, registering one when it does not exist yet. + /// + /// + /// 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. + /// + 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); From abb6e60a0053828b5d3652b2911434299c611cab Mon Sep 17 00:00:00 2001 From: Hansjoerg Petriffer Date: Fri, 18 Sep 2026 16:36:41 +0200 Subject: [PATCH 4/6] [Vsintegration] Collect include file orphans once per walk, not once per file UpdateFileContents ran Delete from IncludeFiles where Id not in (select IdInclude from IncludeFilesPerFile) for every file it wrote, inside the lock that serializes the whole code model. That is a full anti join over IncludeFilesPerFile, and a cold walk of RadixWf.sln does it 39387 times. It was redundant as well: DeleteOrphanFiles() already runs the identical statement in bulk when the database is opened. The statement moves to a new DeleteOrphanIncludeFiles(), called once per project walk from ModelWalker after the pass that drops files which no longer exist on disk. Orphan rows are harmless until they are collected: nothing reads them, and UpdateIncludeFiles reuses an existing row when the include comes back. Measured with two cold walks on the same machine and solution, doing practically identical work (39,387 vs 39,380 writes, 157,628 vs 157,568 parses): total time holding the lock 2088s -> 1020s (-51%) p50 per write 58ms -> 28ms p95 105ms -> 52ms p99 173ms -> 68ms worst single write 5164ms -> 394ms (-92%) writes holding the lock > 1s 3 -> 0 orphan sweeps 39387 -> 98 (one per project walk) So the anti join, not AddTypes or WriteLocalFunctions, was what made the critical section expensive. It also explains the three ~5s holds that happened simultaneously in an earlier walk with no backup running: that was this statement, not threads queueing for the lock. This does not change the shape of the problem. There is still one lock around one connection, with ModelWalker running Parallel.ForEach at 3/4 of the cores straight into it, and XDatabase.Read(XFile) is still called 4.8 times per file on average (217,921 calls over 45,492 distinct files in one session, 793 of them for builtinfunctions.prg alone). That one is left alone on purpose: Read also inserts the row when it is missing and refreshes Id, LastChanged and Size, and it has seven call sites, so a naive "skip when Id is set" guard could stop changes on disk from being noticed. Co-Authored-By: Claude Opus 5 --- .../XSharpCodeModelXs/Database/XDatabase.prg | 26 ++++++++++++++++--- .../XSharpCodeModelXs/Parser/ModelWalker.prg | 4 +++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/VisualStudio/XSharpCodeModelXs/Database/XDatabase.prg b/src/VisualStudio/XSharpCodeModelXs/Database/XDatabase.prg index 5a63abaa07..63c004f5a4 100644 --- a/src/VisualStudio/XSharpCodeModelXs/Database/XDatabase.prg +++ b/src/VisualStudio/XSharpCodeModelXs/Database/XDatabase.prg @@ -627,6 +627,26 @@ STATIC METHOD ValidateSchema( Connection AS DbConnection) AS LOGIC Log(i"Validate database schema: {lOk}") RETURN lOk +STATIC METHOD DeleteOrphanIncludeFiles() AS VOID + // Drop IncludeFiles rows that no longer belong to any file. + // This used to run inside UpdateFileContents, once for every file written: a full anti + // join over IncludeFilesPerFile, while holding the lock that serializes the whole code + // model. A cold walk of RadixWf.sln does that 39387 times. Orphan rows are harmless + // until they are cleaned up - nothing reads them, and UpdateIncludeFiles reuses a row + // when the include comes back - so once per project walk is enough. + CHECKIFOPEN + BEGIN LOCK oConn + TRY + Log("Delete orphan include files") + USING VAR cmd := CreateCommand("Delete from IncludeFiles where Id not in (select IdInclude from IncludeFilesPerFile)", oConn) + cmd:ExecuteNonQuery() + CATCH e AS Exception + Log("Error deleting orphaned include files") + XSettings.Exception(e) + END TRY + END LOCK + RETURN + STATIC METHOD DeleteOrphanFiles() AS List VAR result := List{} CHECKIFOPEN result @@ -1387,10 +1407,8 @@ STATIC PRIVATE METHOD UpdateFileContents(oFile AS XFile) AS VOID // Update Includefile IDs and write to disk UpdateIncludeFiles(oFile) endif - // Remove orphans from IncludeFiles table - oCmd:CommandText := "Delete from IncludeFiles where Id not in (select IdInclude from IncludeFilesPerFile)" - oCmd:Parameters:Clear() - oCmd:ExecuteScalar() + // Orphans in the IncludeFiles table are collected once per project walk, see + // DeleteOrphanIncludeFiles(). Doing it here meant a full anti join per file. CATCH e AS Exception Log("File : "+oFile:FullPath+" "+oFile:Id:ToString()) diff --git a/src/VisualStudio/XSharpCodeModelXs/Parser/ModelWalker.prg b/src/VisualStudio/XSharpCodeModelXs/Parser/ModelWalker.prg index 20d5dbea45..1b2b14380c 100644 --- a/src/VisualStudio/XSharpCodeModelXs/Parser/ModelWalker.prg +++ b/src/VisualStudio/XSharpCodeModelXs/Parser/ModelWalker.prg @@ -220,6 +220,10 @@ PRIVATE STATIC METHOD WalkSource() AS VOID XDatabase.DeleteFile(fileName) endif next + // Collect the include file orphans left behind by this walk. This used to run + // once per file inside the database lock, which is far too often for a full + // anti join over the whole table. + XDatabase.DeleteOrphanIncludeFiles() if _currentProject != null _currentProject:ProjectWalkComplete?:Invoke(_currentProject) endif From 989d7782e0aa7ebbe66af05a46178f5372e34cb5 Mon Sep 17 00:00:00 2001 From: Hansjoerg Petriffer Date: Fri, 18 Sep 2026 16:54:50 +0200 Subject: [PATCH 5/6] [Vsintegration] Write each file to the code model database in one transaction There were no transactions anywhere in XDatabase, so every statement ran as its own implicit transaction. AddTypes issues an INSERT per type and AddMembers one per member, which for a file with 159 entities is around 160 of them, and a cold walk of RadixWf.sln writes 39,380 files. All of it happens inside the lock that serializes the whole code model. UpdateFileContents now wraps its work in BEGIN IMMEDIATE / COMMIT, with a ROLLBACK in a FINALLY so a failed file cannot leave the connection sitting in a transaction for the next one. BEGIN and COMMIT are sent as plain SQL rather than through DbTransaction on purpose. A DbTransaction would have to be assigned to every command created further down the call chain - AddTypes, AddMembers, WriteLocalFunctions, WriteCommentTasks, UpdateIncludeFiles - and Microsoft.Data.Sqlite throws when a command misses it. Since the code builds against both System.Data.SQLite and Microsoft.Data.Sqlite, the plain statements are the safer form. Two cold walks doing identical work (39,380 writes, 157,568 vs 157,576 parses, both producing a 169 MB database, no errors in either): total time holding the lock 1020s -> 197s (-81%) p50 per write 28ms -> 3ms p95 52ms -> 15ms p99 68ms -> 42ms worst single write 394ms -> 265ms mean per write 25.9ms -> 5.0ms Together with the two preceding commits that is 2088s -> 197s of lock time for the same walk, and a worst case of 265ms where it used to be 5164ms. Co-Authored-By: Claude Opus 5 --- .../XSharpCodeModelXs/Database/XDatabase.prg | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/VisualStudio/XSharpCodeModelXs/Database/XDatabase.prg b/src/VisualStudio/XSharpCodeModelXs/Database/XDatabase.prg index 63c004f5a4..5487d19367 100644 --- a/src/VisualStudio/XSharpCodeModelXs/Database/XDatabase.prg +++ b/src/VisualStudio/XSharpCodeModelXs/Database/XDatabase.prg @@ -627,6 +627,13 @@ STATIC METHOD ValidateSchema( Connection AS DbConnection) AS LOGIC Log(i"Validate database schema: {lOk}") RETURN lOk +STATIC PRIVATE METHOD ExecuteSimpleSql(cSql AS STRING) AS VOID + // For statements that take no parameters and return nothing, such as BEGIN and COMMIT. + // The caller is expected to hold the lock on oConn. + USING VAR cmd := CreateCommand(cSql, oConn) + cmd:ExecuteNonQuery() + RETURN + STATIC METHOD DeleteOrphanIncludeFiles() AS VOID // Drop IncludeFiles rows that no longer belong to any file. // This used to run inside UpdateFileContents, once for every file written: a full anti @@ -1369,8 +1376,17 @@ STATIC PRIVATE METHOD UpdateFileContents(oFile AS XFile) AS VOID NEXT NEXT Log(i"Start Updating File contents for file {oFile.FullPath} : # of Entities {oFile.EntityList.Count}") + LOCAL lInTransaction := FALSE AS LOGIC BEGIN LOCK oConn TRY + // One transaction for the whole file. Without it every statement below is its + // own implicit transaction, and AddTypes/AddMembers issue one INSERT per type + // and per member: a file with 159 entities costs about 160 of them. + // BEGIN/COMMIT are sent as plain SQL on purpose. Handing out a DbTransaction + // would mean assigning it to every command created further down the call chain, + // and Microsoft.Data.Sqlite throws when a command misses it. + ExecuteSimpleSql("BEGIN IMMEDIATE") + lInTransaction := TRUE // Check to see if file is in multiple projects. // If so then generate a new type for each of the projects @@ -1410,10 +1426,23 @@ STATIC PRIVATE METHOD UpdateFileContents(oFile AS XFile) AS VOID // Orphans in the IncludeFiles table are collected once per project walk, see // DeleteOrphanIncludeFiles(). Doing it here meant a full anti join per file. + ExecuteSimpleSql("COMMIT") + lInTransaction := FALSE + CATCH e AS Exception Log("File : "+oFile:FullPath+" "+oFile:Id:ToString()) XSettings.Exception(e) + FINALLY + IF lInTransaction + // The commit never happened. Undo the half written file and, more + // importantly, leave the connection out of a transaction for the next one. + TRY + ExecuteSimpleSql("ROLLBACK") + CATCH + NOP + END TRY + ENDIF END TRY END LOCK From 4fcf4c153b6cd9909ce6379cfae304dc00a4ec0e Mon Sep 17 00:00:00 2001 From: Hansjoerg Petriffer Date: Fri, 18 Sep 2026 16:55:05 +0200 Subject: [PATCH 6/6] [Vsintegration] Rebuild a project's global usings lazily instead of per saved file XFile.SaveToDatabase() called Project:RefreshGlobalUsings() after every file it wrote, and that method runs XDatabase.GetProjectGlobalUsings() - a query taking the lock that serializes the code model. A cold walk of RadixWf.sln writes 39,380 files, so it ran 39,380 times, for a list that changes rarely. SaveToDatabase now only marks the lists stale, which costs nothing, and they are rebuilt on the first read through the GlobalUsings / GlobalStaticUsings properties. Two related corrections while in there: - RefreshGlobalUsings built its result by clearing the live lists and refilling them. The walker writes from several threads at once, so a reader iterating GlobalUsings could see the list half emptied. It now builds new lists and swaps them in. - The dirty flag starts TRUE. The only thing that ever populated these lists was a file save, so opening a solution against an up to date database, where no file needs writing, left them empty. Now the first reader fills them. Not separately measured: RefreshGlobalUsings ran after XDatabase.Update returned, so its cost falls outside the Start/End Updating File contents window used to measure the previous commit. It contributed to wall clock and to lock contention, but the walk that went from 6.5 to 2.8 minutes contains both changes and the log has no counter for GetProjectGlobalUsings to separate them. Co-Authored-By: Claude Opus 5 --- .../XSharpCodeModelXs/ProjectSystem/XFile.prg | 4 +- .../ProjectSystem/XProject.prg | 45 ++++++++++++++----- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/src/VisualStudio/XSharpCodeModelXs/ProjectSystem/XFile.prg b/src/VisualStudio/XSharpCodeModelXs/ProjectSystem/XFile.prg index 414d5352b9..d6def593b8 100644 --- a/src/VisualStudio/XSharpCodeModelXs/ProjectSystem/XFile.prg +++ b/src/VisualStudio/XSharpCodeModelXs/ProjectSystem/XFile.prg @@ -166,7 +166,9 @@ NAMESPACE XSharpModel IF ! SELF:Virtual XDatabase.Update(SELF) SELF:Project:ClearCache(SELF) - SELF:Project:RefreshGlobalUsings() + // Just mark them stale. This used to rebuild the project's global usings here, + // which is a database query, for every single file written. + SELF:Project:InvalidateGlobalUsings() IF ! SELF:Interactive SELF:Clear() ENDIF diff --git a/src/VisualStudio/XSharpCodeModelXs/ProjectSystem/XProject.prg b/src/VisualStudio/XSharpCodeModelXs/ProjectSystem/XProject.prg index b067b6affb..53a4f0822a 100644 --- a/src/VisualStudio/XSharpCodeModelXs/ProjectSystem/XProject.prg +++ b/src/VisualStudio/XSharpCodeModelXs/ProjectSystem/XProject.prg @@ -1,4 +1,4 @@ -// +// // Copyright (c) XSharp B.V. All Rights Reserved. // Licensed under the Apache License, Version 2.0. // See License.txt in the project root for license information. @@ -46,6 +46,11 @@ CLASS XProject PRIVATE _resolvingReferences AS LOGIC private _globalUsings AS List private _globalStaticUsing AS List + // Set when a file is written to the database, cleared when the lists are rebuilt. + // Rebuilding queries the database, so it must not happen once per saved file. + // Starts TRUE so the first reader fills the lists even when the project is opened + // from an up to date database and no file is ever written. + private _globalUsingsDirty := TRUE AS LOGIC PRIVATE _cachedAllNamespaces AS IList PRIVATE _cachedUsingStatics AS IList @@ -60,8 +65,8 @@ CLASS XProject PROPERTY HasFiles AS LOGIC GET _SourceFilesDict:Keys:Count > 0 .or. _OtherFilesDict:Keys:Count > 0 PROPERTY Framework AS STRING GET _framework PROPERTY DisplayName AS STRING GET _projectNode?.DisplayName - property GlobalUsings AS List GET SELF:_globalUsings - property GlobalStaticUsings AS List GET SELF:_globalStaticUsing + property GlobalUsings AS List GET SELF:EnsureGlobalUsings():_globalUsings + property GlobalStaticUsings AS List GET SELF:EnsureGlobalUsings():_globalStaticUsing PROPERTY DependentAssemblyList AS STRING GET @@ -656,22 +661,40 @@ CLASS XProject #endregion #region 'Normal' Files + // Mark the global usings as out of date. Deliberately cheap: this is called for every + // file that gets written to the database, and rebuilding the lists means a query, so + // the work is deferred until somebody actually reads them. + METHOD InvalidateGlobalUsings() AS VOID + SELF:_globalUsingsDirty := TRUE + + // Rebuild the lists when they are stale. Returns SELF so the properties can chain. + PRIVATE METHOD EnsureGlobalUsings() AS XProject + IF SELF:_globalUsingsDirty + SELF:RefreshGlobalUsings() + ENDIF + RETURN SELF + METHOD RefreshGlobalUsings() AS VOID + SELF:_globalUsingsDirty := FALSE var usings := XDatabase.GetProjectGlobalUsings(SELF:Id) - SELF:_globalUsings:Clear() - SELF:_globalStaticUsing:Clear() + // Build into fresh lists and swap them in, so a reader iterating the old list never + // sees it half emptied. The walker writes from several threads at once. + var newUsings := List{} + var newStatics := List{} foreach var item in usings if item:Attributes:HasFlag(Modifiers.Global) if item:Attributes:HasFlag(Modifiers.Static) - SELF:AddUniqueUsing(_globalStaticUsing, item:Namespace) + SELF:AddUniqueUsing(newStatics, item:Namespace) else - SELF:AddUniqueUsing(_globalUsings, item:Namespace) + SELF:AddUniqueUsing(newUsings, item:Namespace) endif endif next - SELF:AddUniqueUsing(_globalUsings, "System") - SELF:AddUniqueUsing(_globalUsings, "XSharp") + SELF:AddUniqueUsing(newUsings, "System") + SELF:AddUniqueUsing(newUsings, "XSharp") + SELF:_globalUsings := newUsings + SELF:_globalStaticUsing := newStatics METHOD AddUniqueUsing(list as List, name as string) AS VOID var old := list:Find( { x => x:ToUpper() == name:ToUpper()}) if String.IsNullOrEmpty(old) @@ -1577,7 +1600,7 @@ CLASS XProject NEXT result := asmNS ENDIF - FOREACH var ns in SELF:_globalUsings + FOREACH var ns in SELF:GlobalUsings if !result:Contains(ns) result:Add(ns) endif @@ -1601,7 +1624,7 @@ CLASS XProject ENDIF NEXT ENDIF - FOREACH var ns in SELF:_globalStaticUsing + FOREACH var ns in SELF:GlobalStaticUsings if !statics:Contains(ns) statics:Add(ns) endif