Browse Source

Keep child indices on in-place replacement to avoid O(N^2) rewrites

A node's children carry a cached flattened childIndex, rebuilt by
EnsureChildIndices after any structural mutation invalidates it. The
single-slot setter and the collection indexer invalidated the whole set on
every in-place replace, so the next sibling navigation (e.g. the visitor's
NextSibling walk) rebuilt all indices in O(children). A transform that
replaces each element of a block while traversing it was therefore O(N^2).

But replacing a child in place does not move anything: a single slot always
occupies the same flattened index, and a replaced collection element keeps
its position. So carry the old child's index to the new child and skip the
invalidation. Setting or clearing a slot still invalidates, since the new
child's index is not known locally; a stale carried value is corrected by
the next renumber anyway.

Microbenchmark (replace every statement in an N-statement block): at
N=32000, 2158 ms -> 1 ms. Output is byte-identical and the Pretty suite
stays 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
f1eff1a193
  1. 14
      ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs
  2. 6
      ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs

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

@ -539,10 +539,20 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
throw new ArgumentException("Node is already used in another tree.", nameof(value)); throw new ArgumentException("Node is already used in another tree.", nameof(value));
} }
} }
field?.ClearParentAndIndex(); T? oldField = field;
int oldChildIndex = oldField?.childIndex ?? -1;
oldField?.ClearParentAndIndex();
field = newValue; field = newValue;
newValue?.SetParent(this); newValue?.SetParent(this);
InvalidateChildIndices(); // 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;
else
InvalidateChildIndices();
} }
internal void SetParent(AstNode newParent) internal void SetParent(AstNode newParent)

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

@ -98,10 +98,14 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
if (old == value) if (old == value)
return; return;
ValidateNewChild(value); ValidateNewChild(value);
int oldChildIndex = old.childIndex;
old.ClearParentAndIndex(); old.ClearParentAndIndex();
list[index] = value; list[index] = value;
value.SetParent(parent); value.SetParent(parent);
parent.InvalidateChildIndices(); // Replacing an element in place keeps its flattened position, so no index changes: carry
// the old element's index to the new one rather than invalidating (and rebuilding) the
// parent's whole index set. A stale carried value is corrected by the next renumber.
value.childIndex = oldChildIndex;
} }
} }

Loading…
Cancel
Save