Improve incremental source generator support

[latonz]

With the current approach, almost everything is regenerated all the time. This should be better abstracted to make better use of the incremental source generator. Approaches to be discussed.

[Sergio0694]

You'll want to use ForAttributeWithMetadataName (requires Roslyn 4.3.1 or greater) to target uses of [Mapper] uses. Then you'll need to build a fully comparable model to pass down in the pipeline. Watch out if you have collection types in your model, as eg. ImmutableArray<T> is not equatable. If you need references, an example you can look at is our generators in the MVVM Toolkit, you can find them here: https://aka.ms/mvvmtoolkit/dotnet 🙂

[latonz]

@Sergio0694
Thank you for the inputs / help! I started to work on this here. However, it needs some more effort to complete.
In the meantime, since Mapperly only generates the implementation of user defined partial methods, couldn't RegisterImplementationSourceOutput be used instead of RegisterSourceOutput (which should lead to similar performance improvements)?

[nieldy]

I'm implementing a couple of source generators myself and came across the same problem, that I was re-generating the source on any change within the same project. One way I found to drastically improve this is to use a comparer that will only evaluate Equals to false if the syntax nodes aren't equal (and ignore the compilation). Sample code is below...

    public void Initialize(IncrementalGeneratorInitializationContext context)
    {
        var provider = GetIncrementalValuesProvider(context);
        
        // Combine the CompilationProvider with the transformed data, but add a custom comparer so that
        // differences can be intelligently determined in order to avoid unnecessarily invalidation of
        // the cached generated code. This is vital to avoid lots of unneeded generation in the IDE, as
        // without the comparer any change to any file in the same project will cause a re-generation.
        // See performance tips at https://www.thinktecture.com/en/net/roslyn-source-generators-performance/
        var combined = provider
            .Combine(context.CompilationProvider)
            .WithComparer(new SyntaxNodeComparer<ClassDeclarationSyntax>())
            .Collect()
            .SelectMany((array, _) => array.Distinct());
            
        context.RegisterSourceOutput(combined, (ctx, data) => Execute(ctx, data.Item2, data.Item1));
    }
    
    private IncrementalValuesProvider<ClassDeclarationSyntax> GetIncrementalValuesProvider(
        IncrementalGeneratorInitializationContext context) =>
        context.SyntaxProvider.ForAttributeWithMetadataName(
            AttributeType.FullName!,
            predicate: (node, _) => node is ClassDeclarationSyntax,
            transform: (ctx, _) => (ClassDeclarationSyntax)ctx.TargetNode);

/// <summary>
/// Compares a syntax node and compilation combination by ignoring the compilation and only comparing the
/// syntax nodes for equality. This is used for comparing a syntax node returned from a syntax provider
/// with a combined compilation.
/// The reason we ignore the compilation in the comparision is because the syntax node is immutable, but the
/// compilation is not, and therefore if there are no changes to the syntax, <c>Equals</c> will evaluate to true.
/// If we included the compilation in the equality check also then we would always return false, which would
/// result in the generated source cache frequently being invalidated when there are no changes to the syntax.
/// </summary>
public class SyntaxNodeComparer<TSyntax> : IEqualityComparer<(TSyntax Node, Compilation Compilation)>
    where TSyntax : SyntaxNode
{
    public virtual bool Equals((TSyntax Node, Compilation Compilation) x, (TSyntax Node, Compilation Compilation) y)
    {
        return x.Node.Equals(y.Node);
    }

    public virtual int GetHashCode((TSyntax Node, Compilation Compilation) obj)
    {
        return obj.Node.GetHashCode();
    }
}

Note that I go this idea from https://www.thinktecture.com/en/net/roslyn-source-generators-performance/, which was a really great source of info for me.

The issue with the solution above though is that the re-generation will only happen if the mapper file changes in some way, but not when one of the models change. In order to include the models in the comparison, you'd need to retrieve their syntax nodes and compare them too (which is more expensive... but probably a fair bit cheaper still than re-generating every time)

I have found an issue in my own generation logic that I can't (easily) check the syntax of models that live in a different assembly. I know this is possible, but I haven't gotten around to working on it yet. I found that mapperly has the same limitation (i.e. if I update a model from a different project the mapperly generated file doesn't automatically re-generate). I think this is potentially a pretty common use-case for mappings (i.e. it's common for entities to live in a different project to DTO models), so would be great if this could be improved.

