-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Throttle designer attribute processing. #67056
Changes from 10 commits
cfbb25f
921d7f0
f714530
482ae85
647cef8
cff1313
a9cbbf9
bae9787
ec21669
8155ce3
bd5b9f9
d246bcf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,13 +6,14 @@ | |
using System.Collections.Concurrent; | ||
using System.Collections.Generic; | ||
using System.Collections.Immutable; | ||
using System.ComponentModel; | ||
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.LanguageService; | ||
using Microsoft.CodeAnalysis.PooledObjects; | ||
using Microsoft.CodeAnalysis.Shared.Extensions; | ||
using Roslyn.Utilities; | ||
|
@@ -22,10 +23,17 @@ namespace Microsoft.CodeAnalysis.DesignerAttribute | |
[ExportWorkspaceService(typeof(IDesignerAttributeDiscoveryService)), Shared] | ||
internal sealed partial class DesignerAttributeDiscoveryService : IDesignerAttributeDiscoveryService | ||
{ | ||
/// <summary> | ||
/// Cache from the set of references a project has to a boolean specifying if that project knows about the | ||
/// System.ComponentModel.DesignerCategoryAttribute attribute. Keyed by the metadata-references for a project | ||
/// so that we don't have to recompute it in the common case where a project's references are not changing. | ||
/// </summary> | ||
private static readonly ConditionalWeakTable<IReadOnlyList<MetadataReference>, AsyncLazy<bool>> s_metadataReferencesToDesignerAttributeInfo = new(); | ||
|
||
/// <summary> | ||
/// Protects mutable state in this type. | ||
/// </summary> | ||
private readonly SemaphoreSlim _gate = new SemaphoreSlim(initialCount: 1); | ||
private readonly SemaphoreSlim _gate = new(initialCount: 1); | ||
|
||
/// <summary> | ||
/// Keep track of the last information we reported. We will avoid notifying the host if we recompute and these | ||
|
@@ -59,6 +67,9 @@ public async ValueTask ProcessSolutionAsync( | |
if (priorityDocument != null) | ||
await ProcessProjectAsync(priorityDocument.Project, priorityDocument, callback, cancellationToken).ConfigureAwait(false); | ||
|
||
// Wait a little after the priority document and process the rest at a lower priority. | ||
await Task.Delay(DelayTimeSpan.Short, 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(); | ||
|
@@ -79,57 +90,58 @@ private async Task ProcessProjectAsync( | |
if (!project.SupportsCompilation) | ||
return; | ||
|
||
var compilation = await project.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false); | ||
var designerCategoryType = compilation.DesignerCategoryAttributeType(); | ||
if (designerCategoryType == null) | ||
return; | ||
// Defer expensive work until it's actually needed. | ||
var lazyProjectVersion = AsyncLazy.Create(project.GetSemanticVersionAsync, cacheResult: true); | ||
var lazyHasDesignerCategoryType = s_metadataReferencesToDesignerAttributeInfo.GetValue( | ||
project.MetadataReferences, | ||
_ => AsyncLazy.Create( | ||
async cancellationToken => | ||
{ | ||
var compilation = await project.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false); | ||
return compilation.DesignerCategoryAttributeType() != null; | ||
}, cacheResult: true)); | ||
|
||
await ScanForDesignerCategoryUsageAsync( | ||
project, specificDocument, callback, designerCategoryType, cancellationToken).ConfigureAwait(false); | ||
project, specificDocument, callback, lazyProjectVersion, lazyHasDesignerCategoryType, 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); | ||
await ScanForDesignerCategoryUsageAsync(project, specificDocument: null, callback, lazyProjectVersion, lazyHasDesignerCategoryType, cancellationToken).ConfigureAwait(false); | ||
} | ||
|
||
private async Task ScanForDesignerCategoryUsageAsync( | ||
Project project, | ||
Document? specificDocument, | ||
IDesignerAttributeDiscoveryService.ICallback callback, | ||
INamedTypeSymbol designerCategoryType, | ||
AsyncLazy<VersionStamp> lazyProjectVersion, | ||
AsyncLazy<bool> lazyHasDesignerCategoryType, | ||
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); | ||
|
||
// 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); | ||
project, specificDocument, lazyProjectVersion, lazyHasDesignerCategoryType, cancellationToken).ConfigureAwait(false); | ||
|
||
// Only bother reporting non-empty information to save an unnecessary RPC. | ||
if (!changedData.IsEmpty) | ||
await callback.ReportDesignerAttributeDataAsync(changedData, cancellationToken).ConfigureAwait(false); | ||
await callback.ReportDesignerAttributeDataAsync(changedData.SelectAsArray(d => d.data), 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) | ||
foreach (var (data, projectVersion) in changedData) | ||
_documentToLastReportedInformation[data.DocumentId] = (data.Category, projectVersion); | ||
} | ||
|
||
private async Task<ImmutableArray<DesignerAttributeData>> ComputeChangedDataAsync( | ||
private async Task<ImmutableArray<(DesignerAttributeData data, VersionStamp version)>> ComputeChangedDataAsync( | ||
Project project, | ||
Document? specificDocument, | ||
VersionStamp projectVersion, | ||
INamedTypeSymbol designerCategoryType, | ||
AsyncLazy<VersionStamp> lazyProjectVersion, | ||
AsyncLazy<bool> lazyHasDesignerCategoryType, | ||
CancellationToken cancellationToken) | ||
{ | ||
using var _1 = ArrayBuilder<Task<DesignerAttributeData?>>.GetInstance(out var tasks); | ||
using var _ = ArrayBuilder<(DesignerAttributeData data, VersionStamp version)>.GetInstance(out var results); | ||
foreach (var document in project.Documents) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably add a comment here why we prefer to process documents sequentially instead of concurrently? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure. will do. |
||
{ | ||
// If we're only analyzing a specific document, then skip the rest. | ||
|
@@ -143,47 +155,29 @@ private async Task<ImmutableArray<DesignerAttributeData>> ComputeChangedDataAsyn | |
|
||
// If nothing has changed at the top level between the last time we analyzed this document and now, then | ||
// no need to analyze again. | ||
var projectVersion = await lazyProjectVersion.GetValueAsync(cancellationToken).ConfigureAwait(false); | ||
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) | ||
{ | ||
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); | ||
var data = await ComputeDesignerAttributeDataAsync(document).ConfigureAwait(false); | ||
if (data.Category != existingInfo.category) | ||
results.Add((data, projectVersion)); | ||
} | ||
|
||
return results.ToImmutableAndClear(); | ||
} | ||
return results.ToImmutable(); | ||
|
||
private static async Task<DesignerAttributeData?> ComputeDesignerAttributeDataAsync( | ||
INamedTypeSymbol? designerCategoryType, Document document, CancellationToken cancellationToken) | ||
{ | ||
try | ||
async Task<DesignerAttributeData> ComputeDesignerAttributeDataAsync(Document document) | ||
{ | ||
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); | ||
var category = await ComputeDesignerAttributeCategoryAsync( | ||
lazyHasDesignerCategoryType, document, cancellationToken).ConfigureAwait(false); | ||
|
||
return new DesignerAttributeData | ||
{ | ||
|
@@ -192,8 +186,68 @@ private async Task<ImmutableArray<DesignerAttributeData>> ComputeChangedDataAsyn | |
FilePath = document.FilePath, | ||
}; | ||
} | ||
catch (Exception e) when (FatalError.ReportAndCatchUnlessCanceled(e, cancellationToken)) | ||
} | ||
|
||
public static async Task<string?> ComputeDesignerAttributeCategoryAsync( | ||
AsyncLazy<bool> lazyHasDesignerCategoryType, Document document, CancellationToken cancellationToken) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. helper was in a sibling file. just inlined into this. |
||
{ | ||
var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); | ||
var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>(); | ||
|
||
// Legacy behavior. We only register the designer info for the first non-nested class | ||
// in the file. | ||
var firstClass = FindFirstNonNestedClass(syntaxFacts.GetMembersOfCompilationUnit(root)); | ||
if (firstClass == null) | ||
return null; | ||
|
||
// simple case. If there's no DesignerCategory type in this compilation, then there's | ||
// definitely no designable types. | ||
var hasDesignerCategoryType = await lazyHasDesignerCategoryType.GetValueAsync(cancellationToken).ConfigureAwait(false); | ||
if (!hasDesignerCategoryType) | ||
return null; | ||
|
||
var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); | ||
var firstClassType = (INamedTypeSymbol)semanticModel.GetRequiredDeclaredSymbol(firstClass, cancellationToken); | ||
|
||
foreach (var type in firstClassType.GetBaseTypesAndThis()) | ||
{ | ||
cancellationToken.ThrowIfCancellationRequested(); | ||
|
||
// See if it has the designer attribute on it. Use symbol-equivalence instead of direct equality | ||
// as the symbol we have | ||
var attribute = type.GetAttributes().FirstOrDefault(d => IsDesignerAttribute(d.AttributeClass)); | ||
if (attribute is { ConstructorArguments: [{ Type.SpecialType: SpecialType.System_String, Value: string stringValue }] }) | ||
return stringValue.Trim(); | ||
} | ||
|
||
return null; | ||
|
||
static bool IsDesignerAttribute(INamedTypeSymbol? attributeClass) | ||
=> attributeClass is | ||
{ | ||
Name: nameof(DesignerCategoryAttribute), | ||
ContainingNamespace.Name: nameof(System.ComponentModel), | ||
ContainingNamespace.ContainingNamespace.Name: nameof(System), | ||
ContainingNamespace.ContainingNamespace.ContainingNamespace.IsGlobalNamespace: true, | ||
}; | ||
|
||
SyntaxNode? FindFirstNonNestedClass(SyntaxList<SyntaxNode> members) | ||
{ | ||
foreach (var member in members) | ||
{ | ||
cancellationToken.ThrowIfCancellationRequested(); | ||
if (syntaxFacts.IsBaseNamespaceDeclaration(member)) | ||
{ | ||
var firstClass = FindFirstNonNestedClass(syntaxFacts.GetMembersOfBaseNamespaceDeclaration(member)); | ||
if (firstClass != null) | ||
return firstClass; | ||
} | ||
else if (syntaxFacts.IsClassDeclaration(member)) | ||
{ | ||
return member; | ||
} | ||
} | ||
|
||
return null; | ||
} | ||
} | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original code invoked
GetDependentSemanticVersionAsync
. Is the change toGetSemanticVersionAsync
intentional here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes. will add comment.