Browse Source

Enumerate AST children with a zero-allocation struct enumerator

The generated GetChildNodes materialized a List<AstNode> (plus a boxed
List.Enumerator at every foreach) for each node, so AstNode.Children and
the visitor's per-node child walk allocated three objects per traversal.
Decompiling System.Private.CoreLib that came to ~1.7 GB of extra garbage,
roughly +7% over the linked-list model the slot tree replaced. A yield
iterator removes the List but trades it for an equally costly per-node
state machine, so it is not enough on its own.

Enumerate children through a by-value struct enumerator over the existing
FirstChild/NextSibling primitives, capturing each child's successor before
it is yielded so a transform may still remove or replace the current child
mid-traversal. AstNodeCollection<T> gets the same struct treatment for a
direct foreach. Child enumeration now allocates nothing, bringing total
allocations back to the linked-list baseline at byte-identical output
(full Pretty suite green with CheckInvariant active).

Assisted-by: Claude:claude-opus-4-8:Claude Code
pull/3807/head
Siegfried Pammer 2 weeks ago committed by Siegfried Pammer
parent
commit
721ef6032d
  1. 19
      ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs
  2. 1
      ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj
  3. 120
      ICSharpCode.Decompiler.Tests/Syntax/AstNodeCollectionEnumeratorTests.cs
  4. 90
      ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs
  5. 103
      ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs
  6. 18
      ICSharpCode.Decompiler/CSharp/Syntax/DepthFirstAstVisitor.cs

19
ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs

@ -650,25 +650,6 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator
} }
builder.AppendLine("\t}"); builder.AppendLine("\t}");
builder.AppendLine(); builder.AppendLine();
// Single-pass enumeration of children in slot order (O(children) total) for AstNode.Children
// and the visitors' VisitChildren, instead of the per-step O(slots) index scan (NextSibling).
// Collection slots iterate via their own enumerator, which tolerates removing/replacing the
// current child during traversal.
builder.AppendLine("\tinternal override IEnumerable<AstNode> GetChildNodes()");
builder.AppendLine("\t{");
builder.AppendLine("\t\tvar children = new List<AstNode>(GetChildCount());");
foreach (var s in slots)
{
string field = FieldName(s.PropertyName);
if (s.IsCollection)
builder.AppendLine($"\t\tif ({field} != null) children.AddRange({field});");
else
builder.AppendLine($"\t\tif ({field} != null) children.Add({field});");
}
builder.AppendLine("\t\treturn children;");
builder.AppendLine("\t}");
builder.AppendLine();
} }
// Close the class, trimming the blank line the per-member spacer leaves before the brace. // Close the class, trimming the blank line the per-member spacer leaves before the brace.

1
ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj

@ -333,6 +333,7 @@
<Compile Include="TestCases\Pretty\VariableNamingWithoutSymbols.cs" /> <Compile Include="TestCases\Pretty\VariableNamingWithoutSymbols.cs" />
<Compile Include="PdbGenerationTestRunner.cs" /> <Compile Include="PdbGenerationTestRunner.cs" />
<Compile Include="PatternMatching\PatternMatchingTests.cs" /> <Compile Include="PatternMatching\PatternMatchingTests.cs" />
<Compile Include="Syntax\AstNodeCollectionEnumeratorTests.cs" />
<None Include="TestCases\ILPretty\Issue1047.il" /> <None Include="TestCases\ILPretty\Issue1047.il" />
<None Include="TestCases\ILPretty\Issue959.cs" /> <None Include="TestCases\ILPretty\Issue959.cs" />
<None Include="TestCases\ILPretty\FSharpLoops_Debug.cs" /> <None Include="TestCases\ILPretty\FSharpLoops_Debug.cs" />

120
ICSharpCode.Decompiler.Tests/Syntax/AstNodeCollectionEnumeratorTests.cs

