Skip to content

Commit a81a6da

Browse files
DustinCampbellDustinCampbell
authored
Compiler: Make intermediate nodes a bit more efficient (#11931)
This pull request focuses on two properties of `IntermediateNode`: Diagnostics and Annotations. * `IntermediateNode.Diagnostics` is constructed most of the time, even when there aren't any diagnostics. I've reworked how diagnostics are stored and collected to avoid creating any storage until a diagnostic is actually added. With this change, the `RazorDiagnosticCollection` type can be removed entirely. * `IntermediateNode.Annotations` works like a property bag that is backed by an `ItemCollection`. This is pretty inefficient. First of all, `ItemCollection` uses a `ConcurrentDictionary` under the hood, which is completely unnecessary in the compiler's synchronous code paths. Plus, all of the data stored in `Annotations` (some of which are effectively bools) can just be stored as properties on the strongly-typed `IntermediateNode` types themselves. I recommend reviewing commit-by-commit, as I tried to keep each separate and cohesive. ---- CI Build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2728082&view=results Test Insertion: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/642908 Toolset Run: https://dev.azure.com/dnceng/internal/_build/results?buildId=2728084&view=results
2 parents 74cb149 + a249d9c commit a81a6da

File tree

63 files changed

+578
-1017
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

63 files changed

+578
-1017
lines changed

‎src/Compiler/Microsoft.AspNetCore.Mvc.Razor.Extensions.Version2_X/test/AssemblyAttributeInjectionPassTest.cs‎

Lines changed: 14 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,7 @@ public void Execute_NoOps_IfNamespaceNodeHasEmptyContent()
3636
var @namespace = new NamespaceDeclarationIntermediateNode()
3737
{
3838
Content = string.Empty,
39-
Annotations =
40-
{
41-
[CommonAnnotations.PrimaryNamespace] = CommonAnnotations.PrimaryNamespace
42-
}
39+
IsPrimaryNamespace = true,
4340
};
4441

4542
builder.Push(@namespace);
@@ -83,20 +80,14 @@ public void Execute_NoOps_IfClassNameIsEmpty()
8380
var @namespace = new NamespaceDeclarationIntermediateNode()
8481
{
8582
Content = "SomeNamespace",
86-
Annotations =
87-
{
88-
[CommonAnnotations.PrimaryNamespace] = CommonAnnotations.PrimaryNamespace
89-
}
83+
IsPrimaryNamespace = true,
9084
};
9185

9286
builder.Push(@namespace);
9387

9488
builder.Add(new ClassDeclarationIntermediateNode
9589
{
96-
Annotations =
97-
{
98-
[CommonAnnotations.PrimaryClass] = CommonAnnotations.PrimaryClass,
99-
},
90+
IsPrimaryClass = true,
10091
});
10192

10293
// Act
@@ -125,10 +116,7 @@ public void Execute_NoOps_IfDocumentIsNotViewOrPage()
125116
var @class = new ClassDeclarationIntermediateNode
126117
{
127118
ClassName = "SomeName",
128-
Annotations =
129-
{
130-
[CommonAnnotations.PrimaryClass] = CommonAnnotations.PrimaryClass,
131-
}
119+
IsPrimaryClass = true,
132120
};
133121

134122
builder.Add(@class);
@@ -158,21 +146,15 @@ public void Execute_NoOps_ForDesignTime()
158146
var @namespace = new NamespaceDeclarationIntermediateNode
159147
{
160148
Content = "SomeNamespace",
161-
Annotations =
162-
{
163-
[CommonAnnotations.PrimaryNamespace] = CommonAnnotations.PrimaryNamespace
164-
}
149+
IsPrimaryNamespace = true,
165150
};
166151

167152
builder.Push(@namespace);
168153

169154
var @class = new ClassDeclarationIntermediateNode
170155
{
171156
ClassName = "SomeName",
172-
Annotations =
173-
{
174-
[CommonAnnotations.PrimaryClass] = CommonAnnotations.PrimaryClass,
175-
}
157+
IsPrimaryClass = true,
176158
};
177159

178160
builder.Add(@class);
@@ -205,20 +187,14 @@ public void Execute_AddsRazorViewAttribute_ToViews()
205187
var @namespace = new NamespaceDeclarationIntermediateNode
206188
{
207189
Content = "SomeNamespace",
208-
Annotations =
209-
{
210-
[CommonAnnotations.PrimaryNamespace] = CommonAnnotations.PrimaryNamespace
211-
}
190+
IsPrimaryNamespace = true,
212191
};
213192

214193
builder.Push(@namespace);
215194
var @class = new ClassDeclarationIntermediateNode
216195
{
217196
ClassName = "SomeName",
218-
Annotations =
219-
{
220-
[CommonAnnotations.PrimaryClass] = CommonAnnotations.PrimaryClass,
221-
}
197+
IsPrimaryClass = true,
222198
};
223199

224200
builder.Add(@class);
@@ -258,21 +234,15 @@ public void Execute_EscapesViewPathWhenAddingAttributeToViews()
258234
var @namespace = new NamespaceDeclarationIntermediateNode
259235
{
260236
Content = "SomeNamespace",
261-
Annotations =
262-
{
263-
[CommonAnnotations.PrimaryNamespace] = CommonAnnotations.PrimaryNamespace
264-
}
237+
IsPrimaryNamespace = true,
265238
};
266239

267240
builder.Push(@namespace);
268241

269242
var @class = new ClassDeclarationIntermediateNode
270243
{
271244
ClassName = "SomeName",
272-
Annotations =
273-
{
274-
[CommonAnnotations.PrimaryClass] = CommonAnnotations.PrimaryClass,
275-
}
245+
IsPrimaryClass = true,
276246
};
277247

278248
builder.Add(@class);
@@ -319,21 +289,15 @@ public void Execute_AddsRazorPagettribute_ToPage()
319289
var @namespace = new NamespaceDeclarationIntermediateNode
320290
{
321291
Content = "SomeNamespace",
322-
Annotations =
323-
{
324-
[CommonAnnotations.PrimaryNamespace] = CommonAnnotations.PrimaryNamespace
325-
}
292+
IsPrimaryNamespace = true,
326293
};
327294

328295
builder.Push(@namespace);
329296

330297
var @class = new ClassDeclarationIntermediateNode
331298
{
332299
ClassName = "SomeName",
333-
Annotations =
334-
{
335-
[CommonAnnotations.PrimaryClass] = CommonAnnotations.PrimaryClass,
336-
}
300+
IsPrimaryClass = true,
337301
};
338302

339303
builder.Add(@class);
@@ -374,20 +338,14 @@ public void Execute_EscapesViewPathAndRouteWhenAddingAttributeToPage()
374338
var @namespace = new NamespaceDeclarationIntermediateNode
375339
{
376340
Content = "SomeNamespace",
377-
Annotations =
378-
{
379-
[CommonAnnotations.PrimaryNamespace] = CommonAnnotations.PrimaryNamespace
380-
}
341+
IsPrimaryNamespace = true,
381342
};
382343
builder.Push(@namespace);
383344

384345
var @class = new ClassDeclarationIntermediateNode
385346
{
386347
ClassName = "SomeName",
387-
Annotations =
388-
{
389-
[CommonAnnotations.PrimaryClass] = CommonAnnotations.PrimaryClass,
390-
}
348+
IsPrimaryClass = true,
391349
};
392350

393351
builder.Add(@class);

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/CodeGeneration/DefaultDocumentWriterTest.cs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,10 +234,7 @@ public void WriteDocument_WithNullableContext_WritesClass()
234234
new TypeParameter() { ParameterName = "TValue", },
235235
},
236236
ClassName = "TestClass",
237-
Annotations =
238-
{
239-
[CommonAnnotations.NullableContext] = CommonAnnotations.NullableContext,
240-
},
237+
NullableContext = true,
241238
});
242239

