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

[release/6.0] Don't consider classes that do not have any attributes or base classes #58004

Merged
merged 1 commit into from
Aug 26, 2021

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Aug 24, 2021

Backport of #57750 to release/6.0

/cc @eiriktsarpalis @chsienki

Customer Impact

This is a performance optimization intended to mitigate the issues reported in #56702: sluggish VS performance when editing solutions with a large number of classes.

Testing

Does not make any changes to functional behavior, the existing source gen test suite should suffice in catching any functional regressions.

Risk

Low. Applies a straightforward performance optimization to the ISyntaxReceiver implementation filtering out any syntax nodes that the source generator is explicitly not supporting.

@ghost
Copy link

ghost commented Aug 24, 2021

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #57750 to release/6.0

/cc @eiriktsarpalis @chsienki

Customer Impact

Testing

Risk

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Text.Json

Milestone: -

@eiriktsarpalis eiriktsarpalis self-assigned this Aug 24, 2021
@eiriktsarpalis eiriktsarpalis added this to the 6.0.0 milestone Aug 24, 2021
@eiriktsarpalis eiriktsarpalis added the Servicing-consider Issue for next servicing release review label Aug 24, 2021
@danmoseley
Copy link
Member

there's no template, but approved. Risk seems low and we know we need to improve perf of the source generators to successfully ship.

I assume that a unit test isn't really feasible.

@danmoseley
Copy link
Member

@ericstj @eiriktsarpalis we're likely already flowing another set of changes for RC1, any reason we shouldn't get this in there? That would presumably improve the perf data we get from customers using RC1.

@eiriktsarpalis
Copy link
Member

there's no template, but approved.

Whoops, updated now.

we're likely already flowing another set of changes for RC1, any reason we shouldn't get this in there?

Decision was made to not backport this to RC1, since it only applies to the V1 implementation. cc @ericstj

@eiriktsarpalis
Copy link
Member

@danmoseley this should be ready to merge.

@danmoseley danmoseley merged commit 155a334 into release/6.0 Aug 26, 2021
@danmoseley danmoseley deleted the backport/pr-57750-to-release/6.0 branch August 26, 2021 14:03
@ghost ghost locked as resolved and limited conversation to collaborators Sep 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json Servicing-consider Issue for next servicing release review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants