Maui: Incremental generator shouldn't include symbols in the pipeline

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

TextColorToGenerator includes symbols in userGeneratedClassesProvider and mauiControlsAssemblySymbolProvider, then filtered out in inputs.

https://github.com/CommunityToolkit/Maui/blob/9925e1074a7fc18f3072f2c79036904758d7b48e/src/CommunityToolkit.Maui.SourceGenerators/Generators/TextColorToGenerator.cs#L28-L84

Expected Behavior

It’s a better idea to not include symbols in the pipeline, ie, filter everything early.

Steps To Reproduce

N/A

Link to public reproduction project repository

N/A

Environment

N/A

Anything else?

https://github.com/dotnet/roslyn-analyzers/issues/6352

About this issue

  • Original URL
  • State: open
  • Created 2 years ago
  • Comments: 21 (9 by maintainers)

Most upvoted comments

Sorry my bad, I was missing the last couple lines in the selector. You’re correct, it will not generate all files every time as the last two steps are cached. It will still, however, re-run all previous steps for every single edit up until that point (unnecessarily), and also root compilation objects, causing them to remain in memory for longer than necessary and increase memory usage.

I agree this is less critical than if it was really regenerating all files for every edit, sure. Still worth a fix, given time though 🙂

“your comment was very rude and I didn’t like it”

I’m sure Youssef will be able to say that, but I can guarantee you he didn’t mean that to come across that way (language barrier maybe?). He’s a super active member of the community that’s always extremely helpful and willing to help folks out, and with lots of community contributions in Roslyn and other repos (eg. he helped in the Toolkit before as well). I read that as “let’s establish that this needs an action and it’s not an incorrect issue to dismiss”, rather than “you have to fix this now” as a demand 🙂

“we generate those classes just once”

This is where the confusion might be coming from. The issue we’re trying to point out is that, due to how the pipeline is currently setup, you think you’re generating those classes just once, when in fact the generator is doing the entire work and generating every single file again for every single keystroke currently, all the time, because the pipeline is always invalidated 😅

I’ll clarify my previous comment and hide this and related answers. And I should have written this before, to avoid misunderstandings

Not really stale. This needs an action.

@Youssef1313, your comment was very rude and I didn’t like it, you’re not my boss nor the other members that maintain this project. We (core team members) are not paid to work on this, we do it for free, so we expect more respect from community members. Please, be more kind in your next interactions.

@pictos Nobody’s arguing that the generators are not working, the issue is they’re have some architectural issues that can (and will) cause performance and memory usage overhead. The last PR might very well have improved the situation (I certainly don’t doubt the generator might’ve been worse before than it is now), but that doesn’t mean it’s fine now. It unfortunately is not.

@Sergio0694 that wasn’t the reason for my comment, I’m totally fine to be wrong, TBH I’m wrong most of the time, of course, we can engage and make our SG better when we have time. Since I work on this during my spare time I don’t have too many resources to invest here and my focus goes where we need most. Right now the current performance is fine, we generate those classes just once (since the code is for controls inside .NET MAUI assembly, so the user can’t change it). PRs improving our SG or a better guide will be more than welcome to help me (and the team) to implement this and improve this.

I would like to see a benchmark before changing this. I tested it before moving forward and the SG keeps its incremental behavior. It’s true that having symbols will break the Incremental behavior, but just if it gets inside the RegisterSourceOutput. As you can see at the end all the symbols are converted to our own model.