Merge release/dev16.11-vs-deps to main-vs-deps

azure-pipelines-official.yml

@@ -215,7 +215,6 @@ stages:
         sourcePath: '$(Build.SourcesDirectory)\artifacts\OptProf\$(BuildConfiguration)\Data'
         toLowerCase: false
         usePat: false
-        retentionDays: 90
       displayName: 'OptProf - Publish to Artifact Services - ProfilingInputs'
       condition: succeeded()
 

src/Compilers/Test/Core/ObjectReference.cs

@@ -161,12 +161,24 @@ public U UseReference<U>(Func<T, U> function)
         /// caller must not "leak" the object out of the given action for any lifetime assertions to be safe.
         /// </summary>
         [MethodImpl(MethodImplOptions.NoInlining)]
-        public ObjectReference<U> GetObjectReference<U>(Func<T, U> function) where U : class
+        public ObjectReference<TResult> GetObjectReference<TResult>(Func<T, TResult> function) where TResult : class
         {
             var newValue = function(GetReferenceWithChecks());
             return ObjectReference.Create(newValue);
         }
 
+        /// <summary>
+        /// Provides the underlying strong reference to the given function, lets a function extract some value, and then returns a new ObjectReference.
+        /// This method is marked not be inlined, to ensure that no temporaries are left on the stack that might still root the strong reference. The
+        /// caller must not "leak" the object out of the given action for any lifetime assertions to be safe.
+        /// </summary>
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        public ObjectReference<TResult> GetObjectReference<TResult, TArg>(Func<T, TArg, TResult> function, TArg argument) where TResult : class
+        {
+            var newValue = function(GetReferenceWithChecks(), argument);
+            return ObjectReference.Create(newValue);
+        }
+
         /// <summary>
         /// Fetches the object strongly being held from this. Because the value returned might be cached in a local temporary from
         /// the caller of this function, no further calls to <see cref="AssertHeld"/> or <see cref="AssertReleased"/> may be called

src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationAndGeneratorDriverTranslationAction.cs

@@ -27,6 +27,15 @@ public virtual Task<Compilation> TransformCompilationAsync(Compilation oldCompil
             /// side effects. This opts those out of operating on ones with generated documents where there would be side effects.
             /// </remarks>
             public abstract bool CanUpdateCompilationWithStaleGeneratedTreesIfGeneratorsGiveSameOutput { get; }
+
+            /// <summary>
+            /// When changes are made to a solution, we make a list of translation actions. If multiple similar changes happen in rapid
+            /// succession, we may be able to merge them without holding onto intermediate state.
+            /// </summary>
+            /// <param name="priorAction">The action prior to this one. May be a different type.</param>
+            /// <returns>A non-null <see cref="CompilationAndGeneratorDriverTranslationAction" /> if we could create a merged one, null otherwise.</returns>
+            public virtual CompilationAndGeneratorDriverTranslationAction? TryMergeWithPrior(CompilationAndGeneratorDriverTranslationAction priorAction)
+                => null;
         }
     }
 }

src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationAndGeneratorDriverTranslationAction_Actions.cs

@@ -35,6 +35,17 @@ public override Task<Compilation> TransformCompilationAsync(Compilation oldCompi
                 // Replacing a single tree doesn't impact the generated trees in a compilation, so we can use this against
                 // compilations that have generated trees.
                 public override bool CanUpdateCompilationWithStaleGeneratedTreesIfGeneratorsGiveSameOutput => true;
+
+                public override CompilationAndGeneratorDriverTranslationAction? TryMergeWithPrior(CompilationAndGeneratorDriverTranslationAction priorAction)
+                {
+                    if (priorAction is TouchDocumentAction priorTouchAction &&
+                        priorTouchAction._newState == this._oldState)
+                    {
+                        return new TouchDocumentAction(priorTouchAction._oldState, this._newState);
+                    }
+
+                    return null;
+                }
             }
 
             internal sealed class TouchAdditionalDocumentAction : CompilationAndGeneratorDriverTranslationAction
