From a04be1dd536ff7346427ec0b9634c3738f2e008e Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 20 Jun 2026 20:51:09 +0200 Subject: [PATCH] Maintain child indices incrementally for a node's sole collection slot Removing a collection element invalidated the parent's whole flattened index set, so the next sibling navigation rebuilt it in O(children) -- and that renumber, not the array shift, was the dominant cost: a reverse (tail-first) removal, which shifts nothing, was still quadratic, isolating EnsureChildIndices as the culprit. When a node's only collection is also its last slot it owns the contiguous range [base, base + Count) with nothing after it, so an element's flattened index is just base + its local position (base is the slot's declaration position, since the preceding slots are all single children). On that fast-path -- which the generator now flags, passing the base index -- Add indexes only the appended element, Insert/Remove renumber only the shifted suffix, and IndexOf is base-relative O(1); none of them invalidate. Other shapes (several collections, or a slot after the collection) keep the invalidate-and-rebuild fallback. Tail and scattered removal, and removal during traversal, no longer pay the per-operation renumber. Microbenchmark, removing every element of an N-element block: tail-first at N=32000 went 1493 ms -> 0.5 ms (now O(N)); front-first is ~1.6x faster and no longer renumbers (its residual cost is the inherent array shift). Output is byte-identical and the Pretty suite stays green with CheckInvariant validating the maintained indices after every transform. Assisted-by: Claude:claude-opus-4-8:Claude Code --- .../DecompilerSyntaxTreeGenerator.cs | 11 +++- .../CSharp/Syntax/AstNode.cs | 4 ++ .../CSharp/Syntax/AstNodeCollection.cs | 55 ++++++++++++++++++- 3 files changed, 65 insertions(+), 5 deletions(-) diff --git a/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs b/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs index e0f388031..f926d6299 100644 --- a/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs +++ b/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs @@ -347,14 +347,21 @@ internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator // field (returned null-forgiving for a required child, nullable for an optional one); a // collection slot owns a lazily created AstNodeCollection bound to this node. A slot re-declared from an inherited contract // member (Part I.3 flatten) is an override. - foreach (var (isCollection, name, type, elementType, isOverride, isNullable, kindName, isPartial) in slots) + // A collection can maintain its children's flattened indices incrementally only when it is the + // node's sole collection and its last slot: then it owns the contiguous range [slotIndex, ..) + // with nothing after it, so an element's index is slotIndex + its local position (all + // preceding slots are single children, one index each). + int collectionCount = slots.Count(s => s.IsCollection); + for (int slotIndex = 0; slotIndex < slots.Count; slotIndex++) { + var (isCollection, name, type, elementType, isOverride, isNullable, kindName, isPartial) = slots[slotIndex]; string field = FieldName(name); string partialKw = isPartial ? "partial " : ""; if (isCollection) { + bool supportsIncremental = collectionCount == 1 && slotIndex == slots.Count - 1; builder.AppendLine($"\tAstNodeCollection<{elementType}>? {field};"); - builder.AppendLine($"\tpublic {(isOverride ? "override " : "")}{partialKw}AstNodeCollection<{elementType}> {name} => {field} ??= new AstNodeCollection<{elementType}>(this, Slots.{kindName});"); + builder.AppendLine($"\tpublic {(isOverride ? "override " : "")}{partialKw}AstNodeCollection<{elementType}> {name} => {field} ??= new AstNodeCollection<{elementType}>(this, Slots.{kindName}, {slotIndex}, {(supportsIncremental ? "true" : "false")});"); } else { diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs b/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs index 8f61e62e5..304c48bc3 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs @@ -454,6 +454,10 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax // (many appends with no interleaved index reads) renumbers once instead of once per mutation. bool childIndicesValid = true; + // Whether this node's children currently carry correct flattened childIndex values, so a + // collection can decide between maintaining them incrementally and invalidating the whole set. + internal bool ChildIndicesValid => childIndicesValid; + // Marks this node's children's flattened indices stale. Called by the slot setters and the // collection after any structural change. internal void InvalidateChildIndices() diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs b/ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs index 6ba98154b..2b9eeac08 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs @@ -78,10 +78,34 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax // case for Attributes, TypeArguments, constraints, ...) then costs only this wrapper, not a List. List? list; + // When this is the parent's only collection and its last slot, the collection occupies the + // contiguous flattened range [baseIndex, baseIndex + Count) with nothing after it, so an element's + // flattened childIndex is exactly baseIndex + its local position. That lets mutations maintain + // childIndex incrementally (no full re-index) and IndexOf run in O(1). Other shapes (multiple + // collections, or a slot following this one) fall back to invalidate-and-rebuild. + readonly int baseIndex; + readonly bool supportsIncremental; + public AstNodeCollection(AstNode parent, CSharpSlotInfo kind) + : this(parent, kind, 0, false) + { + } + + public AstNodeCollection(AstNode parent, CSharpSlotInfo kind, int baseIndex, bool supportsIncremental) { this.parent = parent ?? throw new ArgumentNullException(nameof(parent)); this.kind = kind; + this.baseIndex = baseIndex; + this.supportsIncremental = supportsIncremental; + } + + // Reassigns the flattened childIndex of every element from 'start' to the end after a shift, + // keeping the parent's indices valid without a full rebuild. Only meaningful on the incremental + // fast-path (the collection occupies [baseIndex, baseIndex + Count) with nothing after it). + void ReindexFrom(int start) + { + for (int i = start; i < list!.Count; i++) + list[i].childIndex = baseIndex + i; } public int Count { @@ -126,7 +150,12 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax ValidateNewChild(element); (list ??= new List()).Add(element); element.SetParent(parent); - parent.InvalidateChildIndices(); + // Appending leaves every existing index unchanged, so just index the new element instead of + // invalidating; otherwise fall back to a rebuild. + if (supportsIncremental && parent.ChildIndicesValid) + element.childIndex = baseIndex + list.Count - 1; + else + parent.InvalidateChildIndices(); } public void AddRange(IEnumerable nodes) @@ -183,6 +212,14 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax { if (element == null || element.Parent != parent) return -1; + // O(1) when the indices are current and this collection owns a contiguous flattened range: + // the element's local position is its childIndex minus our base. + if (supportsIncremental && parent.ChildIndicesValid && list != null) + { + int local = element.childIndex - baseIndex; + if (local >= 0 && local < list.Count && ReferenceEquals(list[local], element)) + return local; + } return list?.IndexOf(element) ?? -1; } @@ -191,9 +228,15 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax int index = IndexOf(element); if (index < 0) return false; + bool incremental = supportsIncremental && parent.ChildIndicesValid; list!.RemoveAt(index); element.ClearParentAndIndex(); - parent.InvalidateChildIndices(); + // The elements after the removed one shifted down by one; renumber just them rather than + // invalidating (and later rebuilding) the parent's whole index set. + if (incremental) + ReindexFrom(index); + else + parent.InvalidateChildIndices(); return true; } @@ -419,9 +462,15 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax if (newItem == null) return; ValidateNewChild(newItem); + bool incremental = supportsIncremental && parent.ChildIndicesValid; (list ??= new List()).Insert(index, newItem); newItem.SetParent(parent); - parent.InvalidateChildIndices(); + // The new element and everything after it occupy fresh positions; renumber from the insertion + // point rather than invalidating the parent's whole index set. + if (incremental) + ReindexFrom(index); + else + parent.InvalidateChildIndices(); } ///