Skip to content

Commit 8cdb5f9

Browse files
Remove excess async state machines in completion (#11620)
A lot of the code to compute completion lists is asynchronous in order to call `DocumentContext.GetCodeDocumentAsync(...)`. However, many of these calls can be made synchronous if the `RazorCodeDocument` is computed up front and passed in. In addition, I've updated some methods that have both synchronous and asynchronous paths to return `ValueTask`. I hope that I don't conflict too badly with @davidwengier's completion work. CI Build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2662887&view=results Test Insertion: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/619102
2 parents 284b32d + 09542f8 commit 8cdb5f9

File tree

19 files changed

+319
-263
lines changed

19 files changed

+319
-263
lines changed

src/Razor/benchmarks/Microsoft.AspNetCore.Razor.Microbenchmarks/LanguageServer/RazorCompletionBenchmark.cs

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@
77
using System.Threading;
88
using System.Threading.Tasks;
99
using BenchmarkDotNet.Attributes;
10+
using Microsoft.AspNetCore.Razor.Language;
1011
using Microsoft.AspNetCore.Razor.LanguageServer;
1112
using Microsoft.AspNetCore.Razor.LanguageServer.Completion;
1213
using Microsoft.AspNetCore.Razor.LanguageServer.Completion.Delegation;
1314
using Microsoft.AspNetCore.Razor.LanguageServer.EndpointContracts;
1415
using Microsoft.AspNetCore.Razor.LanguageServer.Hosting;
1516
using Microsoft.AspNetCore.Razor.ProjectSystem;
17+
using Microsoft.AspNetCore.Razor.Telemetry;
1618
using Microsoft.CodeAnalysis.Razor.Completion;
1719
using Microsoft.CodeAnalysis.Razor.DocumentMapping;
1820
using Microsoft.CodeAnalysis.Razor.Logging;
@@ -48,7 +50,7 @@ public async Task SetupAsync()
4850
var completionListProvider = new CompletionListProvider(razorCompletionListProvider, delegatedCompletionListProvider, triggerAndCommitCharacters);
4951
var configurationService = new DefaultRazorConfigurationService(clientConnection, loggerFactory);
5052
var optionsMonitor = new RazorLSPOptionsMonitor(configurationService, RazorLSPOptions.Default);
51-
CompletionEndpoint = new RazorCompletionEndpoint(completionListProvider, triggerAndCommitCharacters, telemetryReporter: null, optionsMonitor);
53+
CompletionEndpoint = new RazorCompletionEndpoint(completionListProvider, triggerAndCommitCharacters, NoOpTelemetryReporter.Instance, optionsMonitor);
5254

5355
var clientCapabilities = new VSInternalClientCapabilities
5456
{
@@ -145,24 +147,20 @@ public TestDelegatedCompletionListProvider(
145147
IDocumentMappingService documentMappingService,
146148
IClientConnection clientConnection,
147149
CompletionListCache completionListCache,
148-
CompletionTriggerAndCommitCharacters completionTriggerAndCommitCharacters)
149-
: base(documentMappingService, clientConnection, completionListCache, completionTriggerAndCommitCharacters)
150+
CompletionTriggerAndCommitCharacters triggerAndCommitCharacters)
151+
: base(documentMappingService, clientConnection, completionListCache, triggerAndCommitCharacters)
150152
{
151153
}
152154

153-
public override Task<VSInternalCompletionList?> GetCompletionListAsync(
155+
public override ValueTask<VSInternalCompletionList?> GetCompletionListAsync(
156+
RazorCodeDocument codeDocument,
154157
int absoluteIndex,
155158
VSInternalCompletionContext completionContext,
156159
DocumentContext documentContext,
157160
VSInternalClientCapabilities clientCapabilities,
158161
RazorCompletionOptions completionOptions,
159162
Guid correlationId,
160163
CancellationToken cancellationToken)
161-
{
162-
return Task.FromResult<VSInternalCompletionList?>(
163-
new VSInternalCompletionList
164-
{
165-
});
166-
}
164+
=> new(new VSInternalCompletionList());
167165
}
168166
}

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Completion/CompletionListProvider.cs