@@ -55,6 +66,17 @@ public TouchAdditionalDocumentAction(TextDocumentState oldState, TextDocumentSta
                 // translation (which is a no-op). Since we use a 'false' here to mean that it's not worth keeping
                 // the compilation with stale trees around, answering true is still important.
                 public override bool CanUpdateCompilationWithStaleGeneratedTreesIfGeneratorsGiveSameOutput => true;
+
+                public override CompilationAndGeneratorDriverTranslationAction? TryMergeWithPrior(CompilationAndGeneratorDriverTranslationAction priorAction)
+                {
+                    if (priorAction is TouchAdditionalDocumentAction priorTouchAction &&
+                        priorTouchAction._newState == this._oldState)
+                    {
+                        return new TouchAdditionalDocumentAction(priorTouchAction._oldState, this._newState);
+                    }
+
+                    return null;
+                }
             }
 
             internal sealed class RemoveDocumentsAction : CompilationAndGeneratorDriverTranslationAction

src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.State.cs

@@ -121,7 +121,11 @@ public static ValueSource<Optional<Compilation>> CreateValueSource(
             /// </summary>
             private sealed class InProgressState : State
             {
-                public ImmutableArray<(ProjectState state, CompilationAndGeneratorDriverTranslationAction action)> IntermediateProjects { get; }
+                /// <summary>
+                /// The list of changes that have happened since we last computed a compilation. The oldState corresponds to
+                /// the state of the project prior to the mutation.
+                /// </summary>
+                public ImmutableArray<(ProjectState oldState, CompilationAndGeneratorDriverTranslationAction action)> IntermediateProjects { get; }
 
                 /// <summary>
                 /// The result of taking the original completed compilation that had generated documents and updating them by

src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs

@@ -129,13 +129,33 @@ public ICompilationTracker Fork(
 
                     var intermediateProjects = state is InProgressState inProgressState
                         ? inProgressState.IntermediateProjects
-                        : ImmutableArray.Create<(ProjectState, CompilationAndGeneratorDriverTranslationAction)>();
+                        : ImmutableArray.Create<(ProjectState oldState, CompilationAndGeneratorDriverTranslationAction action)>();
 
-                    var newIntermediateProjects = translate == null
-                         ? intermediateProjects
-                         : intermediateProjects.Add((ProjectState, translate));
+                    if (translate is not null)
+                    {
+                        // We have a translation action; are we able to merge it with the prior one?
+                        var merged = false;
+                        if (intermediateProjects.Any())
+                        {
+                            var (priorState, priorAction) = intermediateProjects.Last();
+                            var mergedTranslation = translate.TryMergeWithPrior(priorAction);
+                            if (mergedTranslation != null)
+                            {
+                                // We can replace the prior action with this new one
+                                intermediateProjects = intermediateProjects.SetItem(intermediateProjects.Length - 1,
+                                    (oldState: priorState, mergedTranslation));
+                                merged = true;
+                            }
+                        }
+
+                        if (!merged)
+                        {
+                            // Just add it to the end
+                            intermediateProjects = intermediateProjects.Add((oldState: this.ProjectState, translate));
+                        }
+                    }
 
-                    var newState = State.Create(newInProgressCompilation, state.GeneratedDocuments, state.FinalCompilationWithGeneratedDocuments?.GetValueOrNull(cancellationToken), newIntermediateProjects);
+                    var newState = State.Create(newInProgressCompilation, state.GeneratedDocuments, state.FinalCompilationWithGeneratedDocuments?.GetValueOrNull(cancellationToken), intermediateProjects);
 
                     return new CompilationTracker(newProject, newState);
                 }
@@ -248,7 +268,7 @@ private void GetPartialCompilationState(
                     return;
                 }
 
-                inProgressProject = inProgressState != null ? inProgressState.IntermediateProjects.First().state : this.ProjectState;
+                inProgressProject = inProgressState != null ? inProgressState.IntermediateProjects.First().oldState : this.ProjectState;
 
                 // if we already have a final compilation we are done.
                 if (compilationWithoutGeneratedDocuments != null && state is FinalState finalState)
@@ -656,7 +676,7 @@ private async Task<CompilationInfo> BuildFinalStateFromInProgressStateAsync(
                             // Also transform the compilation that has generated files; we won't do that though if the transformation either would cause problems with
                             // the generated documents, or if don't have any source generators in the first place.
                             if (intermediateProject.action.CanUpdateCompilationWithStaleGeneratedTreesIfGeneratorsGiveSameOutput &&
-                                intermediateProject.state.SourceGenerators.Any())
+                                intermediateProject.oldState.SourceGenerators.Any())
                             {
                                 compilationWithGenerators = await intermediateProject.action.TransformCompilationAsync(compilationWithGenerators, cancellationToken).ConfigureAwait(false);
                             }

src/Workspaces/Core/Portable/Workspace/Solution/TextDocumentStates.cs

@@ -3,7 +3,6 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
-using System.Collections;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
@@ -12,7 +11,6 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.Collections;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Roslyn.Utilities;
 
@@ -24,7 +22,8 @@ namespace Microsoft.CodeAnalysis
     internal readonly struct TextDocumentStates<TState>
         where TState : TextDocumentState
     {
-        public static readonly TextDocumentStates<TState> Empty = new(ImmutableList<DocumentId>.Empty, ImmutableSortedDictionary<DocumentId, TState>.Empty);
+        public static readonly TextDocumentStates<TState> Empty =
+            new(ImmutableList<DocumentId>.Empty, ImmutableSortedDictionary.Create<DocumentId, TState>(DocumentIdComparer.Instance));
 
         private readonly ImmutableList<DocumentId> _ids;
 
@@ -37,6 +36,8 @@ internal readonly struct TextDocumentStates<TState>
 
         private TextDocumentStates(ImmutableList<DocumentId> ids, ImmutableSortedDictionary<DocumentId, TState> map)
         {
+            Debug.Assert(map.KeyComparer == DocumentIdComparer.Instance);
+
             _ids = ids;
             _map = map;
         }

src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs

@@ -3219,5 +3219,40 @@ public async Task TestUpdatedDocumentTextIsObservablyConstantAsync(bool recovera
             var treeText = newDocTree.GetText();
             Assert.Same(newDocText, treeText);
         }
+
+        [Fact]
+        public async Task ReplacingTextMultipleTimesDoesNotRootIntermediateCopiesIfCompilationNotAskedFor()
+        {
+            // This test replicates the pattern of some operation changing a bunch of files, but the files aren't kept open.
+            // In Visual Studio we do large refactorings by opening files with an invisible editor, making changes, and closing
+            // again. This process means we'll queue up intermediate changes to those files, but we don't want to hold onto
+            // the intermediate edits when we don't really need to since the final version will be all that matters.
+
+            using var workspace = CreateWorkspaceWithProjectAndDocuments();
+
+            var solution = workspace.CurrentSolution;
+            var documentId = solution.Projects.Single().DocumentIds.Single();
+
+            // Fetch the compilation, so further edits are going to be incremental updates of this one
+            var originalCompilation = await solution.Projects.Single().GetCompilationAsync();
+
+            // Create a source text we'll release and ensure it disappears. We'll also make sure we don't accidentally root
+            // that solution in the middle.
+            var sourceTextToRelease = ObjectReference.CreateFromFactory(static () => SourceText.From(Guid.NewGuid().ToString()));
+            var solutionWithSourceTextToRelease = sourceTextToRelease.GetObjectReference(
+                static (sourceText, document) => document.Project.Solution.WithDocumentText(document.Id, sourceText, PreservationMode.PreserveIdentity),
+                solution.GetDocument(documentId));
+
+            // Change it again, this time by editing the text loader; this replicates us closing a file, and we don't want to pin the changes from the
+            // prior change.
+            var finalSolution = solutionWithSourceTextToRelease.GetObjectReference(
+                static (s, documentId) => s.WithDocumentTextLoader(documentId, new TestTextLoader(Guid.NewGuid().ToString()), PreservationMode.PreserveValue), documentId).GetReference();
+
+            // The text in the middle shouldn't be held at all, since we replaced it.
+            solutionWithSourceTextToRelease.ReleaseStrongReference();
+            sourceTextToRelease.AssertReleased();
+
+            GC.KeepAlive(finalSolution);
+        }
     }
 }