Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use IAsyncEnumerable in DesignerAttributeScanning as well. #64616

Merged
merged 27 commits into from
Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
e28254c
Switch designer attribute scanning to IAsyncEnumerable
CyrusNajmabadi Oct 10, 2022
a817931
Update tests
CyrusNajmabadi Oct 10, 2022
fa69c39
Test OOP in tests
CyrusNajmabadi Oct 10, 2022
a49728d
Merge remote-tracking branch 'upstream/main' into designerAttributeSt…
CyrusNajmabadi Oct 17, 2022
c286a2e
Capitalization
CyrusNajmabadi Oct 17, 2022
74de942
Simplify
CyrusNajmabadi Oct 17, 2022
603ab84
Clean
CyrusNajmabadi Oct 17, 2022
662ed93
Inline local functions
CyrusNajmabadi Oct 17, 2022
9c82e2c
in progress
CyrusNajmabadi Oct 17, 2022
8367e95
In progress
CyrusNajmabadi Oct 18, 2022
ae73c44
In progress
CyrusNajmabadi Oct 18, 2022
4741d77
Merge remote-tracking branch 'upstream/main' into designerAttributeSt…
CyrusNajmabadi Oct 18, 2022
a640e5c
cleanup
CyrusNajmabadi Oct 18, 2022
04b3ce6
Add back
CyrusNajmabadi Oct 18, 2022
3ee80ae
Fixup test
CyrusNajmabadi Oct 18, 2022
9294e94
simplify
CyrusNajmabadi Oct 18, 2022
6a75f57
Fix
CyrusNajmabadi Oct 18, 2022
68a6cc1
Update src/VisualStudio/Core/Def/DesignerAttribute/VisualStudioDesign…
CyrusNajmabadi Oct 18, 2022
315438a
in progress
CyrusNajmabadi Oct 18, 2022
fb73214
in progress
CyrusNajmabadi Oct 18, 2022
45f5450
Do it all on teh right thread
CyrusNajmabadi Oct 18, 2022
9c6e84e
Merge branch 'designerAttributeStream' of https://github.com/CyrusNaj…
CyrusNajmabadi Oct 18, 2022
9f6448e
delete
CyrusNajmabadi Oct 18, 2022
e908fda
REvert
CyrusNajmabadi Oct 18, 2022
6a8f876
Ensure we clear out documents
CyrusNajmabadi Oct 19, 2022
58b571c
Simplify
CyrusNajmabadi Oct 19, 2022
5bac3e0
Fix test
CyrusNajmabadi Oct 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,12 @@
// 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;
using System.Composition;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.ErrorReporting;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
Expand All @@ -22,154 +19,51 @@ namespace Microsoft.CodeAnalysis.DesignerAttribute
[ExportWorkspaceService(typeof(IDesignerAttributeDiscoveryService)), Shared]
internal sealed partial class DesignerAttributeDiscoveryService : IDesignerAttributeDiscoveryService
{
/// <summary>
/// Protects mutable state in this type.
/// </summary>
private readonly SemaphoreSlim _gate = new SemaphoreSlim(initialCount: 1);

/// <summary>
/// Keep track of the last information we reported. We will avoid notifying the host if we recompute and these
/// don't change.
/// </summary>
private readonly ConcurrentDictionary<DocumentId, (string? category, VersionStamp projectVersion)> _documentToLastReportedInformation = new();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the remote side. it became completely stateless (which is good). there was a lot of complexity about the remote side trying to keep track of what hte host side might now and preventing updates. this was error prone and doesn't work in the IASyncEnumerable world. e.g. in remote IAE, jsut because you yielded the data, you dont' know that hte host has actually gotten it.

this moved to a cleaner model where the remote side knows nothing, and the host side keeps track.


[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public DesignerAttributeDiscoveryService()
{
}

public async ValueTask ProcessSolutionAsync(
Solution solution,
DocumentId? priorityDocumentId,
IDesignerAttributeDiscoveryService.ICallback callback,
CancellationToken cancellationToken)
{
using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false))
{
// Remove any documents that are now gone.
foreach (var docId in _documentToLastReportedInformation.Keys)
{
if (!solution.ContainsDocument(docId))
_documentToLastReportedInformation.TryRemove(docId, out _);
}

// Handle the priority doc first.
var priorityDocument = solution.GetDocument(priorityDocumentId);
if (priorityDocument != null)
await ProcessProjectAsync(priorityDocument.Project, priorityDocument, callback, cancellationToken).ConfigureAwait(false);

// Process the rest of the projects in dependency order so that their data is ready when we hit the
// projects that depend on them.
var dependencyGraph = solution.GetProjectDependencyGraph();
foreach (var projectId in dependencyGraph.GetTopologicallySortedProjects(cancellationToken))
{
if (projectId != priorityDocumentId?.ProjectId)
await ProcessProjectAsync(solution.GetRequiredProject(projectId), specificDocument: null, callback, cancellationToken).ConfigureAwait(false);
}
}
}

private async Task ProcessProjectAsync(
public async IAsyncEnumerable<DesignerAttributeData> ProcessProjectAsync(
Project project,
Document? specificDocument,
IDesignerAttributeDiscoveryService.ICallback callback,
CancellationToken cancellationToken)
DocumentId? priorityDocumentId,
[EnumeratorCancellation] CancellationToken cancellationToken)
{
// Ignore projects that don't support compilation or don't even have the DesignerCategoryAttribute in it.
if (!project.SupportsCompilation)
return;
yield break;

var compilation = await project.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false);
var designerCategoryType = compilation.DesignerCategoryAttributeType();
if (designerCategoryType == null)
return;

await ScanForDesignerCategoryUsageAsync(
project, specificDocument, callback, designerCategoryType, cancellationToken).ConfigureAwait(false);

// If we scanned just a specific document in the project, now scan the rest of the files.
if (specificDocument != null)
await ScanForDesignerCategoryUsageAsync(project, specificDocument: null, callback, designerCategoryType, cancellationToken).ConfigureAwait(false);
}

private async Task ScanForDesignerCategoryUsageAsync(
Project project,
Document? specificDocument,
IDesignerAttributeDiscoveryService.ICallback callback,
INamedTypeSymbol designerCategoryType,
CancellationToken cancellationToken)
{
// We need to reanalyze the project whenever it (or any of its dependencies) have
// changed. We need to know about dependencies since if a downstream project adds the
// DesignerCategory attribute to a class, that can affect us when we examine the classes
// in this project.
var projectVersion = await project.GetDependentSemanticVersionAsync(cancellationToken).ConfigureAwait(false);
yield break;

// Now get all the values that actually changed and notify VS about them. We don't need
// to tell it about the ones that didn't change since that will have no effect on the
// user experience.
var changedData = await ComputeChangedDataAsync(
project, specificDocument, projectVersion, designerCategoryType, cancellationToken).ConfigureAwait(false);

// Only bother reporting non-empty information to save an unnecessary RPC.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't necessary anymore. by definition, if we don't yield something, we have nothing to send over the rpc channel.

if (!changedData.IsEmpty)
await callback.ReportDesignerAttributeDataAsync(changedData, cancellationToken).ConfigureAwait(false);

// Now, keep track of what we've reported to the host so we won't report unchanged files in the future. We
// do this after the report has gone through as we want to make sure that if it cancels for any reason we
// don't hold onto values that may not have made it all the way to the project system.
foreach (var data in changedData)
_documentToLastReportedInformation[data.DocumentId] = (data.Category, projectVersion);
}
// If there is a priority doc, then scan that first.
var priorityDocument = priorityDocumentId == null ? null : project.GetDocument(priorityDocumentId);
if (priorityDocument is { FilePath: not null })
{
var data = await ComputeDesignerAttributeDataAsync(designerCategoryType, priorityDocument, cancellationToken).ConfigureAwait(false);
if (data != null)
yield return data.Value;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is much cleaner too. we only send the data for things that actually have a category. in practice this is a tiny handful of messages in a few projects. This is different from before when we'd send info about every document, even those without a category.

}

