Skip to content

Commit

Permalink
Avoid retaining memory while waiting for changes
Browse files Browse the repository at this point in the history
  • Loading branch information
sharwell committed Apr 26, 2023
1 parent 663ceb3 commit 928f694
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// 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;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
Expand Down Expand Up @@ -117,7 +116,7 @@ protected abstract ValueTask<ImmutableArray<IDiagnosticSource>> GetOrderedDiagno
/// Used by public workspace pull diagnostics to allow it to keep the connection open until
/// changes occur to avoid the client spamming the server with requests.
/// </summary>
protected virtual Task WaitForChangesAsync(RequestContext context, CancellationToken cancellationToken)
protected virtual Task WaitForChangesAsync(ILspLogger logger, CancellationToken cancellationToken)
{
return Task.CompletedTask;
}
Expand Down Expand Up @@ -191,14 +190,18 @@ await ComputeAndReportCurrentDiagnosticsAsync(
}
}

// Clear out the context to avoid retaining memory
var logger = context.Logger;
context = default;

// Some implementations of the spec will re-open requests as soon as we close them, spamming the server.
// In those cases, we wait for the implementation to indicate that changes have occurred, then we close the connection
// so that the client asks us again.
await WaitForChangesAsync(context, cancellationToken).ConfigureAwait(false);
await WaitForChangesAsync(logger, cancellationToken).ConfigureAwait(false);

// If we had a progress object, then we will have been reporting to that. Otherwise, take what we've been
// collecting and return that.
context.TraceInformation($"{this.GetType()} finished getting diagnostics");
logger.LogInformation($"{this.GetType()} finished getting diagnostics");
return CreateReturn(progress);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.EditAndContinue;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CommonLanguageServerProtocol.Framework;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics.Public;

Expand Down Expand Up @@ -159,7 +159,7 @@ private void UpdateLspChanged()
Interlocked.Exchange(ref _lspChanged, 1);
}

protected override async Task WaitForChangesAsync(RequestContext context, CancellationToken cancellationToken)
protected override async Task WaitForChangesAsync(ILspLogger logger, CancellationToken cancellationToken)
{
// Spin waiting until our LSP change flag has been set. When the flag is set (meaning LSP has changed),
// we reset the flag to false and exit out of the loop allowing the request to close.
Expand All @@ -170,7 +170,7 @@ protected override async Task WaitForChangesAsync(RequestContext context, Cancel
await Task.Delay(TimeSpan.FromMilliseconds(100), cancellationToken).ConfigureAwait(false);
}

context.TraceInformation("Closing workspace/diagnostics request");
logger.LogInformation("Closing workspace/diagnostics request");
// We've hit a change, so we close the current request to allow the client to open a new one.
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ public RequestContext(
Method = method;
}

public ILspLogger Logger => _logger;

public ClientCapabilities GetRequiredClientCapabilities()
{
return _clientCapabilities is null
Expand Down

0 comments on commit 928f694

Please sign in to comment.