Lines changed: 61 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Diagnostics;
67
using System.Linq;
78
using System.Threading;
89
using System.Threading.Tasks;
@@ -22,7 +23,7 @@ internal class CompletionListProvider(
2223
private readonly DelegatedCompletionListProvider _delegatedCompletionListProvider = delegatedCompletionListProvider;
2324
private readonly CompletionTriggerAndCommitCharacters _triggerAndCommitCharacters = triggerAndCommitCharacters;
2425

25-
public async Task<VSInternalCompletionList?> GetCompletionListAsync(
26+
public ValueTask<VSInternalCompletionList?> GetCompletionListAsync(
2627
int absoluteIndex,
2728
VSInternalCompletionContext completionContext,
2829
DocumentContext documentContext,
@@ -31,37 +32,78 @@ internal class CompletionListProvider(
3132
Guid correlationId,
3233
CancellationToken cancellationToken)
3334
{
34-
// First we delegate to get completion items from the individual language server
35-
var delegatedCompletionList = _triggerAndCommitCharacters.IsValidDelegationTrigger(completionContext)
36-
? await _delegatedCompletionListProvider.GetCompletionListAsync(
35+
var isDelegationTrigger = _triggerAndCommitCharacters.IsValidDelegationTrigger(completionContext);
36+
var isRazorTrigger = _triggerAndCommitCharacters.IsValidRazorTrigger(completionContext);
37+
38+
// We don't have a valid trigger, so we can't provide completions
39+
return isDelegationTrigger || isRazorTrigger
40+
? new(GetCompletionListCoreAsync(
3741
absoluteIndex,
3842
completionContext,
3943
documentContext,
4044
clientCapabilities,
4145
razorCompletionOptions,
4246
correlationId,
43-
cancellationToken).ConfigureAwait(false)
44-
: null;
47+
isDelegationTrigger,
48+
isRazorTrigger,
49+
cancellationToken))
50+
: default;
51+
}
52+
53+
private async Task<VSInternalCompletionList?> GetCompletionListCoreAsync(
54+
int absoluteIndex,
55+
VSInternalCompletionContext completionContext,
56+
DocumentContext documentContext,
57+
VSInternalClientCapabilities clientCapabilities,
58+
RazorCompletionOptions razorCompletionOptions,
59+
Guid correlationId,
60+
bool isDelegationTrigger,
61+
bool isRazorTrigger,
62+
CancellationToken cancellationToken)
63+
{
64+
Debug.Assert(isDelegationTrigger || isRazorTrigger);
65+
66+
var codeDocument = await documentContext.GetCodeDocumentAsync(cancellationToken).ConfigureAwait(false);
67+
68+
// First we delegate to get completion items from the individual language server
69+
VSInternalCompletionList? delegatedCompletionList = null;
70+
HashSet<string>? existingItems = null;
4571

46-
// Extract the items we got back from the delegated server, to inform tag helper completion
47-
var existingItems = delegatedCompletionList?.Items != null
48-
? new HashSet<string>(delegatedCompletionList.Items.Select(i => i.Label))
49-
: null;
72+
if (isDelegationTrigger)
73+
{
74+
delegatedCompletionList = await _delegatedCompletionListProvider
75+
.GetCompletionListAsync(
76+
codeDocument,
77+
absoluteIndex,
78+
completionContext,
79+
documentContext,
80+
clientCapabilities,
81+
razorCompletionOptions,
82+
correlationId,
83+
cancellationToken)
84+
.ConfigureAwait(false);
85+
86+
// Extract the items we got back from the delegated server, to inform tag helper completion
87+
if (delegatedCompletionList?.Items is { } delegatedItems)
88+
{
89+
existingItems = [.. delegatedItems.Select(static i => i.Label)];
90+
}
91+
}
5092

5193
// Now we get the Razor completion list, using information from the actual language server if necessary
52-
var razorCompletionList = _triggerAndCommitCharacters.IsValidRazorTrigger(completionContext)
53-
? await _razorCompletionListProvider.GetCompletionListAsync(
94+
VSInternalCompletionList? razorCompletionList = null;
95+
96+
if (isRazorTrigger)
97+
{
98+
razorCompletionList = _razorCompletionListProvider.GetCompletionList(
99+
codeDocument,
54100
absoluteIndex,
55101
completionContext,
56-
documentContext,
57102
clientCapabilities,
58103
existingItems,
59-
razorCompletionOptions,
60-
cancellationToken).ConfigureAwait(false)
61-
: null;
62-
63-
var finalCompletionList = CompletionListMerger.Merge(razorCompletionList, delegatedCompletionList);
104+
razorCompletionOptions);
105+
}
64106

65-
return finalCompletionList;
107+
return CompletionListMerger.Merge(razorCompletionList, delegatedCompletionList);
66108
}
67109
}

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Completion/Delegation/DelegatedCompletionListProvider.cs

Lines changed: 45 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ public DelegatedCompletionListProvider(
4242
}
4343

4444
// virtual for tests
45-
public virtual async Task<VSInternalCompletionList?> GetCompletionListAsync(
45+
public virtual ValueTask<VSInternalCompletionList?> GetCompletionListAsync(
46+
RazorCodeDocument codeDocument,
4647
int absoluteIndex,
4748
VSInternalCompletionContext completionContext,
4849
DocumentContext documentContext,
@@ -51,66 +52,78 @@ public DelegatedCompletionListProvider(
5152
Guid correlationId,
5253
CancellationToken cancellationToken)
5354
{
54-
var positionInfo = await _documentMappingService
55-
.GetPositionInfoAsync(documentContext, absoluteIndex, cancellationToken)
56-
.ConfigureAwait(false);
57-
55+
var positionInfo = _documentMappingService.GetPositionInfo(codeDocument, absoluteIndex);
5856
if (positionInfo.LanguageKind == RazorLanguageKind.Razor)
5957
{
6058
// Nothing to delegate to.
61-
return null;
59+
return default;
6260
}
6361

64-
var provisionalCompletion = await DelegatedCompletionHelper.TryGetProvisionalCompletionInfoAsync(
65-
documentContext,
66-
completionContext,
67-
positionInfo,
68-
_documentMappingService,
69-
cancellationToken).ConfigureAwait(false);
7062
TextEdit? provisionalTextEdit = null;
71-
if (provisionalCompletion is { } provisionalCompletionValue)
63+
if (DelegatedCompletionHelper.TryGetProvisionalCompletionInfo(codeDocument, completionContext, positionInfo, _documentMappingService, out var provisionalCompletion))
7264
{
73-
provisionalTextEdit = provisionalCompletionValue.ProvisionalTextEdit;
74-
positionInfo = provisionalCompletionValue.DocumentPositionInfo;
65+
provisionalTextEdit = provisionalCompletion.ProvisionalTextEdit;
66+
positionInfo = provisionalCompletion.DocumentPositionInfo;
7567
}
7668

7769
if (DelegatedCompletionHelper.RewriteContext(completionContext, positionInfo.LanguageKind, _triggerAndCommitCharacters) is not { } rewrittenContext)
7870
{
79-
return null;
71+
return default;
8072
}
8173

8274
completionContext = rewrittenContext;
8375

84-
var razorCodeDocument = await documentContext.GetCodeDocumentAsync(cancellationToken).ConfigureAwait(false);
8576
// It's a bit confusing, but we have two different "add snippets" options - one is a part of
8677
// RazorCompletionOptions and becomes a part of RazorCompletionContext and is used by
8778
// RazorCompletionFactsService, and the second one below that's used for delegated completion
8879
// Their values are not related in any way.
89-
var shouldIncludeDelegationSnippets = DelegatedCompletionHelper.ShouldIncludeSnippets(razorCodeDocument, absoluteIndex);
80+
var shouldIncludeDelegationSnippets = DelegatedCompletionHelper.ShouldIncludeSnippets(codeDocument, absoluteIndex);
9081

91-
var delegatedParams = new DelegatedCompletionParams(
82+
return new(GetDelegatedCompletionListAsync(
83+
codeDocument,
84+
absoluteIndex,
85+
completionContext,
9286
documentContext.GetTextDocumentIdentifierAndVersion(),
87+
clientCapabilities,
88+
razorCompletionOptions,
89+
correlationId,
90+
positionInfo,
91+
provisionalTextEdit,
92+
shouldIncludeDelegationSnippets,
93+
cancellationToken));
94+
}
95+
96+
private async Task<VSInternalCompletionList?> GetDelegatedCompletionListAsync(
97+
RazorCodeDocument codeDocument,
98+
int absoluteIndex,
99+
VSInternalCompletionContext completionContext,
100+
TextDocumentIdentifierAndVersion identifier,
101+
VSInternalClientCapabilities clientCapabilities,
102+
RazorCompletionOptions razorCompletionOptions,
103+
Guid correlationId,
104+
DocumentPositionInfo positionInfo,
105+
TextEdit? provisionalTextEdit,
106+
bool shouldIncludeDelegationSnippets,
107+
CancellationToken cancellationToken)
108+
{
109+
var delegatedParams = new DelegatedCompletionParams(
110+
identifier,
93111
positionInfo.Position,
94112
positionInfo.LanguageKind,
95113
completionContext,
96114
provisionalTextEdit,
97115
shouldIncludeDelegationSnippets,
98116
correlationId);
99117

100-
var delegatedResponse = await _clientConnection.SendRequestAsync<DelegatedCompletionParams, VSInternalCompletionList?>(
101-
LanguageServerConstants.RazorCompletionEndpointName,
102-
delegatedParams,
103-
cancellationToken).ConfigureAwait(false);
104-
105-
var rewrittenResponse = delegatedParams.ProjectedKind == RazorLanguageKind.CSharp
106-
? await DelegatedCompletionHelper.RewriteCSharpResponseAsync(
107-
delegatedResponse,
108-
absoluteIndex,
109-
documentContext,
110-
delegatedParams.ProjectedPosition,
111-
razorCompletionOptions,
118+
var delegatedResponse = await _clientConnection
119+
.SendRequestAsync<DelegatedCompletionParams, VSInternalCompletionList?>(
120+
LanguageServerConstants.RazorCompletionEndpointName,
121+
delegatedParams,
112122
cancellationToken)
113-
.ConfigureAwait(false)
123+
.ConfigureAwait(false);
124+
125+
var rewrittenResponse = positionInfo.LanguageKind == RazorLanguageKind.CSharp
126+
? DelegatedCompletionHelper.RewriteCSharpResponse(delegatedResponse, absoluteIndex, codeDocument, positionInfo.Position, razorCompletionOptions)
114127
: DelegatedCompletionHelper.RewriteHtmlResponse(delegatedResponse, razorCompletionOptions);
115128

116129
var completionCapability = clientCapabilities?.TextDocument?.Completion as VSInternalCompletionSetting;

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Completion/RazorCompletionEndpoint.cs

Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer.Completion;
1919
[RazorLanguageServerEndpoint(Methods.TextDocumentCompletionName)]
2020
internal class RazorCompletionEndpoint(
2121
CompletionListProvider completionListProvider,
22-
CompletionTriggerAndCommitCharacters completionTriggerAndCommitCharacters,
23-
ITelemetryReporter? telemetryReporter,
22+
CompletionTriggerAndCommitCharacters triggerAndCommitCharacters,
23+
ITelemetryReporter telemetryReporter,
2424
RazorLSPOptionsMonitor optionsMonitor)
2525
: IRazorRequestHandler<CompletionParams, VSInternalCompletionList?>, ICapabilitiesProvider
2626
{
2727
private readonly CompletionListProvider _completionListProvider = completionListProvider;
28-
private readonly CompletionTriggerAndCommitCharacters _triggerAndCommitCharacters = completionTriggerAndCommitCharacters;
29-
private readonly ITelemetryReporter? _telemetryReporter = telemetryReporter;
28+
private readonly CompletionTriggerAndCommitCharacters _triggerAndCommitCharacters = triggerAndCommitCharacters;
29+
private readonly ITelemetryReporter _telemetryReporter = telemetryReporter;
3030
private readonly RazorLSPOptionsMonitor _optionsMonitor = optionsMonitor;
3131

3232
private VSInternalClientCapabilities? _clientCapabilities;
@@ -52,46 +52,43 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(CompletionParams request
5252

5353
public async Task<VSInternalCompletionList?> HandleRequestAsync(CompletionParams request, RazorRequestContext requestContext, CancellationToken cancellationToken)
5454
{
55-
var documentContext = requestContext.DocumentContext;
56-
57-
if (request.Context is null || documentContext is null)
58-
{
59-
return null;
60-
}
61-
62-
var sourceText = await documentContext.GetSourceTextAsync(cancellationToken).ConfigureAwait(false);
63-
if (!sourceText.TryGetAbsoluteIndex(request.Position, out var hostDocumentIndex))
55+
if (request.Context is not VSInternalCompletionContext completionContext ||
56+
requestContext.DocumentContext is not { } documentContext)
6457
{
6558
return null;
6659
}
6760

68-
if (request.Context is not VSInternalCompletionContext completionContext)
61+
var autoShownCompletion = completionContext.InvokeKind != VSInternalCompletionInvokeKind.Explicit;
62+
var options = _optionsMonitor.CurrentValue;
63+
if (autoShownCompletion && !options.AutoShowCompletion)
6964
{
70-
Debug.Fail("Completion context should never be null in practice");
7165
return null;
7266
}
7367

74-
var autoShownCompletion = completionContext.InvokeKind != VSInternalCompletionInvokeKind.Explicit;
75-
if (autoShownCompletion && !_optionsMonitor.CurrentValue.AutoShowCompletion)
68+
var sourceText = await documentContext.GetSourceTextAsync(cancellationToken).ConfigureAwait(false);
69+
if (!sourceText.TryGetAbsoluteIndex(request.Position, out var hostDocumentIndex))
7670
{
7771
return null;
7872
}
7973

8074
var correlationId = Guid.NewGuid();
81-
using var _ = _telemetryReporter?.TrackLspRequest(Methods.TextDocumentCompletionName, LanguageServerConstants.RazorLanguageServerName, TelemetryThresholds.CompletionRazorTelemetryThreshold, correlationId);
75+
using (_telemetryReporter.TrackLspRequest(Methods.TextDocumentCompletionName, LanguageServerConstants.RazorLanguageServerName, TelemetryThresholds.CompletionRazorTelemetryThreshold, correlationId))
76+
{
77+
var razorCompletionOptions = new RazorCompletionOptions(
78+
SnippetsSupported: true,
79+
AutoInsertAttributeQuotes: options.AutoInsertAttributeQuotes,
80+
CommitElementsWithSpace: options.CommitElementsWithSpace);
8281

83-
var razorCompletionOptions = new RazorCompletionOptions(
84-
SnippetsSupported: true,
85-
AutoInsertAttributeQuotes: _optionsMonitor.CurrentValue.AutoInsertAttributeQuotes,
86-
CommitElementsWithSpace: _optionsMonitor.CurrentValue.CommitElementsWithSpace);
87-
var completionList = await _completionListProvider.GetCompletionListAsync(
88-
hostDocumentIndex,
89-
completionContext,
90-
documentContext,
91-
_clientCapabilities!,
92-
razorCompletionOptions,
93-
correlationId,
94-
cancellationToken).ConfigureAwait(false);
95-
return completionList;
82+
return await _completionListProvider
83+
.GetCompletionListAsync(
84+
hostDocumentIndex,
85+
completionContext,
86+
documentContext,
87+
_clientCapabilities.AssumeNotNull(),
88+
razorCompletionOptions,
89+
correlationId,
90+
cancellationToken)
91+
.ConfigureAwait(false);
92+
}
9693
}
9794
}

0 commit comments

Comments
 (0)