Skip to content

Conversation

@eiriktsarpalis
Copy link
Member

@eiriktsarpalis eiriktsarpalis commented May 23, 2023

Addresses the last outstanding issue from #68353, namely

We are calling Collect() on the ClassDeclarationSyntax values provider, feeding every single JsonSerializerContext declaration to the source generator regardless of whether it's a newly updated or cached value.

Slightly refactors the Parser and Emitter classes so that they can be used with singular JsonSerializerContext declarations, allowing the incremental generator to generate source code for each of its inputs independently and on-demand.

Fix #68353.

@ghost
Copy link

ghost commented May 23, 2023

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Addresses the last outstanding issue from #68353, namely

We are calling Collect() on the ClassDeclarationSyntax values provider, feeding every single JsonSerializerContext declaration to the source generator regardless of whether it's a newly updated or cached value.

Slightly refactors the Parser and Emitter classes so that they can be used with singular JsonSerializerContext declarations, allowing the incremental generator to generate source code for each of its inputs independently and on-demand.

Author: eiriktsarpalis
Assignees: eiriktsarpalis
Labels:

area-System.Text.Json

Milestone: -

@eiriktsarpalis eiriktsarpalis added this to the 8.0.0 milestone May 23, 2023

namespace System.Text.Json.SourceGeneration
{
public sealed partial class JsonSourceGenerator
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moves the Diagnostic descriptors to a standalone file for easier access.

JsonSourceGenerationMode generationMode,
CancellationToken cancellationToken)
{
IEnumerable<SyntaxNode> attributeArguments = attributeSyntax.DescendantNodes().Where(node => node is AttributeArgumentSyntax);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic moved to ParseJsonSerializableAttribute


List<ContextGenerationSpec>? contextGenSpecList = null;

foreach (IGrouping<SyntaxTree, ClassDeclarationSyntax> group in classDeclarationSyntaxList.GroupBy(c => c.SyntaxTree))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This grouping was originally included to amortise the cost of the expensive compilation.GetSemanticModel method. However the semantic model is provided to us by the source generator context types, so calling the method is not actually necessary.

@eiriktsarpalis eiriktsarpalis merged commit eac41a3 into dotnet:main May 23, 2023
@eiriktsarpalis eiriktsarpalis deleted the remove/stj-combine branch May 23, 2023 19:38
@ghost ghost locked as resolved and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System.Text.Json incremental source generator performance issues

7 participants