Browse Source

Maintain single-slot child indices by construction, like the IL AST

A single slot always occupies the same flattened index, so filling,
clearing, or replacing it moves no other child -- only the new child's own
index needs setting. The generated single-slot setters now pass that index
to SetChildNode (a compile-time constant when no collection precedes the
slot; SetChild forwards its argument), which assigns childIndex directly and
never invalidates -- mirroring the IL AST's SetChildInstruction(ref, value,
index). Previously a set-from-null could not know the index here and fell
back to invalidating, forcing a later O(children) EnsureChildIndices rebuild.

With the indices now kept current by construction, NextSibling/PrevSibling
inline the validity check and skip the (non-inlinable) EnsureChildIndices
call in the overwhelmingly common already-valid case.

Together these idle the renumber machinery on a System.Private.CoreLib
decompile: EnsureChildIndices calls 53.4M -> 1.6M, actual rebuilds
1.97M -> 183K, elements renumbered 3.40M -> 757K. Output is byte-identical
and the Pretty suite stays green with CheckInvariant validating the
directly-assigned indices after every transform.

Assisted-by: Claude:claude-opus-4-8:Claude Code
pull/3807/head
Siegfried Pammer 2 weeks ago committed by Siegfried Pammer
parent
commit
9212387e3d
  1. 12
      ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs
  2. 66
      ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs

12
ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs

@ -365,10 +365,18 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator @@ -365,10 +365,18 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator
}
else
{
// A single slot occupies a fixed flattened index. When no collection precedes it, that
// index is the constant slotIndex, so the setter can assign childIndex directly (the ILAst
// pattern -- no invalidate, no renumber). After a collection the index is dynamic, so fall
// back to the index-less setter (which invalidates on a set/clear).
bool constIndex = !slots.Take(slotIndex).Any(s => s.IsCollection);
builder.AppendLine($"\t{type}? {field};");
builder.AppendLine($"\tpublic {(isOverride ? "override " : "")}{partialKw}{type}{(isNullable ? "?" : "")} {name}");
builder.AppendLine("\t{");
builder.AppendLine($"\t\tget => {field}{(isNullable ? "" : "!")};");
if (constIndex)
builder.AppendLine($"\t\tset => SetChildNode(ref {field}, value, {slotIndex});");
else
builder.AppendLine($"\t\tset => SetChildNode(ref {field}, value);");
builder.AppendLine("\t}");
}
@ -571,7 +579,7 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator @@ -571,7 +579,7 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator
for (int k = 0; k < slots.Count; k++)
{
builder.AppendLine($"\t\t\tcase {k}:");
builder.AppendLine($"\t\t\t\tSetChildNode(ref {FieldName(slots[k].PropertyName)}, ({slots[k].PropertyType}?)value);");
builder.AppendLine($"\t\t\t\tSetChildNode(ref {FieldName(slots[k].PropertyName)}, ({slots[k].PropertyType}?)value, index);");
builder.AppendLine("\t\t\t\treturn;");
}
builder.AppendLine("\t\t\tdefault:");
@ -603,7 +611,7 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator @@ -603,7 +611,7 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator
{
builder.AppendLine("\t\tif (i == 0)");
builder.AppendLine("\t\t{");
builder.AppendLine($"\t\t\tSetChildNode(ref {field}, ({s.PropertyType}?)value);");
builder.AppendLine($"\t\t\tSetChildNode(ref {field}, ({s.PropertyType}?)value, index);");
builder.AppendLine("\t\t\treturn;");
builder.AppendLine("\t\t}");
if (!last)

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

@ -151,6 +151,9 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -151,6 +151,9 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
get {
if (parent == null)
return null;
// Inline the validity check: sibling navigation is one of the hottest operations, and the
// indices are almost always already current, so skip the (non-inlinable) call when valid.
if (!parent.childIndicesValid)
parent.EnsureChildIndices();
int count = parent.GetChildCount();
for (int i = childIndex + 1; i < count; i++)
@ -167,6 +170,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -167,6 +170,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
get {
if (parent == null)
return null;
if (!parent.childIndicesValid)
parent.EnsureChildIndices();
for (int i = childIndex - 1; i >= 0; i--)
{
@ -522,43 +526,61 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax @@ -522,43 +526,61 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
}
}
// Writes a single-slot backing field: detaches the old child, attaches the new one, renumbers.
// A null value empties the slot. Called by generated single-slot property setters.
internal void SetChildNode<T>(ref T? field, T? value) where T : AstNode
// Self-reference and two-tree guards shared by the single-slot setters; lifts a node out of the
// subtree currently being replaced (e.g. "x.Right = ((BinaryOperatorExpression)x.Right).Right;").
void ValidateNewSingleChild<T>(T? value, T? oldChild) where T : AstNode
{
T? newValue = value;
if (field == newValue)
if (value == null)
return;
if (newValue != null)
{
if (newValue == this)
if (value == this)
throw new ArgumentException("Cannot add a node to itself as a child.", nameof(value));
if (newValue.parent != null)
if (value.parent != null)
{
// Allow lifting a node out of the subtree being replaced, e.g.
// "assignment.Right = ((BinaryOperatorExpression)assignment.Right).Right;".
if (field != null && newValue.Ancestors.Contains(field))
newValue.Remove();
if (oldChild != null && value.Ancestors.Contains(oldChild))
value.Remove();
else
throw new ArgumentException("Node is already used in another tree.", nameof(value));
}
}
// Writes a single-slot backing field when the slot's flattened index is not statically known
// (a single slot following a collection). A single slot always occupies the same index, so an
// in-place replace keeps it (carry the old child's index); a set or clear leaves the new child's
// index unknown here and invalidates, to be reassigned by the next EnsureChildIndices.
internal void SetChildNode<T>(ref T? field, T? value) where T : AstNode
{
if (field == value)
return;
ValidateNewSingleChild(value, field);
T? oldField = field;
int oldChildIndex = oldField?.childIndex ?? -1;
oldField?.ClearParentAndIndex();
field = newValue;
newValue?.SetParent(this);
// A single slot always occupies the same flattened index, so replacing its child in place
// changes no index: carry the slot's index to the new child instead of invalidating (and
// rebuilding) every child's index. Setting or clearing the slot still invalidates, since the
// new child's index is then not known here. When the indices are already stale the carried
// value is corrected by the next EnsureChildIndices anyway.
if (oldField != null && newValue != null)
newValue.childIndex = oldChildIndex;
field = value;
value?.SetParent(this);
if (oldField != null && value != null)
value.childIndex = oldChildIndex;
else
InvalidateChildIndices();
}
// Writes a single-slot backing field whose flattened index is known (the generated setter passes
// it when no collection precedes the slot, and SetChild passes its argument). Filling, clearing,
// or replacing a single slot moves no other child, so assign the index directly and never
// invalidate -- childIndex stays maintained by construction, as in the IL AST.
internal void SetChildNode<T>(ref T? field, T? value, int index) where T : AstNode
{
if (field == value)
return;
ValidateNewSingleChild(value, field);
field?.ClearParentAndIndex();
field = value;
if (value != null)
{
value.SetParent(this);
value.childIndex = index;
}
}
internal void SetParent(AstNode newParent)
{
parent = newParent;

Loading…
Cancel
Save