Skip to content

Commit 3484948

Browse files
authored
ChildSyntaxList.ItemInternal optimization (#11926)
This is a port of dotnet/roslyn#73650 to Razor The general idea behind this PR is that calls to getting items in a syntax list are typically done sequentially, and thus walking the full children of a node is often not necessary. See the Roslyn PR for detailed performance analysis that was done upon the roslyn investigation. In the roslyn speedometer test profile that I'm looking at, 0.8% of CPU is spent in ChildSyntaxList.ItemInternal (1.8 seconds of CPU), so this optimization is worth bringing over from Roslyn. ![image](https://github.com/user-attachments/assets/3dd1069a-b093-412e-9b73-7133d8c2a5ee)
2 parents b47f0f9 + 14e4f07 commit 3484948

File tree

2 files changed

+74
-11
lines changed

2 files changed

+74
-11
lines changed

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Syntax/ChildSyntaxList.Enumerator.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@ public struct Enumerator
1616
private SyntaxNode _node;
1717
private int _count;
1818
private int _childIndex;
19+
private SlotData _slotData;
1920

2021
internal Enumerator(SyntaxNode node, int count)
2122
{
2223
_node = node;
2324
_count = count;
2425
_childIndex = -1;
26+
_slotData = new SlotData(node);
2527
}
2628

2729
// PERF: Initialize an Enumerator directly from a SyntaxNode without going
@@ -31,6 +33,7 @@ internal void InitializeFrom(SyntaxNode node)
3133
_node = node;
3234
_count = CountNodes(node.Green);
3335
_childIndex = -1;
36+
_slotData = new SlotData(node);
3437
}
3538

3639
/// <summary>
@@ -58,8 +61,8 @@ public bool MoveNext()
5861
/// <returns>
5962
/// The element in the <see cref="ChildSyntaxList" /> at the current position of the enumerator.
6063
/// </returns>
61-
public readonly SyntaxNodeOrToken Current
62-
=> ItemInternal(_node, _childIndex);
64+
public SyntaxNodeOrToken Current
65+
=> ItemInternal(_node, _childIndex, ref _slotData);
6366

6467
/// <summary>
6568
/// Sets the enumerator to its initial position, which is before the first element in the collection.
@@ -77,15 +80,15 @@ internal bool TryMoveNextAndGetCurrent(out SyntaxNodeOrToken current)
7780
return false;
7881
}
7982

80-
current = ItemInternal(_node, _childIndex);
83+
current = ItemInternal(_node, _childIndex, ref _slotData);
8184
return true;
8285
}
8386