[latonz]

Mapperly now uses RegisterImplementationSourceOutput which doesn't live update the generated build output but only runs the generator when building the solution. This is okay, as long as Mapperly only provides implementations for user defined methods.
When I tried to implementa fully incremental source generator approach for Mapperly, the main problem I encountered was the ClassifyConversion on the compilation used by Mapperly to determine whether one type can be converted to another.

Have you @nieldy encountered any performance issues while using Mapperly?

[Sergio0694]

"use a comparer that will only evaluate Equals to false if the syntax nodes aren't equal (and ignore the compilation)"

@nieldy absolutely do NOT do this. It will root Compilation objects and cause significant greater memory use.

[nieldy]

Ok, thanks for the tip @Sergio0694. I read about the WithComparer usage in the article I linked in my original post, and it seemed to give me the desired effect, but I had no idea it would cause memory issues. I'm very new to source generation (starting looking at it only a few weeks ago), so still trying to get my head around the best practices. What would you recommend instead of using a comparer if I want to avoid re-generating for every file touched in the project, but still have access to the Compilation?

I'm still quite confused about the effect of RegisterImplementationSourceOutput @latonz, because I tried using it, but it is still re-generating my source live (as opposed to just on the build). When I test with the latest stable version of Mappery (2.8.0), I find the mapper source live re-generates when the mapper file is touched, or one of the model files (providing it's in the same proeject) without requiring a rebuild. If a model is touched in a different project then I'm finding the mapping source isn't re-generating at all, even on a re-build... having said that, I am using Rider, so it's possibly an issue with the way it's handled in Rider. I'll test in VS and see what happens.

[Sergio0694]

"What would you recommend instead of using a comparer if I want to avoid re-generating for every file touched in the project, but still have access to the Compilation?"

You just cannot do that. Gather all the info you need from the first transform step and make it equatable. No symbols or compilation objects or anything that's not equatable should get past the first transform step, generally speaking.

"I tried using it, but it is still re-generating my source live"

RegisterImplementationSourceOutput is not wired up yet in the compiler. Right now it's identical to RegisterSourceOutput.

[nieldy]

Ah right, that makes sense @Sergio0694, thanks. I think RegisterImplementationSourceOutput will be what I want to use (when it's implemented). Do you have any idea of when that might be?

Regarding Rider behaviour vs VS. I just tried in VS and can see it is re-generating the source code when a model from a different assembly/project is modified, so it's clearly a quirk/bug in Rider. The generation also seems to be smoother and faster in VS (in Rider the code jumps about a little before it updates). Seems like JetBrains have a bit of work to do to get their implementation up to scratch.

[Sergio0694]

"I think RegisterImplementationSourceOutput will be what I want to use (when it's implemented). Do you have any idea of when that might be?"

You don't have to wait, if that makes sense for you just use that. It'll just "get faster" when Roslyn starts supporting it 🙂

[TimothyMakkison]

Looking at MVVM Toolkit it looks like it would be possible to convert MapperGenerator.Execute up to NormalizeWhitespace in SourceEmitter.Build into a pipeline step. This step would output (CompilationUnitSyntax,ImmutableArray<DiagnosticInfo>). The errors can be reported before each CompilationUnit is formatted with NormalizeWhitespace and then added.

Not sure how useful it is, this way NormalizeWhitespace, SourceText.From and ctx.AddSource will be skipped.

[latonz]

Closing this for now, as @TimothyMakkison implemented great improvements by caching before normalizing whitespace which seems to work great. We'll probably revisit this later.

[TimothyMakkison]

I can think of two possible improvements, but both are a little tricky...

• Stop using roslyn and instead emit strings - NormalizeWhitespace is the bulk of time and memory usage.
• Make *Mapping cacheable, this is a little tricky due to SourceType and TargetType and the existence of self referential method loops. This could make possible nested configurations easier as it would be easy to cull identical mappings and methods.
  • Remove the source/target types and make *Mapping a pure value, MethodMapping might become a separate object.
  • Not ideal but each *Mapping could implement its own custom comparer