From 66d8b753ef7876dbdde984d3b04df499f8c7ad19 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 18 Jan 2022 16:05:07 -0800 Subject: [PATCH 1/3] Enable using cached results of the transform function for a SyntaxProvider when the SyntaxNode entry is cached. --- .../SourceGeneration/Nodes/SyntaxInputNode.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxInputNode.cs b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxInputNode.cs index fbe9860abfe0c..28f64df8e9f3e 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxInputNode.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxInputNode.cs @@ -97,13 +97,16 @@ public void VisitTree(Lazy root, EntryState state, SemanticModel? mo // now, using the obtained syntax nodes, run the transform foreach (SyntaxNode node in nodes) { - var stopwatch = SharedStopwatch.StartNew(); - var value = new GeneratorSyntaxContext(node, model); - var transformed = _owner._transformFunc(value, cancellationToken); - - if (state == EntryState.Added || !_transformTable.TryModifyEntry(transformed, _owner._comparer, stopwatch.Elapsed, noInputStepsStepInfo, state)) + if (state != EntryState.Cached || !_transformTable.TryUseCachedEntries(TimeSpan.Zero, noInputStepsStepInfo)) { - _transformTable.AddEntry(transformed, EntryState.Added, stopwatch.Elapsed, noInputStepsStepInfo, EntryState.Added); + var stopwatch = SharedStopwatch.StartNew(); + var value = new GeneratorSyntaxContext(node, model); + var transformed = _owner._transformFunc(value, cancellationToken); + + if (state == EntryState.Added || !_transformTable.TryModifyEntry(transformed, _owner._comparer, stopwatch.Elapsed, noInputStepsStepInfo, state)) + { + _transformTable.AddEntry(transformed, EntryState.Added, stopwatch.Elapsed, noInputStepsStepInfo, EntryState.Added); + } } } } From d197ad10759425c8a05921a087a6159d9a925165 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 19 Jan 2022 16:34:27 -0800 Subject: [PATCH 2/3] Never treat the transform input as cached as one of the inputs, the SemanticModel, is never cached between runs. --- .../SourceGeneration/Nodes/SyntaxInputNode.cs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxInputNode.cs b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxInputNode.cs index 28f64df8e9f3e..62471ad9f07f6 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxInputNode.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxInputNode.cs @@ -97,16 +97,17 @@ public void VisitTree(Lazy root, EntryState state, SemanticModel? mo // now, using the obtained syntax nodes, run the transform foreach (SyntaxNode node in nodes) { - if (state != EntryState.Cached || !_transformTable.TryUseCachedEntries(TimeSpan.Zero, noInputStepsStepInfo)) + var stopwatch = SharedStopwatch.StartNew(); + var value = new GeneratorSyntaxContext(node, model); + var transformed = _owner._transformFunc(value, cancellationToken); + + // The SemanticModel we provide to GeneratorSyntaxContext is never guaranteed to be the same between runs, + // so we never consider the input to the transform as cached. + var transformInputState = state == EntryState.Cached ? EntryState.Modified : state; + + if (transformInputState == EntryState.Added || !_transformTable.TryModifyEntry(transformed, _owner._comparer, stopwatch.Elapsed, noInputStepsStepInfo, transformInputState)) { - var stopwatch = SharedStopwatch.StartNew(); - var value = new GeneratorSyntaxContext(node, model); - var transformed = _owner._transformFunc(value, cancellationToken); - - if (state == EntryState.Added || !_transformTable.TryModifyEntry(transformed, _owner._comparer, stopwatch.Elapsed, noInputStepsStepInfo, state)) - { - _transformTable.AddEntry(transformed, EntryState.Added, stopwatch.Elapsed, noInputStepsStepInfo, EntryState.Added); - } + _transformTable.AddEntry(transformed, EntryState.Added, stopwatch.Elapsed, noInputStepsStepInfo, EntryState.Added); } } } From 8631167275c63451e20f4e3a99812f8338b412c3 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 20 Jan 2022 11:01:16 -0800 Subject: [PATCH 3/3] Update tests. --- .../SyntaxAwareGeneratorTests.cs | 79 ++++++++++--------- 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs index ce8de0cfc2d46..40724aa68ed2e 100644 --- a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs @@ -946,16 +946,26 @@ class C Compilation compilation = CreateCompilation(source1, options: TestOptions.DebugDll, parseOptions: parseOptions); compilation.VerifyDiagnostics(); + List syntaxFilterVisited = new(); var testGenerator = new PipelineCallbackGenerator(context => { - var source = context.SyntaxProvider.CreateSyntaxProvider((c, _) => c is FieldDeclarationSyntax fds, (c, _) => ((FieldDeclarationSyntax)c.Node).Declaration.Variables[0].Identifier.ValueText).WithTrackingName("Fields"); + var source = context.SyntaxProvider.CreateSyntaxProvider((c, _) => + { + if (c is FieldDeclarationSyntax fds) + { + syntaxFilterVisited.Add(fds.Declaration.Variables[0].Identifier.ValueText); + return true; + } + return false; + }, (c, _) => ((FieldDeclarationSyntax)c.Node).Declaration.Variables[0].Identifier.ValueText); context.RegisterSourceOutput(source, (spc, fieldName) => { spc.AddSource(fieldName, ""); }); }); - GeneratorDriver driver = CSharpGeneratorDriver.Create(new[] { new IncrementalGeneratorWrapper(testGenerator) }, parseOptions: parseOptions, driverOptions: new GeneratorDriverOptions(IncrementalGeneratorOutputKind.None, trackIncrementalGeneratorSteps: true)); + // Don't enable incremental tracking here as incremental tracking disables the "unchanged compilation implies unchanged syntax trees" optimization. + GeneratorDriver driver = CSharpGeneratorDriver.Create(new[] { new IncrementalGeneratorWrapper(testGenerator) }, parseOptions: parseOptions); driver = driver.RunGenerators(compilation); var results = driver.GetRunResult(); @@ -964,14 +974,9 @@ class C Assert.EndsWith("fieldA.cs", results.GeneratedTrees[0].FilePath); Assert.EndsWith("fieldB.cs", results.GeneratedTrees[1].FilePath); Assert.EndsWith("fieldC.cs", results.GeneratedTrees[2].FilePath); - Assert.Collection(results.Results[0].TrackedSteps["Fields"], - step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldA", IncrementalStepRunReason.New), output)), - step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldB", IncrementalStepRunReason.New), output)), - step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldC", IncrementalStepRunReason.New), output))); + Assert.Equal(new[] { "fieldA", "fieldB", "fieldC" }, syntaxFilterVisited); + syntaxFilterVisited.Clear(); // run again on the *same* compilation driver = driver.RunGenerators(compilation); results = driver.GetRunResult(); @@ -980,13 +985,7 @@ class C Assert.EndsWith("fieldA.cs", results.GeneratedTrees[0].FilePath); Assert.EndsWith("fieldB.cs", results.GeneratedTrees[1].FilePath); Assert.EndsWith("fieldC.cs", results.GeneratedTrees[2].FilePath); - Assert.Collection(results.Results[0].TrackedSteps["Fields"], - step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldA", IncrementalStepRunReason.Cached), output)), - step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldB", IncrementalStepRunReason.Cached), output)), - step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldC", IncrementalStepRunReason.Cached), output))); + Assert.Empty(syntaxFilterVisited); // now change the compilation, but don't change the syntax trees compilation = compilation.WithAssemblyName("newCompilation"); @@ -997,13 +996,7 @@ class C Assert.EndsWith("fieldA.cs", results.GeneratedTrees[0].FilePath); Assert.EndsWith("fieldB.cs", results.GeneratedTrees[1].FilePath); Assert.EndsWith("fieldC.cs", results.GeneratedTrees[2].FilePath); - Assert.Collection(results.Results[0].TrackedSteps["Fields"], - step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldA", IncrementalStepRunReason.Cached), output)), - step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldB", IncrementalStepRunReason.Cached), output)), - step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldC", IncrementalStepRunReason.Cached), output))); + Assert.Equal(new[] { "fieldA", "fieldB", "fieldC" }, syntaxFilterVisited); } [Fact] @@ -1070,11 +1063,11 @@ class D Assert.EndsWith("fieldE.cs", results.GeneratedTrees[4].FilePath); Assert.Collection(results.Results[0].TrackedSteps["Fields"], step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldA", IncrementalStepRunReason.Cached), output)), + output => Assert.Equal(("fieldA", IncrementalStepRunReason.Unchanged), output)), step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldB", IncrementalStepRunReason.Cached), output)), + output => Assert.Equal(("fieldB", IncrementalStepRunReason.Unchanged), output)), step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldC", IncrementalStepRunReason.Cached), output)), + output => Assert.Equal(("fieldC", IncrementalStepRunReason.Unchanged), output)), step => Assert.Collection(step.Outputs, output => Assert.Equal(("fieldD", IncrementalStepRunReason.New), output)), step => Assert.Collection(step.Outputs, @@ -1149,11 +1142,11 @@ class D Assert.EndsWith("fieldC.cs", results.GeneratedTrees[2].FilePath); Assert.Collection(results.Results[0].TrackedSteps["Fields"], step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldA", IncrementalStepRunReason.Cached), output)), + output => Assert.Equal(("fieldA", IncrementalStepRunReason.Unchanged), output)), step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldB", IncrementalStepRunReason.Cached), output)), + output => Assert.Equal(("fieldB", IncrementalStepRunReason.Unchanged), output)), step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldC", IncrementalStepRunReason.Cached), output)), + output => Assert.Equal(("fieldC", IncrementalStepRunReason.Unchanged), output)), step => Assert.Collection(step.Outputs, output => Assert.Equal(("fieldD", IncrementalStepRunReason.Removed), output)), step => Assert.Collection(step.Outputs, @@ -1273,6 +1266,7 @@ class F }", parseOptions); compilation = compilation.ReplaceSyntaxTree(firstTree, newTree); + fieldsCalledFor.Clear(); // now re-run the drivers driver = driver.RunGenerators(compilation); results = driver.GetRunResult(); @@ -1287,9 +1281,10 @@ class F step => Assert.Collection(step.Outputs, output => Assert.Equal(("fieldD", IncrementalStepRunReason.Modified), output)), step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldB", IncrementalStepRunReason.Cached), output)), + output => Assert.Equal(("fieldB", IncrementalStepRunReason.Unchanged), output)), step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldC", IncrementalStepRunReason.Cached), output))); + output => Assert.Equal(("fieldC", IncrementalStepRunReason.Unchanged), output))); + Assert.Equal("fieldD", Assert.Single(fieldsCalledFor)); } [Fact] @@ -1330,7 +1325,8 @@ class E } return false; }, - (c, _) => ((FieldDeclarationSyntax)c.Node).Declaration.Variables[0].Identifier.ValueText).WithTrackingName("Fields"); + (c, _) => ((FieldDeclarationSyntax)c.Node).Declaration.Variables[0].Identifier.ValueText).WithTrackingName("Syntax") + .Select((s, ct) => s).WithTrackingName("Fields"); context.RegisterSourceOutput(source, (spc, fieldName) => { @@ -1367,7 +1363,8 @@ class E .ReplaceSyntaxTree(lastTree, firstTree) .ReplaceSyntaxTree(dummyTree, lastTree); - // now re-run the drivers and confirm we didn't actually run + // now re-run the drivers and confirm we didn't actually run the node selector for any nodes. + // Verify that we still ran the transform. syntaxFieldsCalledFor.Clear(); driver = driver.RunGenerators(compilation); results = driver.GetRunResult(); @@ -1376,6 +1373,13 @@ class E Assert.EndsWith("fieldA.cs", results.GeneratedTrees[0].FilePath); Assert.EndsWith("fieldB.cs", results.GeneratedTrees[1].FilePath); Assert.EndsWith("fieldC.cs", results.GeneratedTrees[2].FilePath); + Assert.Collection(results.Results[0].TrackedSteps["Syntax"], + step => Assert.Collection(step.Outputs, + output => Assert.Equal(("fieldA", IncrementalStepRunReason.Unchanged), output)), + step => Assert.Collection(step.Outputs, + output => Assert.Equal(("fieldB", IncrementalStepRunReason.Unchanged), output)), + step => Assert.Collection(step.Outputs, + output => Assert.Equal(("fieldC", IncrementalStepRunReason.Unchanged), output))); Assert.Collection(results.Results[0].TrackedSteps["Fields"], step => Assert.Collection(step.Outputs, output => Assert.Equal(("fieldA", IncrementalStepRunReason.Cached), output)), @@ -1393,8 +1397,8 @@ class E .ReplaceSyntaxTree(lastTree, firstTree) .ReplaceSyntaxTree(dummyTree, newLastTree); - // now re-run the drivers and confirm we only ran for the 'new' syntax tree - // but then stopped when we got the same value out + // now re-run the drivers and confirm we only ran the selector for the 'new' syntax tree + // but then cached the result after the transform when we got the same value out syntaxFieldsCalledFor.Clear(); driver = driver.RunGenerators(compilation); results = driver.GetRunResult(); @@ -1409,9 +1413,8 @@ class E step => Assert.Collection(step.Outputs, output => Assert.Equal(("fieldB", IncrementalStepRunReason.Cached), output)), step => Assert.Collection(step.Outputs, - output => Assert.Equal(("fieldC", IncrementalStepRunReason.Unchanged), output))); - Assert.Single(syntaxFieldsCalledFor); - Assert.Equal("fieldC", syntaxFieldsCalledFor[0]); + output => Assert.Equal(("fieldC", IncrementalStepRunReason.Cached), output))); + Assert.Equal("fieldC", Assert.Single(syntaxFieldsCalledFor)); } [Fact]