private async Task<ImmutableArray<DesignerAttributeData>> ComputeChangedDataAsync(
Project project,
Document? specificDocument,
VersionStamp projectVersion,
INamedTypeSymbol designerCategoryType,
CancellationToken cancellationToken)
{
using var _1 = ArrayBuilder<Task<DesignerAttributeData?>>.GetInstance(out var tasks);
// now process the rest of the documents.
using var _ = ArrayBuilder<Task<DesignerAttributeData?>>.GetInstance(out var tasks);
foreach (var document in project.Documents)
{
// If we're only analyzing a specific document, then skip the rest.
if (specificDocument != null && document != specificDocument)
continue;

// If we don't have a path for this document, we cant proceed with it.
// We need that path to inform the project system which file we're referring to.
if (document.FilePath == null)
if (document == priorityDocument || document.FilePath is null)
continue;

// If nothing has changed at the top level between the last time we analyzed this document and now, then
// no need to analyze again.
if (_documentToLastReportedInformation.TryGetValue(document.Id, out var existingInfo) &&
existingInfo.projectVersion == projectVersion)
{
continue;
}

tasks.Add(ComputeDesignerAttributeDataAsync(designerCategoryType, document, cancellationToken));
}

using var _2 = ArrayBuilder<DesignerAttributeData>.GetInstance(tasks.Count, out var results);

// Avoid unnecessary allocation of result array.
await Task.WhenAll((IEnumerable<Task>)tasks).ConfigureAwait(false);

foreach (var task in tasks)
// Convert the tasks into one final stream we can read all the results from.
await foreach (var dataOpt in tasks.ToImmutable().StreamAsync(cancellationToken).ConfigureAwait(false))
{
var dataOpt = await task.ConfigureAwait(false);
if (dataOpt == null)
continue;

var data = dataOpt.Value;
_documentToLastReportedInformation.TryGetValue(data.DocumentId, out var existingInfo);
if (existingInfo.category != data.Category)
results.Add(data);
if (dataOpt != null)
yield return dataOpt.Value;
}

return results.ToImmutableAndClear();
}

private static async Task<DesignerAttributeData?> ComputeDesignerAttributeDataAsync(
Expand All @@ -179,18 +73,16 @@ private async Task<ImmutableArray<DesignerAttributeData>> ComputeChangedDataAsyn
{
Contract.ThrowIfNull(document.FilePath);

// We either haven't computed the designer info, or our data was out of date. We need
// So recompute here. Figure out what the current category is, and if that's different
// from what we previously stored.
var category = await DesignerAttributeHelpers.ComputeDesignerAttributeCategoryAsync(
designerCategoryType, document, cancellationToken).ConfigureAwait(false);

return new DesignerAttributeData
{
Category = category,
DocumentId = document.Id,
FilePath = document.FilePath,
};
// If there's no category (the common case) don't return anything. The host itself will see no results
// returned and can handle that case (for example, if a type previously had the attribute but doesn't
// any longer).
if (category == null)
return null;

return new DesignerAttributeData(category, document.Id, document.FilePath);
}
catch (Exception e) when (FatalError.ReportAndCatchUnlessCanceled(e, cancellationToken))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,34 @@ namespace Microsoft.CodeAnalysis.DesignerAttribute
/// Serialization typed used to pass information to/from OOP and VS.
/// </summary>
[DataContract]
internal struct DesignerAttributeData
internal readonly struct DesignerAttributeData
{
/// <summary>
/// The category specified in a <c>[DesignerCategory("...")]</c> attribute.
/// </summary>
[DataMember(Order = 0)]
public string? Category;
public readonly string? Category;

/// <summary>
/// The document this <see cref="Category"/> applies to.
/// </summary>
[DataMember(Order = 1)]
public DocumentId DocumentId;
public readonly DocumentId DocumentId;

/// <summary>
/// Path for this <see cref="DocumentId"/>.
/// </summary>
[DataMember(Order = 2)]
public string FilePath;
public readonly string FilePath;

public DesignerAttributeData(string? category, DocumentId documentId, string filePath)
{
Category = category;
DocumentId = documentId;
FilePath = filePath;
}

public DesignerAttributeData WithCategory(string? category)
=> new(category, DocumentId, FilePath);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ internal static class DesignerAttributeHelpers
}

return null;

static string? GetArgumentString(TypedConstant argument)
=> argument is { IsNull: false, Type.SpecialType: SpecialType.System_String, Value: string stringValue } ? stringValue.Trim() : null;
}

