Skip to content

Commit

Permalink
Merge pull request #50480 from sharwell/async-diagnostics
Browse files Browse the repository at this point in the history 
Avoid thread pool starvation in IDiagnosticUpdateSource.GetDiagnostics
  • Loading branch information
@sharwell
sharwell authored Jan 15, 2021
2 parents 9f0f8bd + ea9d829 commit 1ce26d3
Show file tree
Hide file tree
Showing 20 changed files with 106 additions and 100 deletions.
16 changes: 7 additions & 9 deletions src/EditorFeatures/Core/Implementation/Diagnostics/AbstractDiagnosticsTaggerProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Common;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Editor.Shared.Preview;
using Microsoft.CodeAnalysis.Editor.Shared.Tagging;
Expand Down Expand Up @@ -121,11 +120,10 @@ protected internal virtual ImmutableArray<DiagnosticDataLocation> GetLocationsTo

protected override Task ProduceTagsAsync(TaggerContext<TTag> context, DocumentSnapshotSpan spanToTag, int? caretPosition)
{
ProduceTags(context, spanToTag);
return Task.CompletedTask;
return ProduceTagsAsync(context, spanToTag);
}

private void ProduceTags(TaggerContext<TTag> context, DocumentSnapshotSpan spanToTag)
private async Task ProduceTagsAsync(TaggerContext<TTag> context, DocumentSnapshotSpan spanToTag)
{
if (!this.IsEnabled)
{
Expand Down Expand Up @@ -155,13 +153,13 @@ private void ProduceTags(TaggerContext<TTag> context, DocumentSnapshotSpan spanT

foreach (var bucket in buckets)
{
ProduceTags(
await ProduceTagsAsync(
context, spanToTag, workspace, document,
suppressedDiagnosticsSpans, bucket, cancellationToken);
suppressedDiagnosticsSpans, bucket, cancellationToken).ConfigureAwait(false);
}
}

private void ProduceTags(
private async Task ProduceTagsAsync(
TaggerContext<TTag> context, DocumentSnapshotSpan spanToTag,
Workspace workspace, Document document,
NormalizedSnapshotSpanCollection? suppressedDiagnosticsSpans,
Expand All @@ -170,11 +168,11 @@ private void ProduceTags(
try
{
var id = bucket.Id;
var diagnostics = _diagnosticService.GetPushDiagnostics(
var diagnostics = await _diagnosticService.GetPushDiagnosticsAsync(
workspace, document.Project.Id, document.Id, id,
includeSuppressedDiagnostics: false,
diagnosticMode: InternalDiagnosticsOptions.NormalDiagnosticMode,
cancellationToken);
cancellationToken).ConfigureAwait(false);

var isLiveUpdate = id is ISupportLiveUpdate;

Expand Down
33 changes: 17 additions & 16 deletions src/EditorFeatures/Test/Diagnostics/DiagnosticServiceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.CodeAnalysis.Test.Utilities;
Expand All @@ -31,7 +32,7 @@ private static DiagnosticService GetDiagnosticService(TestWorkspace workspace)
}

[Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]
public void TestGetDiagnostics1()
public async Task TestGetDiagnostics1()
{
using var workspace = new TestWorkspace(composition: FeaturesTestCompositions.Features);
var mutex = new ManualResetEvent(false);
Expand All @@ -46,21 +47,21 @@ public void TestGetDiagnostics1()
var id = Tuple.Create(workspace, document);
var diagnostic = RaiseDiagnosticEvent(mutex, source, workspace, document.Project.Id, document.Id, id);

var data1 = diagnosticService.GetPushDiagnostics(workspace, null, null, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
var data1 = await diagnosticService.GetPushDiagnosticsAsync(workspace, null, null, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
Assert.Equal(diagnostic, data1.Single());

var data2 = diagnosticService.GetPushDiagnostics(workspace, document.Project.Id, null, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
var data2 = await diagnosticService.GetPushDiagnosticsAsync(workspace, document.Project.Id, null, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
Assert.Equal(diagnostic, data2.Single());

var data3 = diagnosticService.GetPushDiagnostics(workspace, document.Project.Id, document.Id, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
var data3 = await diagnosticService.GetPushDiagnosticsAsync(workspace, document.Project.Id, document.Id, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
Assert.Equal(diagnostic, data3.Single());

var data4 = diagnosticService.GetPushDiagnostics(workspace, document.Project.Id, document.Id, id, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
var data4 = await diagnosticService.GetPushDiagnosticsAsync(workspace, document.Project.Id, document.Id, id, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
Assert.Equal(diagnostic, data4.Single());
}

[Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]
public void TestGetDiagnostics2()
public async Task TestGetDiagnostics2()
{
using var workspace = new TestWorkspace(composition: FeaturesTestCompositions.Features);
var mutex = new ManualResetEvent(false);
Expand All @@ -85,24 +86,24 @@ public void TestGetDiagnostics2()
RaiseDiagnosticEvent(mutex, source, workspace, document.Project.Id, null, id3);
RaiseDiagnosticEvent(mutex, source, workspace, null, null, Tuple.Create(workspace));

var data1 = diagnosticService.GetPushDiagnostics(workspace, null, null, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
var data1 = await diagnosticService.GetPushDiagnosticsAsync(workspace, null, null, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
Assert.Equal(5, data1.Count());

var data2 = diagnosticService.GetPushDiagnostics(workspace, document.Project.Id, null, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
var data2 = await diagnosticService.GetPushDiagnosticsAsync(workspace, document.Project.Id, null, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
Assert.Equal(4, data2.Count());

var data3 = diagnosticService.GetPushDiagnostics(workspace, document.Project.Id, null, id3, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
var data3 = await diagnosticService.GetPushDiagnosticsAsync(workspace, document.Project.Id, null, id3, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
Assert.Equal(1, data3.Count());

var data4 = diagnosticService.GetPushDiagnostics(workspace, document.Project.Id, document.Id, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
var data4 = await diagnosticService.GetPushDiagnosticsAsync(workspace, document.Project.Id, document.Id, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
Assert.Equal(2, data4.Count());

var data5 = diagnosticService.GetPushDiagnostics(workspace, document.Project.Id, document.Id, id, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
var data5 = await diagnosticService.GetPushDiagnosticsAsync(workspace, document.Project.Id, document.Id, id, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
Assert.Equal(1, data5.Count());
}

[Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]
public void TestCleared()
public async Task TestCleared()
{
using var workspace = new TestWorkspace(composition: FeaturesTestCompositions.Features);
var mutex = new ManualResetEvent(false);
Expand All @@ -128,7 +129,7 @@ public void TestCleared()
RaiseDiagnosticEvent(mutex, source2, workspace, null, null, Tuple.Create(workspace));

// confirm data is there.
var data1 = diagnosticService.GetPushDiagnostics(workspace, null, null, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
var data1 = await diagnosticService.GetPushDiagnosticsAsync(workspace, null, null, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
Assert.Equal(5, data1.Count());

diagnosticService.DiagnosticsUpdated -= MarkSet;
Expand All @@ -143,7 +144,7 @@ public void TestCleared()
mutex.WaitOne();

// confirm there are 2 data left
var data2 = diagnosticService.GetPushDiagnostics(workspace, null, null, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
var data2 = await diagnosticService.GetPushDiagnosticsAsync(workspace, null, null, null, includeSuppressedDiagnostics: false, InternalDiagnosticsOptions.NormalDiagnosticMode, CancellationToken.None);
Assert.Equal(2, data2.Count());

void MarkCalled(object sender, DiagnosticsUpdatedArgs args)
Expand Down Expand Up @@ -207,8 +208,8 @@ public TestDiagnosticUpdateSource(bool support, DiagnosticData[] diagnosticData)
public event EventHandler<DiagnosticsUpdatedArgs> DiagnosticsUpdated;
public event EventHandler DiagnosticsCleared;

public ImmutableArray<DiagnosticData> GetDiagnostics(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default)
=> _support ? _diagnosticData : ImmutableArray<DiagnosticData>.Empty;
public ValueTask<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default)
=> new(_support ? _diagnosticData : ImmutableArray<DiagnosticData>.Empty);

public void RaiseDiagnosticsUpdatedEvent(DiagnosticsUpdatedArgs args)
=> DiagnosticsUpdated?.Invoke(this, args);
Expand Down
15 changes: 8 additions & 7 deletions src/EditorFeatures/Test/Diagnostics/MockDiagnosticService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis.Common;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Options;
using Roslyn.Utilities;
using Xunit;

namespace Microsoft.CodeAnalysis.Editor.UnitTests.Diagnostics
Expand All @@ -33,17 +33,18 @@ public MockDiagnosticService()
{
}

[Obsolete]
public ImmutableArray<DiagnosticData> GetDiagnostics(Workspace workspace, ProjectId? projectId, DocumentId? documentId, object? id, bool includeSuppressedDiagnostics, CancellationToken cancellationToken)
=> GetPushDiagnostics(workspace, projectId, documentId, id, includeSuppressedDiagnostics, InternalDiagnosticsOptions.NormalDiagnosticMode, cancellationToken);
=> GetPushDiagnosticsAsync(workspace, projectId, documentId, id, includeSuppressedDiagnostics, InternalDiagnosticsOptions.NormalDiagnosticMode, cancellationToken).AsTask().WaitAndGetResult_CanCallOnBackground(cancellationToken);

public ImmutableArray<DiagnosticData> GetPullDiagnostics(Workspace workspace, ProjectId? projectId, DocumentId? documentId, object? id, bool includeSuppressedDiagnostics, Option2<DiagnosticMode> diagnosticMode, CancellationToken cancellationToken)
public ValueTask<ImmutableArray<DiagnosticData>> GetPullDiagnosticsAsync(Workspace workspace, ProjectId? projectId, DocumentId? documentId, object? id, bool includeSuppressedDiagnostics, Option2<DiagnosticMode> diagnosticMode, CancellationToken cancellationToken)
{
return GetDiagnostics(workspace, projectId, documentId);
return new ValueTask<ImmutableArray<DiagnosticData>>(GetDiagnostics(workspace, projectId, documentId));
}

public ImmutableArray<DiagnosticData> GetPushDiagnostics(Workspace workspace, ProjectId? projectId, DocumentId? documentId, object? id, bool includeSuppressedDiagnostics, Option2<DiagnosticMode> diagnosticMode, CancellationToken cancellationToken)
public ValueTask<ImmutableArray<DiagnosticData>> GetPushDiagnosticsAsync(Workspace workspace, ProjectId? projectId, DocumentId? documentId, object? id, bool includeSuppressedDiagnostics, Option2<DiagnosticMode> diagnosticMode, CancellationToken cancellationToken)
{
return GetDiagnostics(workspace, projectId, documentId);
return new ValueTask<ImmutableArray<DiagnosticData>>(GetDiagnostics(workspace, projectId, documentId));
}

private ImmutableArray<DiagnosticData> GetDiagnostics(Workspace workspace, ProjectId? projectId, DocumentId? documentId)
Expand Down
4 changes: 2 additions & 2 deletions src/EditorFeatures/TestUtilities/Squiggles/TestDiagnosticTagProducer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ public event EventHandler DiagnosticsCleared { add { } remove { } }

public bool SupportGetDiagnostics => false;

public ImmutableArray<DiagnosticData> GetDiagnostics(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default)
=> includeSuppressedDiagnostics ? _diagnostics : _diagnostics.WhereAsArray(d => !d.IsSuppressed);
public ValueTask<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default)
=> new(includeSuppressedDiagnostics ? _diagnostics : _diagnostics.WhereAsArray(d => !d.IsSuppressed));
}
}
}
5 changes: 3 additions & 2 deletions src/Features/Core/Portable/Diagnostics/AbstractHostDiagnosticUpdateSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Diagnostics
Expand All @@ -24,8 +25,8 @@ internal abstract class AbstractHostDiagnosticUpdateSource : IDiagnosticUpdateSo

public bool SupportGetDiagnostics => false;

public ImmutableArray<DiagnosticData> GetDiagnostics(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics, CancellationToken cancellationToken)
=> ImmutableArray<DiagnosticData>.Empty;
public ValueTask<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics, CancellationToken cancellationToken)
=> new(ImmutableArray<DiagnosticData>.Empty);

public event EventHandler<DiagnosticsUpdatedArgs>? DiagnosticsUpdated;
public event EventHandler DiagnosticsCleared { add { } remove { } }
Expand Down
4 changes: 2 additions & 2 deletions src/Features/Core/Portable/Diagnostics/DefaultDiagnosticAnalyzerService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ public event EventHandler DiagnosticsCleared { add { } remove { } }
// this only support push model, pull model will be provided by DiagnosticService by caching everything this one pushed
public bool SupportGetDiagnostics => false;

public ImmutableArray<DiagnosticData> GetDiagnostics(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default)
public ValueTask<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default)
{
// pull model not supported
return ImmutableArray<DiagnosticData>.Empty;
return new ValueTask<ImmutableArray<DiagnosticData>>(ImmutableArray<DiagnosticData>.Empty);
}

internal void RaiseDiagnosticsUpdated(DiagnosticsUpdatedArgs state)
Expand Down
6 changes: 3 additions & 3 deletions src/Features/Core/Portable/Diagnostics/DiagnosticAnalyzerService_UpdateSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,14 @@ internal void RaiseBulkDiagnosticsUpdated(Func<Action<DiagnosticsUpdatedArgs>, T

bool IDiagnosticUpdateSource.SupportGetDiagnostics => true;

ImmutableArray<DiagnosticData> IDiagnosticUpdateSource.GetDiagnostics(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics, CancellationToken cancellationToken)
ValueTask<ImmutableArray<DiagnosticData>> IDiagnosticUpdateSource.GetDiagnosticsAsync(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics, CancellationToken cancellationToken)
{
if (id != null)
{
return GetSpecificCachedDiagnosticsAsync(workspace, id, includeSuppressedDiagnostics, cancellationToken).WaitAndGetResult_CanCallOnBackground(cancellationToken);
return new ValueTask<ImmutableArray<DiagnosticData>>(GetSpecificCachedDiagnosticsAsync(workspace, id, includeSuppressedDiagnostics, cancellationToken));
}

return GetCachedDiagnosticsAsync(workspace, projectId, documentId, includeSuppressedDiagnostics, cancellationToken).WaitAndGetResult_CanCallOnBackground(cancellationToken);
return new ValueTask<ImmutableArray<DiagnosticData>>(GetCachedDiagnosticsAsync(workspace, projectId, documentId, includeSuppressedDiagnostics, cancellationToken));
}
}
}
Loading

0 comments on commit 1ce26d3

Please sign in to comment.