From f1eff1a193e581d4d17d584c35b2dcb357e64a3f Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 20 Jun 2026 20:38:13 +0200 Subject: [PATCH] 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 --- ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs | 14 ++++++++++++-- .../CSharp/Syntax/AstNodeCollection.cs | 6 +++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs b/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs index 4a98052c5..8f61e62e5 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs +++ b/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)); } } - field?.ClearParentAndIndex(); + T? oldField = field; + int oldChildIndex = oldField?.childIndex ?? -1; + oldField?.ClearParentAndIndex(); field = newValue; 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) diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs b/ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs index 3b1ce699b..6ba98154b 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs @@ -98,10 +98,14 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax if (old == value) return; ValidateNewChild(value); + int oldChildIndex = old.childIndex; old.ClearParentAndIndex(); list[index] = value; 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; } }