Merge pull request #13056 from heejaechang/reduceallocation

reduced a bit more allocations

src/EditorFeatures/Test/Diagnostics/DiagnosticDataSerializerTests.cs

@@ -63,7 +63,7 @@ public async Task SerializationTest_Document()
                 Assert.True(await serializer.SerializeAsync(document, key, diagnostics, CancellationToken.None).ConfigureAwait(false));
                 var recovered = await serializer.DeserializeAsync(document, key, CancellationToken.None);
 
-                AssertDiagnostics(diagnostics, recovered);
+                AssertDiagnostics(diagnostics, recovered.Value);
             }
         }
 
@@ -103,7 +103,7 @@ public async Task SerializationTest_Project()
                 Assert.True(await serializer.SerializeAsync(document, key, diagnostics, CancellationToken.None).ConfigureAwait(false));
                 var recovered = await serializer.DeserializeAsync(document, key, CancellationToken.None);
 
-                AssertDiagnostics(diagnostics, recovered);
+                AssertDiagnostics(diagnostics, recovered.Value);
             }
         }
 

src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.ProjectState.cs

@@ -2,6 +2,7 @@
 
 using System;
 using System.Collections.Immutable;
+using System.Runtime.CompilerServices;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Shared.Options;
@@ -17,6 +18,9 @@ internal partial class DiagnosticIncrementalAnalyzer
         /// </summary>
         private class ProjectState
         {
+            private static readonly Task<StrongBox<ImmutableArray<DiagnosticData>>> s_emptyResultTaskCache =
+                Task.FromResult(new StrongBox<ImmutableArray<DiagnosticData>>(ImmutableArray<DiagnosticData>.Empty));
+
             // project id of this state
             private readonly StateSet _owner;
 
@@ -369,26 +373,44 @@ private async Task<bool> TryDeserializeDocumentAsync(DiagnosticDataSerializer se
                 CancellationToken cancellationToken) where T : class
             {
                 var diagnostics = await DeserializeAsync(serializer, documentOrProject, key, stateKey, cancellationToken).ConfigureAwait(false);
-                if (diagnostics.IsDefault)
+                if (diagnostics == null)
                 {
                     return false;
                 }
 
-                add(key, diagnostics);
+                add(key, diagnostics.Value);
                 return true;
             }
 
-            private async Task<ImmutableArray<DiagnosticData>> DeserializeAsync(DiagnosticDataSerializer serializer, object documentOrProject, object key, string stateKey, CancellationToken cancellationToken)
+            private Task<StrongBox<ImmutableArray<DiagnosticData>>> DeserializeAsync(DiagnosticDataSerializer serializer, object documentOrProject, object key, string stateKey, CancellationToken cancellationToken)
             {
+                // when VS is loading new solution, we will try to find out all diagnostics persisted from previous VS session.
+                // in that situation, it is possible that we have a lot of deserialization returning empty result. previously we used to
+                // return default(ImmutableArray) for such case, but it turns out async/await framework has allocation issues with returning
+                // default(value type), so we are using StrongBox to return no data as null. async/await has optimization where it will return
+                // cached empty task if given value is null for reference type. (see AsyncMethodBuilder.GetTaskForResult)
+                // 
+                // right now, we can't use Nullable either, since it is not one of value type the async/await will reuse cached task. in future,
+                // if they do, we can change it to return Nullable<ImmutableArray>
+                //
+                // after initial deserialization, we track actual document/project that actually have diagnostics so no data won't be a common
+                // case.
+
                 // check cache first
                 CacheEntry entry;
                 if (InMemoryStorage.TryGetValue(_owner.Analyzer, ValueTuple.Create(key, stateKey), out entry) && serializer.Version == entry.Version)
                 {
-                    return entry.Diagnostics;
+                    if (entry.Diagnostics.Length == 0)
+                    {
+                        // if there is no result, use cached task
+                        return s_emptyResultTaskCache;
+                    }
+
+                    return Task.FromResult(new StrongBox<ImmutableArray<DiagnosticData>>(entry.Diagnostics));
                 }
 
                 // try to deserialize it
-                return await serializer.DeserializeAsync(documentOrProject, stateKey, cancellationToken).ConfigureAwait(false);
+                return serializer.DeserializeAsync(documentOrProject, stateKey, cancellationToken);
             }
 
             private void RemoveInMemoryCache(DiagnosticAnalysisResult lastResult)

src/VisualStudio/Core/Def/Implementation/Workspace/Esent/EsentPersistentStorage.cs

@@ -21,6 +21,12 @@ internal partial class EsentPersistentStorage : AbstractPersistentStorage
         private const string StorageExtension = "vbcs.cache";
         private const string PersistentStorageFileName = "storage.ide";
 
+        // cache delegates so that we don't re-create it every times
+        private readonly Func<int, object, object, object, CancellationToken, Stream> _readStreamSolution;
+        private readonly Func<EsentStorage.Key, int, object, object, CancellationToken, Stream> _readStream;
+        private readonly Func<int, Stream, object, object, CancellationToken, bool> _writeStreamSolution;
+        private readonly Func<EsentStorage.Key, int, Stream, object, CancellationToken, bool> _writeStream;
+
         private readonly ConcurrentDictionary<string, int> _nameTableCache;
         private readonly EsentStorage _esentStorage;
 
@@ -28,6 +34,12 @@ internal partial class EsentPersistentStorage : AbstractPersistentStorage
             IOptionService optionService, string workingFolderPath, string solutionFilePath, Action<AbstractPersistentStorage> disposer) :
             base(optionService, workingFolderPath, solutionFilePath, disposer)
         {
+            // cache delegates
+            _readStreamSolution = ReadStreamSolution;
+            _readStream = ReadStream;
+            _writeStreamSolution = WriteStreamSolution;
+            _writeStream = WriteStream;
+
             // solution must exist in disk. otherwise, we shouldn't be here at all.
             Contract.ThrowIfTrue(string.IsNullOrWhiteSpace(solutionFilePath));
 
@@ -77,7 +89,7 @@ public override Task<Stream> ReadStreamAsync(Document document, string name, Can
                 return SpecializedTasks.Default<Stream>();
             }
 
-            var stream = EsentExceptionWrapper(key, nameId, ReadStream, cancellationToken);
+            var stream = EsentExceptionWrapper(key, nameId, _readStream, cancellationToken);
             return SpecializedTasks.DefaultOrResult(stream);
         }
 