8487
internal SyntaxNode? TryMoveNextAndGetCurrentAsNode()
8588
{
8689
while (MoveNext())
8790
{
88-
var nodeValue = ItemInternalAsNode(_node, _childIndex);
91+
var nodeValue = ItemInternalAsNode(_node, _childIndex, ref _slotData);
8992
if (nodeValue != null)
9093
{
9194
return nodeValue;

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

Lines changed: 67 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,17 +75,60 @@ private static int Occupancy(GreenNode green)
7575
return green.IsList ? green.SlotCount : 1;
7676
}
7777

78+
internal readonly struct SlotData
79+
{
80+
/// <summary>
81+
/// The green node slot index at which to start the search
82+
/// </summary>
83+
public readonly int SlotIndex;
84+
85+
/// <summary>
86+
/// Indicates the total number of occupants in preceding slots
87+
/// </summary>
88+
public readonly int PrecedingOccupantSlotCount;
89+
90+
/// <summary>
91+
/// Indicates the node start position plus any prior slot full widths
92+
/// </summary>
93+
public readonly int PositionAtSlotIndex;
94+
95+
public SlotData(SyntaxNode node)
96+
: this(slotIndex: 0, precedingOccupantSlotCount: 0, node.Position)
97+
{
98+
}
99+
100+
public SlotData(int slotIndex, int precedingOccupantSlotCount, int positionAtSlotIndex)
101+
{
102+
SlotIndex = slotIndex;
103+
PrecedingOccupantSlotCount = precedingOccupantSlotCount;
104+
PositionAtSlotIndex = positionAtSlotIndex;
105+
}
106+
}
107+
108+
internal static SyntaxNodeOrToken ItemInternal(SyntaxNode node, int index)
109+
{
110+
var slotData = new SlotData(node);
111+
112+
return ItemInternal(node, index, ref slotData);
113+
}
114+
78115
/// <summary>
79116
/// An internal indexer that does not verify index.
80117
/// Used when caller has already ensured that index is within bounds.
81118
/// </summary>
82-
internal static SyntaxNodeOrToken ItemInternal(SyntaxNode node, int index)
119+
internal static SyntaxNodeOrToken ItemInternal(SyntaxNode node, int index, ref SlotData slotData)
83120
{
84121
GreenNode greenChild;
85122
var green = node.Green;
86-
var idx = index;
87-
var slotIndex = 0;
88-
var position = node.Position;
123+
124+
// slotData may contain information that allows us to start the loop below using data
125+
// calculated during a previous call. As index represents the offset into all children of
126+
// node, idx represents the offset requested relative to the given slot index.
127+
var idx = index - slotData.PrecedingOccupantSlotCount;
128+
var slotIndex = slotData.SlotIndex;
129+
var position = slotData.PositionAtSlotIndex;
130+
131+
Debug.Assert(idx >= 0);
89132

90133
// find a slot that contains the node or its parent list (if node is in a list)
91134
// we will be skipping whole slots here so we will not loop for long
@@ -112,6 +155,12 @@ internal static SyntaxNodeOrToken ItemInternal(SyntaxNode node, int index)
112155
slotIndex++;
113156
}
114157

158+
if (slotIndex != slotData.SlotIndex)
159+
{
160+
// (index - idx) represents the number of occupants prior to this new slotIndex
161+
slotData = new SlotData(slotIndex, index - idx, position);
162+
}
163+
115164
// get node that represents this slot
116165
var red = node.GetNodeSlot(slotIndex);
117166
if (!greenChild.IsList)
@@ -132,6 +181,7 @@ internal static SyntaxNodeOrToken ItemInternal(SyntaxNode node, int index)
132181
// this is our node
133182
return redChild;
134183
}
184+
135185
// must be a separator
136186
// update greenChild and position and let it be handled as a token
137187
greenChild = greenChild.GetSlot(idx);
@@ -237,12 +287,15 @@ internal static SyntaxNodeOrToken ChildThatContainsPosition(SyntaxNode node, int
237287
/// An internal indexer that does not verify index.
238288
/// Used when caller has already ensured that index is within bounds.
239289
/// </summary>
240-
internal static SyntaxNode? ItemInternalAsNode(SyntaxNode node, int index)
290+
internal static SyntaxNode? ItemInternalAsNode(SyntaxNode node, int index, ref SlotData slotData)
241291
{
242292
GreenNode greenChild;
243293
var green = node.Green;
244-
var idx = index;
245-
var slotIndex = 0;
294+
var idx = index - slotData.PrecedingOccupantSlotCount;
295+
var slotIndex = slotData.SlotIndex;
296+
var position = slotData.PositionAtSlotIndex;
297+
298+
Debug.Assert(idx >= 0);
246299

247300
// find a slot that contains the node or its parent list (if node is in a list)
248301
// we will be skipping whole slots here so we will not loop for long
@@ -262,11 +315,18 @@ internal static SyntaxNodeOrToken ChildThatContainsPosition(SyntaxNode node, int
262315
}
263316

264317
idx -= currentOccupancy;
318+
position += greenChild.Width;
265319
}
266320

267321
slotIndex++;
268322
}
269323

324+
if (slotIndex != slotData.SlotIndex)
325+
{
326+
// (index - idx) represents the number of occupants prior to this new slotIndex
327+
slotData = new SlotData(slotIndex, index - idx, position);
328+
}
329+
270330
// get node that represents this slot
271331
var red = node.GetNodeSlot(slotIndex);
272332
if (greenChild.IsList && red != null)

0 commit comments

Comments
 (0)