private static SyntaxNode? FindFirstNonNestedClass(
Expand All @@ -77,18 +80,5 @@ internal static class DesignerAttributeHelpers

return null;
}

private static string? GetArgumentString(TypedConstant argument)
{
if (argument.Type == null ||
argument.Type.SpecialType != SpecialType.System_String ||
argument.IsNull ||
argument.Value is not string stringValue)
{
return null;
}

return stringValue.Trim();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,14 @@
// 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.Immutable;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Host;

namespace Microsoft.CodeAnalysis.DesignerAttribute
{
internal partial interface IDesignerAttributeDiscoveryService : IWorkspaceService
internal interface IDesignerAttributeDiscoveryService : IWorkspaceService
{
public interface ICallback
{
ValueTask ReportDesignerAttributeDataAsync(ImmutableArray<DesignerAttributeData> data, CancellationToken cancellationToken);
}

ValueTask ProcessSolutionAsync(Solution solution, DocumentId? priorityDocumentId, ICallback callback, CancellationToken cancellationToken);
IAsyncEnumerable<DesignerAttributeData> ProcessProjectAsync(Project project, DocumentId? priorityDocumentId, CancellationToken cancellationToken);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,8 @@
// 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.Immutable;
using System.Composition;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Remote;

namespace Microsoft.CodeAnalysis.DesignerAttribute
{
Expand All @@ -18,27 +13,6 @@ namespace Microsoft.CodeAnalysis.DesignerAttribute
/// </summary>
internal interface IRemoteDesignerAttributeDiscoveryService
{
internal interface ICallback
{
ValueTask ReportDesignerAttributeDataAsync(RemoteServiceCallbackId callbackId, ImmutableArray<DesignerAttributeData> data, CancellationToken cancellationToken);
}

ValueTask DiscoverDesignerAttributesAsync(RemoteServiceCallbackId callbackId, Checksum solutionChecksum, DocumentId? priorityDocument, CancellationToken cancellationToken);
}

[ExportRemoteServiceCallbackDispatcher(typeof(IRemoteDesignerAttributeDiscoveryService)), Shared]
internal sealed class RemoteDesignerAttributeDiscoveryCallbackDispatcher : RemoteServiceCallbackDispatcher, IRemoteDesignerAttributeDiscoveryService.ICallback
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public RemoteDesignerAttributeDiscoveryCallbackDispatcher()
{
}

private new IDesignerAttributeDiscoveryService.ICallback GetCallback(RemoteServiceCallbackId callbackId)
=> (IDesignerAttributeDiscoveryService.ICallback)base.GetCallback(callbackId);

public ValueTask ReportDesignerAttributeDataAsync(RemoteServiceCallbackId callbackId, ImmutableArray<DesignerAttributeData> data, CancellationToken cancellationToken)
=> GetCallback(callbackId).ReportDesignerAttributeDataAsync(data, cancellationToken);
IAsyncEnumerable<DesignerAttributeData> DiscoverDesignerAttributesAsync(Checksum solutionChecksum, ProjectId project, DocumentId? priorityDocument, CancellationToken cancellationToken);
}
}
Loading