243240
var codeDocument = TestRazorCodeDocument.CreateEmpty();

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/Components/ComponentMarkupEncodingPassTest.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public void Execute_MixedHtmlContent_NoNewLineorSpecialCharacters_DoesNotSetEnco
7575
// Assert
7676
var node = documentNode.FindDescendantNodes<HtmlContentIntermediateNode>().Single();
7777
Assert.Equal(expected, GetHtmlContent(node));
78-
Assert.False(node.IsEncoded());
78+
Assert.False(node.HasEncodedContent);
7979
}
8080

8181
[Fact]
@@ -96,7 +96,7 @@ public void Execute_MixedHtmlContent_NewLine_SetsEncoded()
9696
// Assert
9797
var node = documentNode.FindDescendantNodes<HtmlContentIntermediateNode>().Single();
9898
Assert.Equal(expected, GetHtmlContent(node));
99-
Assert.True(node.IsEncoded());
99+
Assert.True(node.HasEncodedContent);
100100
}
101101

102102
[Fact]
@@ -115,7 +115,7 @@ public void Execute_MixedHtmlContent_Ampersand_SetsEncoded()
115115
// Assert
116116
var node = documentNode.FindDescendantNodes<HtmlContentIntermediateNode>().Single();
117117
Assert.Equal(expected, GetHtmlContent(node));
118-
Assert.True(node.IsEncoded());
118+
Assert.True(node.HasEncodedContent);
119119
}
120120

121121
[Fact]
@@ -134,7 +134,7 @@ public void Execute_MixedHtmlContent_NonAsciiCharacter_SetsEncoded()
134134
// Assert
135135
var node = documentNode.FindDescendantNodes<HtmlContentIntermediateNode>().Single();
136136
Assert.Equal(expected, GetHtmlContent(node));
137-
Assert.True(node.IsEncoded());
137+
Assert.True(node.HasEncodedContent);
138138
}
139139

140140
[Fact]
@@ -153,7 +153,7 @@ public void Execute_MixedHtmlContent_HTMLEntity_DoesNotSetEncoded()
153153
// Assert
154154
var node = documentNode.FindDescendantNodes<HtmlContentIntermediateNode>().Single();
155155
Assert.Equal(expected, GetHtmlContent(node));
156-
Assert.False(node.IsEncoded());
156+
Assert.False(node.HasEncodedContent);
157157
}
158158

159159
[Fact]
@@ -172,7 +172,7 @@ public void Execute_MixedHtmlContent_MultipleHTMLEntities_DoesNotSetEncoded()
172172
// Assert
173173
var node = documentNode.FindDescendantNodes<HtmlContentIntermediateNode>().Single();
174174
Assert.Equal(expected, GetHtmlContent(node));
175-
Assert.False(node.IsEncoded());
175+
Assert.False(node.HasEncodedContent);
176176
}
177177

178178
private string NormalizeContent(string content)

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultRazorCSharpLoweringPhaseTest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public void Execute_CollatesIRDocumentDiagnosticsFromSourceDocument()
7676
var expectedDiagnostic = RazorDiagnostic.Create(
7777
new RazorDiagnosticDescriptor("1234", "I am an error.", RazorDiagnosticSeverity.Error),
7878
new SourceSpan("SomeFile.cshtml", 11, 0, 11, 1));
79-
irDocument.Diagnostics.Add(expectedDiagnostic);
79+
irDocument.AddDiagnostic(expectedDiagnostic);
8080
codeDocument.SetDocumentIntermediateNode(irDocument);
8181

8282
// Act

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DirectiveRemovalOptimizationPassTest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public void Execute_DirectiveWithError_PreservesDiagnosticsAndRemovesDirectiveNo
9393

9494
// Add the diagnostic to the directive node.
9595
var directiveNode = documentNode.FindDescendantNodes<DirectiveIntermediateNode>().Single();
96-
directiveNode.Diagnostics.Add(expectedDiagnostic);
96+
directiveNode.AddDiagnostic(expectedDiagnostic);
9797

9898
// Act
9999
processor.ExecutePass<DirectiveRemovalOptimizationPass>();

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DocumentClassifierPassBaseTest.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,13 +228,13 @@ public void Execute_AddsPrimaryAnnotations()
228228

229229
// Assert
230230
var @namespace = SingleChild<NamespaceDeclarationIntermediateNode>(documentNode);
231-
AnnotationEquals(@namespace, CommonAnnotations.PrimaryNamespace);
231+
Assert.True(@namespace.IsPrimaryNamespace);
232232

233233
var @class = SingleChild<ClassDeclarationIntermediateNode>(@namespace);
234-
AnnotationEquals(@class, CommonAnnotations.PrimaryClass);
234+
Assert.True(@class.IsPrimaryClass);
235235

236236
var method = SingleChild<MethodDeclarationIntermediateNode>(@class);
237-
AnnotationEquals(method, CommonAnnotations.PrimaryMethod);
237+
Assert.True(method.IsPrimaryMethod);
238238
}
239239

240240
private class TestDocumentClassifierPass : DocumentClassifierPassBase

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/Extensions/DefaultTagHelperOptimizationPassTest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public void DefaultTagHelperOptimizationPass_Execute_ReplacesChildren()
4747
Assert.IsType<DefaultTagHelperRuntimeIntermediateNode>(@class.Children[0]);
4848

4949
var fieldDeclaration = Assert.IsType<FieldDeclarationIntermediateNode>(@class.Children[1]);
50-
Assert.Equal(bool.TrueString, fieldDeclaration.Annotations[CommonAnnotations.DefaultTagHelperExtension.TagHelperField]);
50+
Assert.True(fieldDeclaration.IsTagHelperField);
5151
Assert.Equal("__TestTagHelper", fieldDeclaration.FieldName);
5252
Assert.Equal("global::TestTagHelper", fieldDeclaration.FieldType);
5353
Assert.Equal("private", fieldDeclaration.Modifiers.First());

0 commit comments

Comments
 (0)