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

CodeBehindGenerator has improper pipeline #12978

Closed
Youssef1313 opened this issue Jan 28, 2023 · 9 comments
Closed

CodeBehindGenerator has improper pipeline #12978

Youssef1313 opened this issue Jan 28, 2023 · 9 comments
Assignees
Labels
legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf)
Milestone

Comments

@Youssef1313
Copy link
Member

Youssef1313 commented Jan 28, 2023

var sourceProvider = projectItemProvider
.Combine(xmlnsDefinitionsProvider)
.Combine(initContext.CompilationProvider)
.Select(static (t, _) => (t.Left.Left, t.Left.Right, t.Right));

This won't work well. Combining CompilationProvider and having Compilation as part of the pipeline will cause the generator to re-run on every change. So, it's not incremental at all. Note that sourceProvider is the last step.

This also roots Compilation objects in memory. See dotnet/roslyn-analyzers#6352 (Edit: I don't think there is a memory leak concern in this case)

@jsuarezruiz jsuarezruiz added the t/bug Something isn't working label Jan 30, 2023
@jsuarezruiz jsuarezruiz added this to the Backlog milestone Jan 30, 2023
@ghost
Copy link

ghost commented Jan 30, 2023

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@Youssef1313
Copy link
Member Author

Note: I think this needs a second look from Roslyn team.

@Eilon Eilon added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Jan 30, 2023
@StephaneDelcroix
Copy link
Contributor

@jonathanpeppers thoughts ?

@jonathanpeppers
Copy link
Member

I asked in Teams (private link), where some of the Roslyn folks are.

@jfversluis
Copy link
Member

jfversluis commented Oct 11, 2023

@Youssef1313 @jonathanpeppers @StephaneDelcroix any updates on this?

@jfversluis jfversluis added the legacy-area-perf Startup / Runtime performance label Oct 11, 2023
@jonathanpeppers
Copy link
Member

The points in teams are:

  1. The MAUI source generator is currently not incremental at all. It reruns on every keystroke.
  2. This is likely not an easy thing to solve at all.

You can define any type in XAML and then give it a x:Name. But it really needs to rerun if types or their namespaces change?
So, if you added a method, maybe we wouldn't need to run the source generator in that case.

One comment from the Roslyn team:

One possible approach here is to extract type/namespace names from source and metadata, and refresh that index when it changes.
It's unfortunate that somewhat requires reiventing our decltrees.

@homeyf homeyf added s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage labels Dec 1, 2023
@homeyf
Copy link

homeyf commented Dec 1, 2023

Verified this issue with Visual Studio Enterprise 17.9.0 Preview 1.0. Can repro on android platform.

mgoertz-msft added a commit that referenced this issue Feb 12, 2024
…hability

- Split out CSS SourceGen, which does not depend on Compilation at all
- Added TrackingNames to support new SourceGen unit test project

Fixes Issue #12978 CodeBehindGenerator has improper pipeline
Fixes AB#1947659: `CodeBehindGenerator` has improper pipeline
rmarinho pushed a commit that referenced this issue Feb 23, 2024
* - Restructured CodeBehindGenerator pipeline to maximize SourceGen cachability
- Split out CSS SourceGen, which does not depend on Compilation at all
- Added TrackingNames to support new SourceGen unit test project

Fixes Issue #12978 CodeBehindGenerator has improper pipeline
Fixes AB#1947659: `CodeBehindGenerator` has improper pipeline

* - Use file-scoped namespaces throughout PR
- Use raw string literals for SourceGen tests
rmarinho pushed a commit that referenced this issue Feb 27, 2024
* - Restructured CodeBehindGenerator pipeline to maximize SourceGen cachability
- Split out CSS SourceGen, which does not depend on Compilation at all
- Added TrackingNames to support new SourceGen unit test project

Fixes Issue #12978 CodeBehindGenerator has improper pipeline
Fixes AB#1947659: `CodeBehindGenerator` has improper pipeline

* - Use file-scoped namespaces throughout PR
- Use raw string literals for SourceGen tests
@mgoertz-msft
Copy link
Contributor

Completed with PR #20524. Not sure why it didn't auto-complete this item.

@akoeplinger
Copy link
Member

Not sure why it didn't auto-complete this item.

Because the PR had Fixes Issue #12978 but it needs to be Fixes #12978 for GitHub to recognize it.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 30, 2024
@Eilon Eilon added t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf) and removed legacy-area-perf Startup / Runtime performance labels May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf)
Projects
None yet
Development

No branches or pull requests

9 participants