@ -0,0 +1,120 @@
// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team
//
// Permission is hereby granted, free of charge, to any person obtaining a copy of this
// software and associated documentation files (the "Software"), to deal in the Software
// without restriction, including without limitation the rights to use, copy, modify, merge,
// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
// to whom the Software is furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in all copies or
// substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.
using System.Collections.Generic;
using System.Linq;
using ICSharpCode.Decompiler.CSharp.Syntax;
using NUnit.Framework;
namespace ICSharpCode.Decompiler.Tests.Syntax
{
/// <summary>
/// Pins the mutation-during-enumeration contract of <see cref="AstNodeCollection{T}.Enumerator"/>:
/// the cursor follows node identity hand-over-hand (like the old linked-list enumerator), so a transform
/// may remove or replace the current element mid-foreach, and inserting or removing other elements only
/// shifts the backing-list indices without skipping or re-yielding a surviving element. Each case also
/// exercises the enumerator's DEBUG position guard, which asserts when the cursor mislocates.
/// </summary>
[TestFixture]
public class AstNodeCollectionEnumeratorTests
{
static ExpressionStatement Stmt(string name) => new ExpressionStatement(new IdentifierExpression(name));
static string NameOf(Statement statement) => ((IdentifierExpression)((ExpressionStatement)statement).Expression).Identifier;
// Walks the collection, invoking 'mutate' once when the named statement is the current element, and
// returns the names yielded in order. The foreach uses the struct enumerator under test.
static List<string> EnumerateWith(BlockStatement block, string mutateAt, System.Action<BlockStatement, Statement> mutate)
{
var visited = new List<string>();
foreach (Statement statement in block.Statements)
{
visited.Add(NameOf(statement));
if (mutate != null && NameOf(statement) == mutateAt)
mutate(block, statement);
}
return visited;
}
static BlockStatement Block(params string[] names)
{
var block = new BlockStatement();
foreach (string name in names)
block.Statements.Add(Stmt(name));
return block;
}
[Test]
public void PlainEnumeration_YieldsAllInOrder()
{
var block = Block("a", "b", "c", "d");
Assert.That(EnumerateWith(block, null, null), Is.EqualTo(new[] { "a", "b", "c", "d" }));
}
[Test]
public void RemovingCurrent_DoesNotSkipTheSuccessor()
{
var block = Block("a", "b", "c", "d");
var visited = EnumerateWith(block, "a", (b, current) => current.Remove());
Assert.That(visited, Is.EqualTo(new[] { "a", "b", "c", "d" }));
Assert.That(block.Statements.Select(NameOf), Is.EqualTo(new[] { "b", "c", "d" }));
}
[Test]
public void ReplacingCurrent_AdvancesPastTheReplacement()
{
var block = Block("a", "b", "c", "d");
var visited = EnumerateWith(block, "b", (b, current) => current.ReplaceWith(Stmt("b2")));
// The replacement takes the current slot but is not re-visited.
Assert.That(visited, Is.EqualTo(new[] { "a", "b", "c", "d" }));
Assert.That(block.Statements.Select(NameOf), Is.EqualTo(new[] { "a", "b2", "c", "d" }));
}
[Test]
public void RemovingAnAlreadyVisitedElement_DoesNotDisturbTheRemainder()
{
var block = Block("a", "b", "c", "d");
// Removing 'a' while 'c' is current shifts 'd' down one index; the identity resume absorbs it.
var visited = EnumerateWith(block, "c", (b, current) => b.Statements.First(s => NameOf(s) == "a").Remove());
Assert.That(visited, Is.EqualTo(new[] { "a", "b", "c", "d" }));
}
[Test]
public void InsertingBeforeTheCursor_DoesNotSkipSurvivingElements()
{
var block = Block("a", "b", "c", "d");
// Inserting 'x' before the current 'b' shifts every later index up one; the cursor still resumes
// on its captured successor 'c'. The freshly inserted node is not visited (it is behind the cursor).
var visited = EnumerateWith(block, "b", (b, current) => b.Statements.InsertBefore((Statement)current, Stmt("x")));
Assert.That(visited, Is.EqualTo(new[] { "a", "b", "c", "d" }));
Assert.That(block.Statements.Select(NameOf), Is.EqualTo(new[] { "a", "x", "b", "c", "d" }));
}
[Test]
public void RemovingTheCapturedSuccessor_StopsEnumeration()
{
var block = Block("a", "b", "c", "d");
// 'c' is the successor captured when 'b' is yielded; removing it leaves the cursor with no node to
// resume on, so enumeration stops -- the one mutation the identity walk cannot follow.
var visited = EnumerateWith(block, "b", (b, current) => b.Statements.First(s => NameOf(s) == "c").Remove());
Assert.That(visited, Is.EqualTo(new[] { "a", "b" }));
}
}
}

90
ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs

@ -26,6 +26,7 @@
// THE SOFTWARE. // THE SOFTWARE.
using System; using System;
using System.Collections;
using System.Collections.Generic; using System.Collections.Generic;
using System.Diagnostics; using System.Diagnostics;
using System.IO; using System.IO;
@ -208,12 +209,91 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
} }
} }
public IEnumerable<AstNode> Children => GetChildNodes(); /// <summary>
/// The children of this node in document order. The collection and its enumerator are structs, so
/// a <c>foreach</c> over <see cref="Children"/> allocates nothing; only enumeration through the
/// <see cref="IEnumerable{T}"/> interface (e.g. LINQ) boxes the enumerator. The enumerator captures
/// the successor before handing out each child, so the loop body may remove or replace the current
/// child mid-traversal (the behavior the visitor and transforms rely on).
/// </summary>
public ChildrenCollection Children => new ChildrenCollection(this);
public readonly struct ChildrenCollection : IReadOnlyList<AstNode>
{
readonly AstNode node;
internal ChildrenCollection(AstNode node) => this.node = node;
public ChildEnumerator GetEnumerator() => new ChildEnumerator(node);
IEnumerator<AstNode> IEnumerable<AstNode>.GetEnumerator() => new ChildEnumerator(node);
IEnumerator IEnumerable.GetEnumerator() => new ChildEnumerator(node);
public int Count {
get {
int count = 0;
for (ChildEnumerator e = GetEnumerator(); e.MoveNext();)
count++;
return count;
}
}
public AstNode this[int index] {
get {
int i = 0;
foreach (AstNode child in this)
{
if (i++ == index)
return child;
}
throw new ArgumentOutOfRangeException(nameof(index));
}
}
}
/// <summary>
/// Zero-allocation enumerator over a node's children. It captures each child's successor before
/// the child is handed out, so removing or replacing the current child in the loop body does not
/// disturb the traversal -- the same guarantee the old linked-list enumerator gave.
/// </summary>
public struct ChildEnumerator : IEnumerator<AstNode>
{
readonly AstNode node;
AstNode? current;
AstNode? next;
bool started;
internal ChildEnumerator(AstNode node)
{
this.node = node;
this.current = null;
this.next = null;
this.started = false;
}
// Enumerates the children in slot order in a single pass (O(children)). The generator overrides this public readonly AstNode Current => current!;
// per node; the base (a leaf node with no slots) has none. Collection slots iterate via their own readonly object IEnumerator.Current => current!;
// enumerator, which tolerates removing/replacing the current child mid-traversal.
internal virtual IEnumerable<AstNode> GetChildNodes() => System.Linq.Enumerable.Empty<AstNode>(); public bool MoveNext()
{
current = started ? next : node.FirstChild;
started = true;
if (current == null)
return false;
// Remember the successor before the child is yielded, so the loop body may remove or
// replace 'current' without losing our place.
next = current.NextSibling;
return true;
}
public void Reset()
{
current = null;
next = null;
started = false;
}
public readonly void Dispose() { }
}
/// <summary> /// <summary>
/// Gets the ancestors of this node (excluding this node itself) /// Gets the ancestors of this node (excluding this node itself)

103
ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs

@ -21,6 +21,7 @@
using System; using System;
using System.Collections; using System.Collections;
using System.Collections.Generic; using System.Collections.Generic;
using System.Diagnostics;
using ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching; using ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching;
@ -267,23 +268,103 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
get { return false; } get { return false; }
} }
// Tolerates removing or replacing the current element during enumeration (the common // Returned by value, so a foreach over a concrete collection allocates nothing; only enumeration
// transform pattern), matching the previous linked-list collection's behavior. // through the IEnumerable<T> interface (e.g. LINQ) boxes it.
public IEnumerator<T> GetEnumerator() public Enumerator GetEnumerator() => new Enumerator(list);
IEnumerator<T> IEnumerable<T>.GetEnumerator() => new Enumerator(list);
IEnumerator IEnumerable.GetEnumerator() => new Enumerator(list);
/// <summary>
/// Enumerates the collection while tolerating removal or replacement of the current element during
/// the loop body -- the behavior transforms that mutate a collection mid-foreach rely on. Before
/// yielding an element it captures the following one, then resumes on that captured successor
/// located by identity, so the walk follows node identity hand-over-hand like the old linked-list
/// enumerator: removing or replacing the current element advances to the captured successor (a
/// replacement is not re-visited), and inserting or removing other elements only shifts the
/// backing-list indices, which the identity resume absorbs. The one mutation it cannot follow is
/// removing the captured successor itself during the body -- that element is gone, so enumeration
/// stops there (the linked-list enumerator lost track of it too).
/// </summary>
public struct Enumerator : IEnumerator<T>
{
readonly List<T> list;
int pos; // index at which 'current' was found when it was yielded
T? current; // the element handed out by the last MoveNext
T? next; // the element that followed 'current' at the moment it was yielded
bool started;
internal Enumerator(List<T> list)
{
this.list = list;
this.pos = 0;
this.current = null;
this.next = null;
this.started = false;
}
public readonly T Current => current!;
readonly object? IEnumerator.Current => current;
public bool MoveNext()
{
if (!started)
{ {
int pos = 0; started = true;
while (pos < list.Count) if (list.Count == 0)
return false;
pos = 0;
}
else
{ {
T cur = list[pos]; if (next == null)
yield return cur; {
if (pos < list.Count && ReferenceEquals(list[pos], cur)) current = null;
pos++; return false;
}
// Resume on the successor captured before the body ran, located by identity. The two
// fast paths cover the common shapes -- the successor still sits at pos+1, or 'current'
// was removed so the successor dropped into pos -- and any other mutation falls back to
// an O(n) identity search. Each fast path is guarded by ReferenceEquals(next), so it can
// only fire when the successor genuinely sits there; landing the cursor anywhere else
// would be the only way to skip or re-yield an element.
int resume;
if (pos + 1 < list.Count && ReferenceEquals(list[pos + 1], next))
resume = pos + 1;
else if (pos < list.Count && ReferenceEquals(list[pos], next))
resume = pos;
else
resume = list.IndexOf(next);
// Runtime guard against the cursor mislocating: the chosen index must equal the
// authoritative identity position (also -1 == -1 when the successor was removed). It
// holds for every mutation the body can make today, and the decompiler test suite runs
// it after every transform, so a future change that lets the cursor skip or re-yield an
// element fails loudly here instead of silently corrupting the output.
Debug.Assert(resume == list.IndexOf(next),
"AstNodeCollection enumerator lost track of its position during a mid-enumeration mutation.");
if (resume < 0)
{
current = null;
next = null;
return false;
}
pos = resume;
} }
current = list[pos];
next = pos + 1 < list.Count ? list[pos + 1] : null;
return true;
} }
IEnumerator IEnumerable.GetEnumerator() public void Reset()
{ {
return GetEnumerator(); pos = 0;
current = null;
next = null;
started = false;
}
public readonly void Dispose() { }
} }
#region Equals and GetHashCode implementation #region Equals and GetHashCode implementation

18
ICSharpCode.Decompiler/CSharp/Syntax/DepthFirstAstVisitor.cs

@ -35,9 +35,9 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
{ {
protected virtual void VisitChildren(AstNode node) protected virtual void VisitChildren(AstNode node)
{ {
// GetChildNodes enumerates in slot order and tolerates the visitor removing or replacing // Children enumerates in document order and tolerates the visitor removing or replacing
// the current child. // the current child (its enumerator captures the successor before yielding).
foreach (var child in node.GetChildNodes()) foreach (var child in node.Children)
{ {
child.AcceptVisitor(this); child.AcceptVisitor(this);
} }
@ -707,9 +707,9 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
{ {
protected virtual T VisitChildren(AstNode node) protected virtual T VisitChildren(AstNode node)
{ {
// GetChildNodes enumerates in slot order and tolerates the visitor removing or replacing // Children enumerates in document order and tolerates the visitor removing or replacing
// the current child. // the current child (its enumerator captures the successor before yielding).
foreach (var child in node.GetChildNodes()) foreach (var child in node.Children)
{ {
child.AcceptVisitor(this); child.AcceptVisitor(this);
} }
@ -1382,9 +1382,9 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
{ {
protected virtual S VisitChildren(AstNode node, T data) protected virtual S VisitChildren(AstNode node, T data)
{ {
// GetChildNodes enumerates in slot order and tolerates the visitor removing or replacing // Children enumerates in document order and tolerates the visitor removing or replacing
// the current child. // the current child (its enumerator captures the successor before yielding).
foreach (var child in node.GetChildNodes()) foreach (var child in node.Children)
{ {
child.AcceptVisitor(this, data); child.AcceptVisitor(this, data);
} }

Loading…
Cancel
Save