Merge pull request #60753 from CyrusNajmabadi/revertSolutionRefCount

Do not try to refcount solution syncing when communicating with OOP
dotnet · Apr 14, 2022 · 7711d35 · 7711d35
2 parents 646e3c4 + 1dfba6a
commit 7711d35
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 0 deletions.
diff --git a/src/Workspaces/Remote/Core/SolutionAssetStorage.cs b/src/Workspaces/Remote/Core/SolutionAssetStorage.cs
@@ -104,6 +104,9 @@ public async ValueTask<IReadOnlyDictionary<Checksum, SolutionAsset>> GetAssetsAs
                 result[Checksum.Null] = SolutionAsset.Null;
             }
 
+            if (!_solutionStates.ContainsKey(scopeId))
+                throw new InvalidOperationException($"Request for scopeId '{scopeId}' that was not pinned on the host side.");
+
             await FindAssetsAsync(_solutionStates[scopeId].Solution, checksumsToFind.Object, result, cancellationToken).ConfigureAwait(false);
             if (result.Count == numberOfChecksumsToSearch)
             {

diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.cs
@@ -43,13 +43,15 @@ internal sealed partial class RemoteWorkspace : Workspace
         /// </summary>
         private int _currentRemoteWorkspaceVersion = -1;
 
+#if SHARE_SOLUTIONS_ACROSS_CONCURRENT_CALLS
         /// <summary>
         /// Mapping from solution checksum to to the solution computed for it.  This is used so that we can hold a
         /// solution around as long as the checksum for it is being used in service of some feature operation (e.g.
         /// classification).  As long as we're holding onto it, concurrent feature requests for the same checksum can
         /// share the computation of that particular solution and avoid duplicated concurrent work.
         /// </summary>
         private readonly Dictionary<Checksum, (int refCount, AsyncLazy<Solution> lazySolution)> _checksumToRefCountAndLazySolution = new();
+#endif
 
         // internal for testing purposes.
         internal RemoteWorkspace(HostServices hostServices, string? workspaceKind)
@@ -189,8 +191,13 @@ await SlowGetSolutionAndRunAsync(
                 await DecrementLazySolutionRefcountAsync().ConfigureAwait(false);
             }
 
+#pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously
             async ValueTask<AsyncLazy<Solution>> GetLazySolutionAndIncrementRefCountAsync()
             {
+#if !SHARE_SOLUTIONS_ACROSS_CONCURRENT_CALLS
+                return
+                    AsyncLazy.Create(c => ComputeSolutionAsync(assetProvider, solutionChecksum, workspaceVersion, fromPrimaryBranch, c), cacheResult: true);
+#else
                 using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false))
                 {
                     if (_checksumToRefCountAndLazySolution.TryGetValue(solutionChecksum, out var tuple))
@@ -212,6 +219,7 @@ async ValueTask<AsyncLazy<Solution>> GetLazySolutionAndIncrementRefCountAsync()
 
                     return tuple.lazySolution;
                 }
+#endif
             }
 
             async ValueTask SetLastRequestedSolutionAsync(Solution solution)
@@ -228,6 +236,9 @@ async ValueTask SetLastRequestedSolutionAsync(Solution solution)
 
             async ValueTask DecrementLazySolutionRefcountAsync()
             {
+#if !SHARE_SOLUTIONS_ACROSS_CONCURRENT_CALLS
+                return;
+#else
                 // We use CancellationToken.None here as we have to ensure the refcount is decremented, or else we will
                 // have a memory leak.  This should hopefully not ever be an issue as we only ever hold this gate for
                 // very short periods of time in order to set do basic operations on our state.
@@ -247,7 +258,9 @@ async ValueTask DecrementLazySolutionRefcountAsync()
                         _checksumToRefCountAndLazySolution[solutionChecksum] = (refCount, lazySolution);
                     }
                 }
+#endif
             }
+#pragma warning restore CS1998 // Async method lacks 'await' operators and will run synchronously
         }
 
         /// <summary>