@@ -98,7 +110,7 @@ public override Task<Stream> ReadStreamAsync(Project project, string name, Cance
                 return SpecializedTasks.Default<Stream>();
             }
 
-            var stream = EsentExceptionWrapper(key, nameId, ReadStream, cancellationToken);
+            var stream = EsentExceptionWrapper(key, nameId, _readStream, cancellationToken);
             return SpecializedTasks.DefaultOrResult(stream);
         }
 
@@ -117,7 +129,7 @@ public override Task<Stream> ReadStreamAsync(string name, CancellationToken canc
                 return SpecializedTasks.Default<Stream>();
             }
 
-            var stream = EsentExceptionWrapper(nameId, ReadStream, cancellationToken);
+            var stream = EsentExceptionWrapper(nameId, _readStreamSolution, cancellationToken);
             return SpecializedTasks.DefaultOrResult(stream);
         }
 
@@ -136,7 +148,7 @@ private Stream ReadStream(EsentStorage.Key key, int nameId, object unused1, obje
             }
         }
 
-        private Stream ReadStream(int nameId, object unused1, object unused2, object unused3, CancellationToken cancellationToken)
+        private Stream ReadStreamSolution(int nameId, object unused1, object unused2, object unused3, CancellationToken cancellationToken)
         {
             using (var accessor = _esentStorage.GetSolutionTableAccessor())
             using (var esentStream = accessor.GetReadStream(nameId))
@@ -169,7 +181,7 @@ public override Task<bool> WriteStreamAsync(Document document, string name, Stre
                 return SpecializedTasks.False;
             }
 
-            var success = EsentExceptionWrapper(key, nameId, stream, WriteStream, cancellationToken);
+            var success = EsentExceptionWrapper(key, nameId, stream, _writeStream, cancellationToken);
             return success ? SpecializedTasks.True : SpecializedTasks.False;
         }
 
@@ -191,7 +203,7 @@ public override Task<bool> WriteStreamAsync(Project project, string name, Stream
                 return SpecializedTasks.False;
             }
 
-            var success = EsentExceptionWrapper(key, nameId, stream, WriteStream, cancellationToken);
+            var success = EsentExceptionWrapper(key, nameId, stream, _writeStream, cancellationToken);
             return success ? SpecializedTasks.True : SpecializedTasks.False;
         }
 
@@ -211,7 +223,7 @@ public override Task<bool> WriteStreamAsync(string name, Stream stream, Cancella
                 return SpecializedTasks.False;
             }
 
-            var success = EsentExceptionWrapper(nameId, stream, WriteStream, cancellationToken);
+            var success = EsentExceptionWrapper(nameId, stream, _writeStreamSolution, cancellationToken);
 
             return success ? SpecializedTasks.True : SpecializedTasks.False;
         }
@@ -226,7 +238,7 @@ private bool WriteStream(EsentStorage.Key key, int nameId, Stream stream, object
             }
         }
 
-        private bool WriteStream(int nameId, Stream stream, object unused1, object unused2, CancellationToken cancellationToken)
+        private bool WriteStreamSolution(int nameId, Stream stream, object unused1, object unused2, CancellationToken cancellationToken)
         {
             using (var accessor = _esentStorage.GetSolutionTableAccessor())
             using (var esentStream = accessor.GetWriteStream(nameId))

src/Workspaces/Core/Portable/Diagnostics/DiagnosticDataSerializer.cs

@@ -6,6 +6,7 @@
 using System.Collections.ObjectModel;
 using System.IO;
 using System.Linq;
+using System.Runtime.CompilerServices;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Diagnostics;
@@ -53,7 +54,7 @@ public async Task<bool> SerializeAsync(object documentOrProject, string key, Imm
             }
         }
 
-        public async Task<ImmutableArray<DiagnosticData>> DeserializeAsync(object documentOrProject, string key, CancellationToken cancellationToken)
+        public async Task<StrongBox<ImmutableArray<DiagnosticData>>> DeserializeAsync(object documentOrProject, string key, CancellationToken cancellationToken)
         {
             // we have persisted data
             var solution = GetSolution(documentOrProject);
@@ -64,11 +65,13 @@ public async Task<ImmutableArray<DiagnosticData>> DeserializeAsync(object docume
             {
                 if (stream == null)
                 {
-                    return default(ImmutableArray<DiagnosticData>);
+                    return null;
                 }
 
                 using (var reader = new ObjectReader(stream))
                 {
+                    // we return StrongBox rather than ImmutableArray due to task lib's issue with allocations
+                    // when returning default(value type)
                     return ReadFrom(reader, documentOrProject, cancellationToken);
                 }
             }
@@ -205,7 +208,7 @@ private Task<Stream> ReadStreamAsync(IPersistentStorage storage, string key, obj
             return storage.ReadStreamAsync(project, key, cancellationToken);
         }
 
-        public ImmutableArray<DiagnosticData> ReadFrom(ObjectReader reader, object documentOrProject, CancellationToken cancellationToken)
+        public StrongBox<ImmutableArray<DiagnosticData>> ReadFrom(ObjectReader reader, object documentOrProject, CancellationToken cancellationToken)
         {
             var document = documentOrProject as Document;
             if (document != null)
@@ -217,7 +220,7 @@ public ImmutableArray<DiagnosticData> ReadFrom(ObjectReader reader, object docum
             return ReadFrom(reader, project, null, cancellationToken);
         }
 
-        private ImmutableArray<DiagnosticData> ReadFrom(ObjectReader reader, Project project, Document document, CancellationToken cancellationToken)
+        private StrongBox<ImmutableArray<DiagnosticData>> ReadFrom(ObjectReader reader, Project project, Document document, CancellationToken cancellationToken)
         {
             try
             {
@@ -228,29 +231,30 @@ private ImmutableArray<DiagnosticData> ReadFrom(ObjectReader reader, Project pro
                     var format = reader.ReadInt32();
                     if (format != FormatVersion)
                     {
-                        return default(ImmutableArray<DiagnosticData>);
+                        return null;
                     }
 
                     // saved data is for same analyzer of different version of dll
                     var analyzerVersion = VersionStamp.ReadFrom(reader);
                     if (analyzerVersion != AnalyzerVersion)
                     {
-                        return default(ImmutableArray<DiagnosticData>);
+                        return null;
                     }
 
                     var version = VersionStamp.ReadFrom(reader);
                     if (version != VersionStamp.Default && version != Version)
                     {
-                        return default(ImmutableArray<DiagnosticData>);
+                        return null;
                     }
 
                     ReadFrom(reader, project, document, list, cancellationToken);
-                    return list.ToImmutableArray();
+
+                    return new StrongBox<ImmutableArray<DiagnosticData>>(list.ToImmutableArray());
                 }
             }
             catch (Exception)
             {
-                return default(ImmutableArray<DiagnosticData>);
+                return null;
             }
         }
 

src/Workspaces/Remote/ServiceHub/Diagnostics/DiagnosticResultSerializer.cs

@@ -3,6 +3,7 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Runtime.CompilerServices;
 using System.Threading;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Diagnostics.Telemetry;
@@ -71,7 +72,7 @@ public static void Serialize(ObjectWriter writer, DiagnosticAnalysisResultMap<st
 
                 var analysisResult = new DiagnosticAnalysisResult(
                     project.Id, version,
-                    syntaxLocalMap, semanticLocalMap, nonLocalMap, others,
+                    syntaxLocalMap, semanticLocalMap, nonLocalMap, GetOrDefault(others),
                     documentIds: null, fromBuild: false);
 
                 analysisMap.Add(analyzer, analysisResult);
@@ -96,7 +97,7 @@ public static void Serialize(ObjectWriter writer, DiagnosticAnalysisResultMap<st
                 var analyzer = analyzerMap[reader.ReadString()];
                 var exceptions = diagnosticDataSerializer.ReadFrom(reader, project, cancellationToken);
 
-                exceptionMap.Add(analyzer, exceptions);
+                exceptionMap.Add(analyzer, GetOrDefault(exceptions));
             }
 
             return DiagnosticAnalysisResultMap.Create(analysisMap.ToImmutable(), telemetryMap.ToImmutable(), exceptionMap.ToImmutable());
@@ -129,7 +130,7 @@ public static void Serialize(ObjectWriter writer, DiagnosticAnalysisResultMap<st
                 var documentId = Serializer.DeserializeDocumentId(reader, cancellationToken);
                 var diagnostics = serializer.ReadFrom(reader, project.GetDocument(documentId), cancellationToken);
 
-                map.Add(documentId, diagnostics);
+                map.Add(documentId, GetOrDefault(diagnostics));
             }
 
             return map.ToImmutable();
@@ -199,5 +200,10 @@ private static AnalyzerTelemetryInfo Deserialize(ObjectReader reader, Cancellati
                 ExecutionTime = executionTime
             };
         }
+
+        private static ImmutableArray<T> GetOrDefault<T>(StrongBox<ImmutableArray<T>> items)
+        {
+            return items?.Value ?? default(ImmutableArray<T>);
+        }
     }
 }