From 6498d38c344cedb25af66ef922bd12a4517a7c18 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Fri, 13 Mar 2026 06:42:43 -0700 Subject: [PATCH 1/2] Reduce allocations in normal elfie usage In the binary, non-patch case (aka, the "normal" case), this should prevent a LOH byte[] allocation reading a roughly 15 MB binary file. Instead, use a stream so that the whole file need not be read into memory at once. Will add more details if a speedometer run comes back looking good. --- .../Windows/IDatabaseFactoryService.cs | 3 +- .../SymbolSearch/Windows/IIOService.cs | 1 + ...archUpdateEngine.DatabaseFactoryService.cs | 7 ++-- .../SymbolSearchUpdateEngine.IOService.cs | 2 + .../SymbolSearchUpdateEngine.Update.cs | 41 +++++++++++-------- .../SymbolSearchUpdateEngineTests.vb | 14 +++---- 6 files changed, 38 insertions(+), 30 deletions(-) diff --git a/src/Features/Core/Portable/SymbolSearch/Windows/IDatabaseFactoryService.cs b/src/Features/Core/Portable/SymbolSearch/Windows/IDatabaseFactoryService.cs index 6ab4a9fe280f9..3568445da0cc4 100644 --- a/src/Features/Core/Portable/SymbolSearch/Windows/IDatabaseFactoryService.cs +++ b/src/Features/Core/Portable/SymbolSearch/Windows/IDatabaseFactoryService.cs @@ -2,11 +2,12 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.IO; using Microsoft.CodeAnalysis.Elfie.Model; namespace Microsoft.CodeAnalysis.SymbolSearch; internal interface IDatabaseFactoryService { - AddReferenceDatabase CreateDatabaseFromBytes(byte[] bytes, bool isBinary); + AddReferenceDatabase CreateDatabaseFromStream(Stream stream, bool isBinary); } diff --git a/src/Features/Core/Portable/SymbolSearch/Windows/IIOService.cs b/src/Features/Core/Portable/SymbolSearch/Windows/IIOService.cs index ae1b103c5262d..4f642cfbc6705 100644 --- a/src/Features/Core/Portable/SymbolSearch/Windows/IIOService.cs +++ b/src/Features/Core/Portable/SymbolSearch/Windows/IIOService.cs @@ -15,6 +15,7 @@ internal interface IIOService void Create(DirectoryInfo directory); void Delete(FileInfo file); bool Exists(FileSystemInfo info); + Stream OpenRead(string path); byte[] ReadAllBytes(string path); void Replace(string sourceFileName, string destinationFileName, string? destinationBackupFileName, bool ignoreMetadataErrors); void Move(string sourceFileName, string destinationFileName); diff --git a/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.DatabaseFactoryService.cs b/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.DatabaseFactoryService.cs index a603461b1ca9b..05b6c7b20b1ab 100644 --- a/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.DatabaseFactoryService.cs +++ b/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.DatabaseFactoryService.cs @@ -11,19 +11,18 @@ internal sealed partial class SymbolSearchUpdateEngine { private sealed class DatabaseFactoryService : IDatabaseFactoryService { - public AddReferenceDatabase CreateDatabaseFromBytes(byte[] bytes, bool isBinary) + public AddReferenceDatabase CreateDatabaseFromStream(Stream stream, bool isBinary) { - using var memoryStream = new MemoryStream(bytes); var database = new AddReferenceDatabase(ArdbVersion.V1); if (isBinary) { - using var binaryReader = new BinaryReader(memoryStream); + using var binaryReader = new BinaryReader(stream); database.ReadBinary(binaryReader); } else { - using var streamReader = new StreamReader(memoryStream); + using var streamReader = new StreamReader(stream); database.ReadText(streamReader); } diff --git a/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.IOService.cs b/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.IOService.cs index 36b2b653ad2d4..a82d7dcd1f296 100644 --- a/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.IOService.cs +++ b/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.IOService.cs @@ -17,6 +17,8 @@ private sealed class IOService : IIOService public bool Exists(FileSystemInfo info) => info.Exists; + public Stream OpenRead(string path) => File.OpenRead(path); + public byte[] ReadAllBytes(string path) => File.ReadAllBytes(path); public void Replace(string sourceFileName, string destinationFileName, string? destinationBackupFileName, bool ignoreMetadataErrors) diff --git a/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.Update.cs b/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.Update.cs index 3cdbe883397d3..f3cfcb73a4126 100644 --- a/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.Update.cs +++ b/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.Update.cs @@ -273,7 +273,8 @@ private async Task DownloadFullDatabaseAsync(FileInfo databaseFileInfo // searching. try { - database = CreateAndSetInMemoryDatabase(bytes, isBinary: false); + using var stream = new MemoryStream(bytes); + database = CreateAndSetInMemoryDatabase(stream, isBinary: false); } catch (Exception e) when (_service._reportAndSwallowExceptionUnlessCanceled(e, cancellationToken)) { @@ -391,18 +392,18 @@ private async Task PatchLocalDatabaseAsync(FileInfo databaseFileInfo, LogInfo("Reading in local database"); - var (databaseBytes, isBinary) = GetDatabaseBytes(databaseFileInfo); + using var stream = GetDatabaseStream(databaseFileInfo, out var isBinary); - LogInfo($"Reading in local database completed. databaseBytes.Length={databaseBytes.Length}"); + LogInfo($"Reading in local database completed. isBinary={isBinary}"); - // Make a database instance out of those bytes and set is as the current in memory database - // that searches will run against. If we can't make a database instance from these bytes + // Make a database instance from the stream and set it as the current in memory database + // that searches will run against. If we can't make a database instance from the stream // then our local database is corrupt and we need to download the full database to get back // into a good state. AddReferenceDatabase database; try { - database = CreateAndSetInMemoryDatabase(databaseBytes, isBinary); + database = CreateAndSetInMemoryDatabase(stream, isBinary); } catch (Exception e) when (_service._reportAndSwallowExceptionUnlessCanceled(e, cancellationToken)) { @@ -424,7 +425,8 @@ private async Task PatchLocalDatabaseAsync(FileInfo databaseFileInfo, databaseFileInfo, element, // We pass a delegate to get the database bytes so that we can avoid reading the bytes when we don't need them due to no patch to apply. - getDatabaseBytes: () => isBinary ? _service._ioService.ReadAllBytes(databaseFileInfo.FullName) : databaseBytes, + // Note: ApplyPatch requires byte[], so we must read into memory here. + getDatabaseBytes: () => _service._ioService.ReadAllBytes(isBinary ? GetBinaryFileInfo(databaseFileInfo).FullName : databaseFileInfo.FullName), cancellationToken).ConfigureAwait(false); LogInfo("Downloading and processing patch file completed"); @@ -432,33 +434,35 @@ private async Task PatchLocalDatabaseAsync(FileInfo databaseFileInfo, return delayUntilUpdate; - (byte[] dataBytes, bool isBinary) GetDatabaseBytes(FileInfo databaseFileInfo) + Stream GetDatabaseStream(FileInfo databaseFileInfo, out bool isBinary) { var databaseBinaryFileInfo = GetBinaryFileInfo(databaseFileInfo); try { // First attempt to read from the binary file. If that fails, fall back to the text file. - return (_service._ioService.ReadAllBytes(databaseBinaryFileInfo.FullName), isBinary: true); + isBinary = true; + return _service._ioService.OpenRead(databaseBinaryFileInfo.FullName); } catch (Exception e) when (IOUtilities.IsNormalIOException(e)) { } // (intentionally not wrapped in IOUtilities. If this throws we want to restart). - return (_service._ioService.ReadAllBytes(databaseFileInfo.FullName), isBinary: false); + isBinary = false; + return _service._ioService.OpenRead(databaseFileInfo.FullName); } } /// - /// Creates a database instance with the bytes passed in. If creating the database succeeds, + /// Creates a database instance with the stream passed in. If creating the database succeeds, /// then it will be set as the current in memory version. In the case of failure (which /// indicates that our data is corrupt), the exception will bubble up and must be appropriately /// dealt with by the caller. /// - private AddReferenceDatabase CreateAndSetInMemoryDatabase(byte[] bytes, bool isBinary) + private AddReferenceDatabase CreateAndSetInMemoryDatabase(Stream stream, bool isBinary) { - var database = CreateDatabaseFromBytes(bytes, isBinary); + var database = CreateDatabaseFromStream(stream, isBinary); _service._sourceToDatabase[_source] = new AddReferenceDatabaseWrapper(database); return database; } @@ -520,7 +524,8 @@ private async Task ProcessPatchXElementAsync( LogInfo($"Applying patch completed. finalBytes.Length={finalBytes.Length}"); // finalBytes is generated from the current database and the patch, not from the binary file. - database = CreateAndSetInMemoryDatabase(finalBytes, isBinary: false); + using var stream = new MemoryStream(finalBytes); + database = CreateAndSetInMemoryDatabase(stream, isBinary: false); // Attempt to persist the txt and binary forms of the index. It's ok if either of these writes // don't succeed. If the txt file fails to persist, a subsequent VS session will redownload the @@ -560,11 +565,11 @@ private static void ParsePatchElement(XElement patchElement, out bool upToDate, } } - private AddReferenceDatabase CreateDatabaseFromBytes(byte[] bytes, bool isBinary) + private AddReferenceDatabase CreateDatabaseFromStream(Stream stream, bool isBinary) { - LogInfo("Creating database from bytes"); - var result = _service._databaseFactoryService.CreateDatabaseFromBytes(bytes, isBinary); - LogInfo("Creating database from bytes completed"); + LogInfo("Creating database from stream"); + var result = _service._databaseFactoryService.CreateDatabaseFromStream(stream, isBinary); + LogInfo("Creating database from stream completed"); return result; } diff --git a/src/VisualStudio/Core/Test/SymbolSearch/SymbolSearchUpdateEngineTests.vb b/src/VisualStudio/Core/Test/SymbolSearch/SymbolSearchUpdateEngineTests.vb index 55751b94d400b..7097068db8f5c 100644 --- a/src/VisualStudio/Core/Test/SymbolSearch/SymbolSearchUpdateEngineTests.vb +++ b/src/VisualStudio/Core/Test/SymbolSearch/SymbolSearchUpdateEngineTests.vb @@ -255,7 +255,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.SymbolSearch Dim factoryMock = New Mock(Of IDatabaseFactoryService)(MockBehavior.Strict) ' Simulate Elfie throwing when trying to make a database from the contents of that response - factoryMock.Setup(Function(f) f.CreateDatabaseFromBytes(It.IsAny(Of Byte()), It.IsAny(Of Boolean))). + factoryMock.Setup(Function(f) f.CreateDatabaseFromStream(It.IsAny(Of Stream), It.IsAny(Of Boolean))). Throws(New NotImplementedException()) ' Because the parsing failed we will expect to call into the 'UpdateFailedDelay' to @@ -300,7 +300,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.SymbolSearch ' Successfully create a database from that response. Dim factoryMock = New Mock(Of IDatabaseFactoryService)(MockBehavior.Strict) - factoryMock.Setup(Function(f) f.CreateDatabaseFromBytes(It.IsAny(Of Byte()), It.IsAny(Of Boolean))). + factoryMock.Setup(Function(f) f.CreateDatabaseFromStream(It.IsAny(Of Stream), It.IsAny(Of Boolean))). Returns(New AddReferenceDatabase()) ' Expect that we'll write the database to disk successfully. @@ -349,7 +349,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.SymbolSearch ' Create a database from the client response. Dim factoryMock = New Mock(Of IDatabaseFactoryService)(MockBehavior.Strict) - factoryMock.Setup(Function(f) f.CreateDatabaseFromBytes(It.IsAny(Of Byte()), It.IsAny(Of Boolean))). + factoryMock.Setup(Function(f) f.CreateDatabaseFromStream(It.IsAny(Of Stream), It.IsAny(Of Boolean))). Returns(New AddReferenceDatabase()) Dim delayMock = New Mock(Of IDelayService)(MockBehavior.Strict) @@ -402,7 +402,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.SymbolSearch ' We'll successfully read in the local database. Dim databaseFactoryMock = New Mock(Of IDatabaseFactoryService)(MockBehavior.Strict) - databaseFactoryMock.Setup(Function(f) f.CreateDatabaseFromBytes(It.IsAny(Of Byte()), It.IsAny(Of Boolean))). + databaseFactoryMock.Setup(Function(f) f.CreateDatabaseFromStream(It.IsAny(Of Stream), It.IsAny(Of Boolean))). Returns(New AddReferenceDatabase()) ' Create a client that will return a patch that says things are up to date. @@ -447,7 +447,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.SymbolSearch ' We'll successfully read in the local database. Dim databaseFactoryMock = New Mock(Of IDatabaseFactoryService)(MockBehavior.Strict) - databaseFactoryMock.Setup(Function(f) f.CreateDatabaseFromBytes(It.IsAny(Of Byte()), It.IsAny(Of Boolean))). + databaseFactoryMock.Setup(Function(f) f.CreateDatabaseFromStream(It.IsAny(Of Stream), It.IsAny(Of Boolean))). Returns(New AddReferenceDatabase()) ' Create a client that will return a patch that says things are too old. @@ -501,7 +501,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.SymbolSearch ' We'll successfully read in the local database. Dim databaseFactoryMock = New Mock(Of IDatabaseFactoryService)(MockBehavior.Strict) - databaseFactoryMock.Setup(Function(f) f.CreateDatabaseFromBytes(It.IsAny(Of Byte()), It.IsAny(Of Boolean))). + databaseFactoryMock.Setup(Function(f) f.CreateDatabaseFromStream(It.IsAny(Of Stream), It.IsAny(Of Boolean))). Returns(New AddReferenceDatabase()) ' Create a client that will return a patch with contents. @@ -561,7 +561,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.SymbolSearch ' We'll successfully read in the local database. Dim databaseFactoryMock = New Mock(Of IDatabaseFactoryService)(MockBehavior.Strict) - databaseFactoryMock.Setup(Function(f) f.CreateDatabaseFromBytes(It.IsAny(Of Byte()), It.IsAny(Of Boolean))). + databaseFactoryMock.Setup(Function(f) f.CreateDatabaseFromStream(It.IsAny(Of Stream), It.IsAny(Of Boolean))). Returns(New AddReferenceDatabase()) ' Create a client that will return a patch with contents. From c1bbcc6ad0a4bae66cd7547d186683c9ff9b9ea7 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Fri, 13 Mar 2026 21:25:01 -0700 Subject: [PATCH 2/2] Missed some mock methods needed in unit tests --- .../Core/Test/SymbolSearch/SymbolSearchUpdateEngineTests.vb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/VisualStudio/Core/Test/SymbolSearch/SymbolSearchUpdateEngineTests.vb b/src/VisualStudio/Core/Test/SymbolSearch/SymbolSearchUpdateEngineTests.vb index 7097068db8f5c..149ba450b6420 100644 --- a/src/VisualStudio/Core/Test/SymbolSearch/SymbolSearchUpdateEngineTests.vb +++ b/src/VisualStudio/Core/Test/SymbolSearch/SymbolSearchUpdateEngineTests.vb @@ -399,6 +399,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.SymbolSearch ' Simulate the database being there. ioMock.Setup(Function(s) s.Exists(It.IsAny(Of FileSystemInfo))).Returns(True) ioMock.Setup(Function(s) s.ReadAllBytes(It.IsAny(Of String))).Returns({}) + ioMock.Setup(Function(s) s.OpenRead(It.IsAny(Of String))).Returns(New MemoryStream()) ' We'll successfully read in the local database. Dim databaseFactoryMock = New Mock(Of IDatabaseFactoryService)(MockBehavior.Strict) @@ -443,6 +444,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.SymbolSearch ' Simulate the database being there. ioMock.Setup(Function(s) s.Exists(It.IsAny(Of FileSystemInfo))).Returns(True) ioMock.Setup(Function(s) s.ReadAllBytes(It.IsAny(Of String))).Returns({}) + ioMock.Setup(Function(s) s.OpenRead(It.IsAny(Of String))).Returns(New MemoryStream()) ioMock.Setup(Sub(s) s.Delete(It.IsAny(Of FileInfo))) ' We'll successfully read in the local database. @@ -497,6 +499,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.SymbolSearch ' Simulate the database being there. ioMock.Setup(Function(s) s.Exists(It.IsAny(Of FileSystemInfo))).Returns(True) ioMock.Setup(Function(s) s.ReadAllBytes(It.IsAny(Of String))).Returns({}) + ioMock.Setup(Function(s) s.OpenRead(It.IsAny(Of String))).Returns(New MemoryStream()) ioMock.Setup(Sub(s) s.Delete(It.IsAny(Of FileInfo))) ' We'll successfully read in the local database. @@ -557,6 +560,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.SymbolSearch ' Simulate the database being there. ioMock.Setup(Function(s) s.Exists(It.IsAny(Of FileSystemInfo))).Returns(True) ioMock.Setup(Function(s) s.ReadAllBytes(It.IsAny(Of String))).Returns({}) + ioMock.Setup(Function(s) s.OpenRead(It.IsAny(Of String))).Returns(New MemoryStream()) ioMock.Setup(Sub(s) s.Delete(It.IsAny(Of FileInfo))) ' We'll successfully read in the local database.