Browse Source

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
pull/3807/head
Siegfried Pammer 2 weeks ago committed by Siegfried Pammer
parent
commit
a04be1dd53
  1. 13
      ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs
  2. 4
      ICSharpCode.Decompiler/CSharp/Syntax/AstNode.cs
  3. 49
      ICSharpCode.Decompiler/CSharp/Syntax/AstNodeCollection.cs

13
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 // 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 // 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. // 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 field = FieldName(name);
string partialKw = isPartial ? "partial " : ""; string partialKw = isPartial ? "partial " : "";
if (isCollection) if (isCollection)
{ {
bool supportsIncremental = collectionCount == 1 && slotIndex == slots.Count - 1;
builder.AppendLine($"\tAstNodeCollection<{elementType}>? {field};"); 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 else
{ {

4
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. // (many appends with no interleaved index reads) renumbers once instead of once per mutation.
bool childIndicesValid = true; 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 // Marks this node's children's flattened indices stale. Called by the slot setters and the
// collection after any structural change. // collection after any structural change.
internal void InvalidateChildIndices() internal void InvalidateChildIndices()

49
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. // case for Attributes, TypeArguments, constraints, ...) then costs only this wrapper, not a List.
List<T>? list; List<T>? 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) 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.parent = parent ?? throw new ArgumentNullException(nameof(parent));
this.kind = kind; 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 { public int Count {
@ -126,6 +150,11 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
ValidateNewChild(element); ValidateNewChild(element);
(list ??= new List<T>()).Add(element); (list ??= new List<T>()).Add(element);
element.SetParent(parent); element.SetParent(parent);
// 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(); parent.InvalidateChildIndices();
} }
@ -183,6 +212,14 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
{ {
if (element == null || element.Parent != parent) if (element == null || element.Parent != parent)
return -1; 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; return list?.IndexOf(element) ?? -1;
} }
@ -191,8 +228,14 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
int index = IndexOf(element); int index = IndexOf(element);
if (index < 0) if (index < 0)
return false; return false;
bool incremental = supportsIncremental && parent.ChildIndicesValid;
list!.RemoveAt(index); list!.RemoveAt(index);
element.ClearParentAndIndex(); element.ClearParentAndIndex();
// 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(); parent.InvalidateChildIndices();
return true; return true;
} }
@ -419,8 +462,14 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax
if (newItem == null) if (newItem == null)
return; return;
ValidateNewChild(newItem); ValidateNewChild(newItem);
bool incremental = supportsIncremental && parent.ChildIndicesValid;
(list ??= new List<T>()).Insert(index, newItem); (list ??= new List<T>()).Insert(index, newItem);
newItem.SetParent(parent); newItem.SetParent(parent);
// 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(); parent.InvalidateChildIndices();
} }

Loading…
Cancel
Save