Skip to content

Perf: Skip GetAttributes() for non-viable component properties#13091

Merged
ToddGrun merged 1 commit into
dotnet:mainfrom
ToddGrun:dev/toddgrun/skip-getattributes-ignored-properties
Apr 23, 2026
Merged

Perf: Skip GetAttributes() for non-viable component properties#13091
ToddGrun merged 1 commit into
dotnet:mainfrom
ToddGrun:dev/toddgrun/skip-getattributes-ignored-properties

Conversation

@ToddGrun
Copy link
Copy Markdown

During tag helper discovery, ComponentTagHelperProducer.GetProperties() calls GetAttributes() on every property to check for [Parameter]. This triggers expensive attribute binding and NullableWalker analysis in the Roslyn compiler for each call.

Properties that are private, static, indexers, or lack a public setter will always be Ignored regardless of attributes. For non-override properties, we can skip GetAttributes() entirely.

In Speedometer's RazorEditingTests.CompletionInCohostingForComponents, tag helper discovery re-runs on every keystroke because the source assembly symbol changes per compilation. The GetAttributes() calls contribute significantly to the 281MB of CLR_BytesAllocated_NonDevenv allocations in the devhub process — particularly NullableWalker (50MB+), BoundAttribute (13MB), and related binding infrastructure.

Override properties still go through GetAttributes() since [Parameter] determines shadow-vs-passthrough behavior.

During tag helper discovery, ComponentTagHelperProducer.GetProperties() calls GetAttributes() on every property to check for [Parameter]. This triggers expensive attribute binding and NullableWalker analysis in the Roslyn compiler for each call.

Properties that are private, static, indexers, or lack a public setter will always be Ignored regardless of attributes. For non-override properties, we can skip GetAttributes() entirely.

In Speedometer's RazorEditingTests.CompletionInCohostingForComponents, tag helper discovery re-runs on every keystroke because the source assembly symbol changes per compilation. The GetAttributes() calls contribute significantly to the 281MB of CLR_BytesAllocated_NonDevenv allocations in the devhub process — particularly NullableWalker (50MB+), BoundAttribute (13MB), and related binding infrastructure.

Override properties still go through GetAttributes() since [Parameter] determines shadow-vs-passthrough behavior.
@ToddGrun ToddGrun requested a review from a team as a code owner April 23, 2026 18:34
@ToddGrun ToddGrun merged commit b7e20f5 into dotnet:main Apr 23, 2026
10 checks passed
@dotnet-policy-service dotnet-policy-service Bot added this to the Next milestone Apr 23, 2026
@davidwengier
Copy link
Copy Markdown
Member

davidwengier commented Apr 23, 2026

For non-override properties, we can skip GetAttributes() entirely

I must be missing something about the original code, because I don't understand how this makes sense. Does that means [Parameter] public int Value {get;set;} will always be ignored because its not override? In the old code, the non-override properties were only ignored if they didn't have the attribute. BUT I must be wrong because none of the tests failed, and surely there is coverage for such a simple case. Maybe I need to have another coffee?

@ToddGrun
Copy link
Copy Markdown
Author

Copilot wrote the comment, so if there's a way to clean that up to make more sense, just let me know and I'll change it.

However, logically, if any of these property conditions in the merged if are true, the code would have previously set kind to Ignored and then called the expensive GetAttributes method. Based on the results of that, it could have previously hit the continue if IsOverride was true, otherwise it would have always added to names/results. The check I added just says if IsOverride is false, then we know we wouldn't have hit the continue, and thus would have done the addition to names/results. So, thus we can skip the GetAttributes call.


In reply to: 4309173208

@davidwengier
Copy link
Copy Markdown
Member

🤦‍♂️. Thank you! Your reply pointed out what I was missing: The big red block of removed lines made me miss that the !IsOveride check is inside that merged if, so my example would have never hit there since the property is public.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants