Reduce allocations in normal elfie usage

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);
 }

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);

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);
             }
 

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)

src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.Update.cs

@@ -273,7 +273,8 @@ private async Task<TimeSpan> 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<TimeSpan> 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,41 +425,44 @@ private async Task<TimeSpan> 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");
             LogInfo("Patching local database completed");
 
             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);
             }
         }
 
         /// <summary>
-        /// 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.
         /// </summary>
-        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<TimeSpan> 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;
         }
 

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)
@@ -399,10 +399,11 @@ 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)
-                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.
@@ -443,11 +444,12 @@ 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.
                 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.
@@ -497,11 +499,12 @@ 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.
                 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.
@@ -557,11 +560,12 @@ 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.
                 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.