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

Iterate on config binding generator parsing pipeline to benefit from input caching provided by the incremental-generator feature #83534

Closed
Tracked by #79527
layomia opened this issue Mar 16, 2023 · 4 comments · Fixed by #89587
Assignees
Labels
area-Extensions-Configuration blocking-release source-generator Indicates an issue with a source generator feature
Milestone

Comments

@layomia
Copy link
Contributor

layomia commented Mar 16, 2023

Address feedback from @eiriktsarpalis and @sharwell in #82179 (review) and related discussions.

@layomia layomia added area-Extensions-Configuration source-generator Indicates an issue with a source generator feature labels Mar 16, 2023
@layomia layomia added this to the 8.0.0 milestone Mar 16, 2023
@layomia layomia self-assigned this Mar 16, 2023
@ghost
Copy link

ghost commented Mar 16, 2023

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

Address feedback from @eiriktsarpalis in #82179 (review) and related discussions.

Author: layomia
Assignees: layomia
Labels:

area-Extensions-Configuration, source-generator

Milestone: 8.0.0

@layomia
Copy link
Contributor Author

layomia commented May 16, 2023

Address feedback from #86285 (comment).

@layomia
Copy link
Contributor Author

layomia commented Aug 15, 2023

Triage: moving this issue to 9.0, mitigating 8.0 parsing perf with #90619.

  • We haven't had customer reports of the performance problems driving this issue. This effort is preemptive based on best practice / code-review.
  • Generator is off-by default. If enabled and perf issues arise, a workaround would be as follows:
    • for projects with no binding, disable the generator
    • for project with binding, remove it from design time builds with an MSBuild target

@layomia
Copy link
Contributor Author

layomia commented Aug 23, 2023

From @jaredpar in https://github.com/dotnet/runtime/pull/90835/files#r1303627652. Address in #89587:

The equals implementation on BinderInvocation uses ref equality, not value equality. That means the CreateSyntaxProvider call above is never cached. This is an issue for IDE performance.

@layomia layomia modified the milestones: 9.0.0, 8.0.0 Aug 30, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Sep 27, 2023
@ghost ghost added in-pr There is an active PR which will close this issue when it is merged and removed in-pr There is an active PR which will close this issue when it is merged labels Sep 27, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Nov 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-Configuration blocking-release source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant