Skip to content

Commit f091572

Browse files
CR feedback for GreenNode.ToString and GetDebuggerDisplay
1. GetDebuggerDisplay should just return an interpolated string. (Oops!) 2. ToString should implement two specific optimizations to avoid allocating a new string unless necessary. - If the width is zero, return string.Empty. - If there is just a single non-zero-width token with the same width as the node, just return that token's content.
1 parent f5e8bf0 commit f091572

File tree

2 files changed

+313
-9
lines changed
  • src/Compiler

2 files changed

+313
-9
lines changed

src/Compiler/Microsoft.AspNetCore.Razor.Language/test/Syntax/GreenNodeTests.cs

Lines changed: 284 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -934,4 +934,288 @@ public void ToString_WidthMatchesStringLength()
934934
Assert.Equal("Hello World!", result);
935935
Assert.Equal(result.Length, root.Width);
936936
}
937+
938+
[Fact]
939+
public void ToString_ZeroWidth_ReturnsEmptyString()
940+
{
941+
// This test verifies the first optimization: if _width == 0, return string.Empty
942+
943+
// Arrange - Create a node with zero width (empty token)
944+
var token = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, "");
945+
946+
// Act
947+
var result = token.ToString();
948+
949+
// Assert
950+
Assert.Equal(string.Empty, result);
951+
Assert.Same(string.Empty, result); // Verify it's the same instance, not a new allocation
952+
Assert.Equal(0, token.Width);
953+
}
954+
955+
[Fact]
956+
public void ToString_SingleTokenOptimization_ReturnsSameTokenContent()
957+
{
958+
// This test verifies the second optimization: single token returns token.Content directly
959+
960+
// Arrange - Create a node with exactly one token
961+
var tokenContent = "Hello World";
962+
var token = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, tokenContent);
963+
var node = InternalSyntax.SyntaxFactory.MarkupTextLiteral(token);
964+
965+
// Act
966+
var result = node.ToString();
967+
968+
// Assert
969+
Assert.Equal(tokenContent, result);
970+
// Verify it's the same string instance (optimization check)
971+
Assert.Same(tokenContent, result);
972+
}
973+
974+
[Fact]
975+
public void ToString_SingleTokenDirectCall_ReturnsSameTokenContent()
976+
{
977+
// Test the optimization when calling ToString() directly on a token
978+
979+
// Arrange
980+
var tokenContent = "Direct token call";
981+
var token = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, tokenContent);
982+
983+
// Act
984+
var result = token.ToString();
985+
986+
// Assert
987+
Assert.Equal(tokenContent, result);
988+
Assert.Same(tokenContent, result); // Should be the same instance
989+
}
990+
991+
[Fact]
992+
public void ToString_MultipleTokens_AllocatesNewString()
993+
{
994+
// This test verifies that when there are multiple tokens, a new string is allocated
995+
996+
// Arrange - Create a node with multiple tokens
997+
var token1Content = "Hello";
998+
var token2Content = " World";
999+
1000+
var token1 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, token1Content);
1001+
var child1 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(token1);
1002+
1003+
var token2 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, token2Content);
1004+
var child2 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(token2);
1005+
1006+
var root = InternalSyntax.SyntaxFactory.GenericBlock([child1, child2]);
1007+
1008+
// Act
1009+
var result = root.ToString();
1010+
1011+
// Assert
1012+
Assert.Equal("Hello World", result);
1013+
// Verify it's NOT the same instance as either token (new allocation)
1014+
Assert.NotSame(token1Content, result);
1015+
Assert.NotSame(token2Content, result);
1016+
}
1017+
1018+
[Fact]
1019+
public void ToString_EmptyTokensInComplexTree_ReturnsEmptyString()
1020+
{
1021+
// Test zero width optimization with a more complex tree structure
1022+
1023+
// Arrange - Create a tree with only empty tokens
1024+
var emptyToken1 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, "");
1025+
var child1 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(emptyToken1);
1026+
1027+
var emptyToken2 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Whitespace, "");
1028+
var child2 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(emptyToken2);
1029+
1030+
var root = InternalSyntax.SyntaxFactory.GenericBlock([child1, child2]);
1031+
1032+
// Act
1033+
var result = root.ToString();
1034+
1035+
// Assert
1036+
Assert.Equal(string.Empty, result);
1037+
Assert.Same(string.Empty, result);
1038+
Assert.Equal(0, root.Width);
1039+
}
1040+
1041+
[Fact]
1042+
public void ToString_SingleEmptyTokenInNode_ReturnsEmptyString()
1043+
{
1044+
// Test single token optimization when that token is empty
1045+
1046+
// Arrange
1047+
var emptyToken = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, "");
1048+
var node = InternalSyntax.SyntaxFactory.MarkupTextLiteral(emptyToken);
1049+
1050+
// Act
1051+
var result = node.ToString();
1052+
1053+
// Assert
1054+
Assert.Equal(string.Empty, result);
1055+
Assert.Same(string.Empty, result); // Should return string.Empty directly
1056+
}
1057+
1058+
[Fact]
1059+
public void ToString_SingleTokenWithLongContent_ReturnsTokenContent()
1060+
{
1061+
// Test single token optimization with longer content
1062+
1063+
// Arrange
1064+
var longContent = "This is a much longer piece of content that would normally require allocation but should be optimized";
1065+
var token = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, longContent);
1066+
var node = InternalSyntax.SyntaxFactory.MarkupTextLiteral(token);
1067+
1068+
// Act
1069+
var result = node.ToString();
1070+
1071+
// Assert
1072+
Assert.Equal(longContent, result);
1073+
Assert.Same(longContent, result); // Should be the same instance
1074+
}
1075+
1076+
[Fact]
1077+
public void ToString_OptimizationPathVsAllocationPath_ProduceSameResult()
1078+
{
1079+
// Verify that both code paths produce the same result
1080+
1081+
// Arrange - Single token (optimization path)
1082+
var content = "Test Content";
1083+
var singleToken = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, content);
1084+
var singleNode = InternalSyntax.SyntaxFactory.MarkupTextLiteral(singleToken);
1085+
1086+
// Arrange - Multiple tokens that concatenate to same content (allocation path)
1087+
var token1 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, "Test");
1088+
var child1 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(token1);
1089+
1090+
var token2 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, " ");
1091+
var child2 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(token2);
1092+
1093+
var token3 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, "Content");
1094+
var child3 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(token3);
1095+
1096+
var multiNode = InternalSyntax.SyntaxFactory.GenericBlock([child1, child2, child3]);
1097+
1098+
// Act
1099+
var singleResult = singleNode.ToString();
1100+
var multiResult = multiNode.ToString();
1101+
1102+
// Assert
1103+
Assert.Equal(singleResult, multiResult);
1104+
Assert.Same(content, singleResult); // Single token should be optimized
1105+
Assert.NotSame(content, multiResult); // Multiple tokens should allocate new string
1106+
}
1107+
1108+
[Fact]
1109+
public void ToString_SingleTokenOptimization_SkipsZeroWidthTokens()
1110+
{
1111+
// This test verifies the optimization when there are zero-width tokens followed by a single non-zero-width token
1112+
// The algorithm should skip the zero-width tokens and use the single token optimization for the non-zero-width token
1113+
1114+
// Arrange - Create a tree with zero-width tokens followed by one non-zero-width token
1115+
var zeroWidthToken1 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, "");
1116+
var child1 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(zeroWidthToken1);
1117+
1118+
var zeroWidthToken2 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Whitespace, "");
1119+
var child2 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(zeroWidthToken2);
1120+
1121+
var nonZeroTokenContent = "ActualContent";
1122+
var nonZeroToken = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, nonZeroTokenContent);
1123+
var child3 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(nonZeroToken);
1124+
1125+
var root = InternalSyntax.SyntaxFactory.GenericBlock([child1, child2, child3]);
1126+
1127+
// Act
1128+
var result = root.ToString();
1129+
1130+
// Assert
1131+
Assert.Equal(nonZeroTokenContent, result);
1132+
// Verify it uses the single token optimization (same string instance)
1133+
Assert.Same(nonZeroTokenContent, result);
1134+
Assert.Equal(nonZeroTokenContent.Length, root.Width);
1135+
}
1136+
1137+
[Fact]
1138+
public void ToString_SingleTokenOptimization_WithMultipleZeroWidthTokensAndOneNonZero()
1139+
{
1140+
// Test edge case with many zero-width tokens and one content token
1141+
1142+
// Arrange - Create multiple zero-width tokens followed by one with content
1143+
var emptyToken1 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, "");
1144+
var child1 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(emptyToken1);
1145+
1146+
var emptyToken2 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Whitespace, "");
1147+
var child2 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(emptyToken2);
1148+
1149+
var emptyToken3 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, "");
1150+
var child3 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(emptyToken3);
1151+
1152+
var contentTokenValue = "OnlyRealToken";
1153+
var contentToken = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, contentTokenValue);
1154+
var child4 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(contentToken);
1155+
1156+
var root = InternalSyntax.SyntaxFactory.GenericBlock([child1, child2, child3, child4]);
1157+
1158+
// Act
1159+
var result = root.ToString();
1160+
1161+
// Assert
1162+
Assert.Equal(contentTokenValue, result);
1163+
Assert.Same(contentTokenValue, result); // Should use single token optimization
1164+
}
1165+
1166+
[Fact]
1167+
public void ToString_NoOptimization_WithZeroWidthTokenBetweenNonZeroTokens()
1168+
{
1169+
// Test that optimization is NOT used when there are multiple non-zero-width tokens,
1170+
// even if separated by zero-width tokens
1171+
1172+
// Arrange
1173+
var token1Content = "First";
1174+
var token1 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, token1Content);
1175+
var child1 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(token1);
1176+
1177+
var zeroWidthToken = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, "");
1178+
var child2 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(zeroWidthToken);
1179+
1180+
var token3Content = "Second";
1181+
var token3 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, token3Content);
1182+
var child3 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(token3);
1183+
1184+
var root = InternalSyntax.SyntaxFactory.GenericBlock([child1, child2, child3]);
1185+
1186+
// Act
1187+
var result = root.ToString();
1188+
1189+
// Assert
1190+
Assert.Equal("FirstSecond", result);
1191+
// Should NOT use optimization (new string allocation)
1192+
Assert.NotSame(token1Content, result);
1193+
Assert.NotSame(token3Content, result);
1194+
}
1195+
1196+
[Fact]
1197+
public void ToString_SingleTokenOptimization_WithTrailingZeroWidthTokens()
1198+
{
1199+
// Test optimization when non-zero-width token is followed by zero-width tokens
1200+
1201+
// Arrange
1202+
var contentTokenValue = "MainContent";
1203+
var contentToken = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, contentTokenValue);
1204+
var child1 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(contentToken);
1205+
1206+
var emptyToken1 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Text, "");
1207+
var child2 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(emptyToken1);
1208+
1209+
var emptyToken2 = InternalSyntax.SyntaxFactory.Token(SyntaxKind.Whitespace, "");
1210+
var child3 = InternalSyntax.SyntaxFactory.MarkupTextLiteral(emptyToken2);
1211+
1212+
var root = InternalSyntax.SyntaxFactory.GenericBlock([child1, child2, child3]);
1213+
1214+
// Act
1215+
var result = root.ToString();
1216+
1217+
// Assert
1218+
Assert.Equal(contentTokenValue, result);
1219+
Assert.Same(contentTokenValue, result); // Should use single token optimization
1220+
}
9371221
}

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Syntax/GreenNode.cs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -212,18 +212,38 @@ internal SyntaxAnnotation[] GetAnnotations()
212212

213213
#region Text
214214
private string GetDebuggerDisplay()
215-
{
216-
return string.Build(this, static (ref builder, node) =>
217-
{
218-
builder.Append(node.GetType().Name);
219-
builder.Append("<");
220-
builder.Append(node.Kind.ToString());
221-
builder.Append(">");
222-
});
223-
}
215+
=> $"{GetType().Name}<{Kind}>";
224216

225217
public override string ToString()
226218
{
219+
// Simple case: Zero width is just an empty string.
220+
if (_width == 0)
221+
{
222+
return string.Empty;
223+
}
224+
225+
// Special case: See if there's just a single non-zero-width descendant token.
226+
// If so, we can just return the content of that token rather than creating a new string for it.
227+
foreach (var token in Tokens())
228+
{
229+
if (token.Width == _width)
230+
{
231+
// If this token has the same width as this node, just return it's content.
232+
return token.Content;
233+
}
234+
235+
// If this is a zero-width token, skip it.
236+
if (token.Width == 0)
237+
{
238+
continue;
239+
}
240+
241+
// Otherwise, this is a non-zero-width token but there much be more tokens that make up the
242+
// total width. Break out of the loop to allocate a new string with all of the content.
243+
break;
244+
}
245+
246+
// At this point, we know that we have multiple tokens and need to allocate a string.
227247
return string.Create(length: _width, this, static (span, node) =>
228248
{
229249
foreach (var token in node.Tokens())

0 commit comments

Comments
 (0)