perf(engine): back strong Map/Set with a SameValueZero-keyed ordered hash

docs/adr/0077-samevaluezero-ordered-collections.md

@@ -0,0 +1,15 @@
+# SameValueZero-keyed ordered store for Map and Set
+
+**Date:** 2026-06-27
+**Area:** `data-structures` / `engine`
+**Issue:** [#807](https://github.com/frostney/GocciaScript/issues/807)
+
+Strong `Map` and `Set` stored their entries in a flat list and located keys by a linear SameValueZero scan, so every `get`/`set`/`has`/`delete` was O(n) and any build-or-probe loop was O(n²); set-algebra (`union`/`intersection`/…) was O(n·m). `WeakMap`/`WeakSet` already use [`THashMap`](0019-custom-hash-maps-over-tdictionary.md) keyed by pointer identity, but strong collections cannot reuse it: their keys compare by **SameValueZero** (ES2026 §7.2.11) — `NaN` equals `NaN`, `-0` equals `+0`, strings and BigInts compare by content, and only objects/functions/symbols compare by reference. Identity hashing would split equal primitives across buckets. The distinction is therefore **content-hash (strong Map/Set) vs identity-hash (weak collections)**, and the two stores stay separate.
+
+We back both `Map` and `Set` with a single new type, `TGocciaOrderedValueMap` (`source/units/Goccia.Values.OrderedValueMap.pas`) — the first production consumer of the generic insertion-ordered `TOrderedMap` (ADR 0019). It overrides the map's protected `HashKey`/`KeysEqual` with a hash that is **exactly consistent with `IsSameValueZero`** (number with NaN- and signed-zero canonicalization over the IEEE-754 bits, string/BigInt by content, primitives by category, objects by pointer — the GC never relocates, so pointers are stable) and `IsSameValueZero` itself for equality. `Set` reuses the same type with the element stored as both key and value, so there is one store, one hash, and one set of tests rather than a per-collection subclass or a parallel structure. Operations become O(1) amortized and set-algebra O(n+m). The hash/equality pair is the load-bearing invariant — an inconsistent hash silently loses entries — so it is guarded by a Pascal invariant test (`Goccia.Values.OrderedValueMap.Test.pas`) in addition to the JavaScript suite.
+
+Live-iterator semantics follow the spec's `[[MapData]]` model: the entry list is append-only and `delete` writes a tombstone rather than shifting, so a Map/Set iterator (and `forEach`) holds a physical cursor that skips tombstones and sees entries appended mid-iteration (ES2026 §24.1.3.5). The one hazard is the inherited tombstone-reclaiming compaction, which renumbers entries; we add a `CanCompact` virtual to `TOrderedMap` (default unchanged) and gate it on a per-store live-iterator counter (`RetainIterator`/`ReleaseIterator`), so while any iteration is in flight the store only grows (which preserves indices) and never renumbers, then reclaims tombstones once no iteration is active. This is the same shape as engines that only shrink a keyed collection's storage once no iterator references it. The residual gap is an iterator that is partially consumed and then abandoned (never exhausted or closed) before being garbage-collected: it leaves the gate held, so that store stops reclaiming tombstones until it is cleared, and sustained `set`+`delete` churn on it grows storage meanwhile. That is a safe degradation, not a correctness bug — the common cases (full `for...of`, spread, `forEach`, an exhausted or `return()`-closed iterator) always release the gate — and releasing it for abandoned iterators needs GC-driven notification (the iterator cannot safely touch its source store from a sweep-time destructor), which is left as a follow-up. Insertion also applies CanonicalizeKeyedCollectionKey (ES2026 §24.5.1), storing `-0` as `+0`.
+
+Adopting the spec model fixed four pre-existing conformance bugs the flat-list implementation carried (none covered by the prior suite): `-0` keys were stored un-normalized; `forEach`/iterators did not visit entries appended during iteration; deleting during `forEach` threw `RangeError: Argument out of range`; and deleting the current key during `for...of` skipped the next entry and left `size` wrong. Regression tests for all four ship with this change.
+
+Alternatives rejected: **extending `THashMap` with SameValueZero** — it is unordered, tombstone-free, and identity-tuned, so it would need insertion ordering and content equality bolted on, i.e. effectively this store; a **purpose-built standalone store** modelled on `TOrderedStringMap` — avoids virtual `HashKey` dispatch but adds a fourth map implementation to maintain and leaves `TOrderedMap` with no production user; **keeping the flat list with a side hash index** — leaves `delete` O(n) and does not fix the live-iteration bugs; and **compacting eagerly always** (renumbers live cursors) or **never** (unbounded growth under sustained `set`+`delete` churn). See [ADR 0019](0019-custom-hash-maps-over-tdictionary.md) and [docs/value-system.md](../value-system.md).

docs/adr/README.md

@@ -86,3 +86,4 @@ Durable architecture and implementation decisions for GocciaScript. New ADRs use
 - [0074 — Deferred bytecode call-stack frames](0074-deferred-bytecode-call-stack-frames.md)
 - [0075 — ShadowRealm full test262 conformance](0075-shadowrealm-conformance.md)
 - [0076 — Same-runner benchmark comparison](0076-same-runner-benchmark-comparison.md)
+- [0077 — SameValueZero-keyed ordered store for Map and Set](0077-samevaluezero-ordered-collections.md)

source/shared/OrderedMap.pas

@@ -72,6 +72,12 @@     TEnumerator = record
   protected
     function HashKey(const AKey: TKey): Cardinal; virtual;
     function KeysEqual(const A, B: TKey): Boolean; virtual;
+    // Gate for the tombstone-reclaiming Compact in Add.  Default True.
+    // Subclasses whose entries are observed by live, index-based cursors
+    // (e.g. JS Map/Set iterators) override this to suppress compaction
+    // while a cursor is active: Compact renumbers FEntries, but Grow does
+    // not, so returning False keeps physical indices stable for the cursor.
+    function CanCompact: Boolean; virtual;
 
     function GetCount: Integer; override;
     function GetValue(const AKey: TKey): TValue; override;
@@ -93,6 +99,17 @@     TEnumerator = record
     function GetEnumerator: TEnumerator; inline;
     function EntryAt(AIndex: Integer): TBaseMap<TKey, TValue>.TKeyValuePair;
 
+    // Number of physical entry slots, including tombstones — the upper bound of
+    // valid physical indices. Stable while compaction is suppressed; grows only
+    // by appends. Callers that need "the entries present at a point in time"
+    // (e.g. spec-bounded Set operations) capture this, then iterate with
+    // NextEntryBounded so entries appended later are not visited.
+    function EntrySlotCount: Integer;
+    // Like GetNextEntry, but stops once the next active slot would be at or past
+    // ALimit. Skips tombstones below ALimit.
+    function NextEntryBounded(var AIterState: Integer; ALimit: Integer;
+      out AKey: TKey; out AValue: TValue): Boolean;
+
     property Capacity: Integer read FBucketCount;
   end;
 
@@ -140,6 +157,11 @@ function TOrderedMap<TKey, TValue>.KeysEqual(const A, B: TKey): Boolean;
     Result := CompareMem(@A, @B, SizeOf(TKey));
 end;
 
+function TOrderedMap<TKey, TValue>.CanCompact: Boolean;
+begin
+  Result := True;
+end;
+
 { Probe }
 
 function TOrderedMap<TKey, TValue>.FindBucket(const AKey: TKey; AHash: Cardinal;
@@ -282,7 +304,7 @@ procedure TOrderedMap<TKey, TValue>.Add(const AKey: TKey; const AValue: TValue);
 
   if (FEntryCount + 1) * 100 > FBucketCount * LOAD_FACTOR_PERCENT then
   begin
-    if FCount < FEntryCount div 2 then
+    if (FCount < FEntryCount div 2) and CanCompact then
       Compact
     else
       Grow;
@@ -428,6 +450,31 @@ function TOrderedMap<TKey, TValue>.GetNextEntry(var AIterState: Integer;
   Result := False;
 end;
 
+function TOrderedMap<TKey, TValue>.EntrySlotCount: Integer;
+begin
+  Result := FEntryCount;
+end;
+
+function TOrderedMap<TKey, TValue>.NextEntryBounded(var AIterState: Integer;
+  ALimit: Integer; out AKey: TKey; out AValue: TValue): Boolean;
+begin
+  if ALimit > FEntryCount then
+    ALimit := FEntryCount;
+  while AIterState < ALimit do
+  begin
+    if FEntries[AIterState].Active then
+    begin
+      AKey := FEntries[AIterState].Key;
+      AValue := FEntries[AIterState].Value;
+      Inc(AIterState);
+      Result := True;
+      Exit;
+    end;
+    Inc(AIterState);
+  end;
+  Result := False;
+end;
+
 function TOrderedMap<TKey, TValue>.EntryAt(
   AIndex: Integer): TBaseMap<TKey, TValue>.TKeyValuePair;
 var

source/units/Goccia.Builtins.GlobalMap.pas

@@ -63,7 +63,7 @@ function TGocciaGlobalMap.MapGroupBy(const AArgs: TGocciaArgumentsCollection; co
 
   procedure AddToGroup(const AValue: TGocciaValue; const AIndex: Integer);
   var
-    EntryIndex: Integer;
+    ExistingGroup: TGocciaValue;
   begin
     CallArgs := TGocciaArgumentsCollection.Create;
     try
@@ -75,9 +75,8 @@ function TGocciaGlobalMap.MapGroupBy(const AArgs: TGocciaArgumentsCollection; co
     end;
 
     // Use Map's SameValueZero lookup to preserve key identity (1 vs "1", etc.)
-    EntryIndex := ResultMap.FindEntry(GroupKey);
-    if EntryIndex >= 0 then
-      GroupArray := TGocciaArrayValue(ResultMap.Entries[EntryIndex].Value)
+    if ResultMap.TryGetValue(GroupKey, ExistingGroup) then
+      GroupArray := TGocciaArrayValue(ExistingGroup)
     else
     begin
       GroupArray := TGocciaArrayValue.Create;

source/units/Goccia.Builtins.Globals.pas

@@ -835,31 +835,45 @@ function CloneArray(const AArr: TGocciaArrayValue;
 function CloneMap(const AMap: TGocciaMapValue;
   const AMemory: THashMap<TGocciaValue, TGocciaValue>): TGocciaMapValue;
 var
-  I: Integer;
-  Entry: TGocciaMapEntry;
+  Cursor: Integer;
+  Key, Value: TGocciaValue;
 begin
   Result := TGocciaMapValue.Create;
   AMemory.Add(AMap, Result);
 
-  for I := 0 to AMap.Entries.Count - 1 do
-  begin
-    Entry := AMap.Entries[I];
-    Result.SetEntry(
-      StructuredCloneValue(Entry.Key, AMemory),
-      StructuredCloneValue(Entry.Value, AMemory));
+  // StructuredCloneValue can run user getters that mutate AMap; retain it so
+  // compaction cannot renumber entries mid-walk and invalidate Cursor.
+  Cursor := 0;
+  AMap.RetainIterator;
+  try
+    while AMap.NextEntry(Cursor, Key, Value) do
+      Result.SetEntry(
+        StructuredCloneValue(Key, AMemory),
+        StructuredCloneValue(Value, AMemory));
+  finally
+    AMap.ReleaseIterator;
   end;
 end;
 
 function CloneSet(const ASet: TGocciaSetValue;
   const AMemory: THashMap<TGocciaValue, TGocciaValue>): TGocciaSetValue;
 var
-  I: Integer;
+  Cursor: Integer;
+  Item: TGocciaValue;
 begin
   Result := TGocciaSetValue.Create;
   AMemory.Add(ASet, Result);
 
-  for I := 0 to ASet.Items.Count - 1 do
-    Result.AddItem(StructuredCloneValue(ASet.Items[I], AMemory));
+  // StructuredCloneValue can run user getters that mutate ASet; retain it so
+  // compaction cannot renumber entries mid-walk and invalidate Cursor.
+  Cursor := 0;
+  ASet.RetainIterator;
+  try
+    while ASet.NextItem(Cursor, Item) do
+      Result.AddItem(StructuredCloneValue(Item, AMemory));
+  finally
+    ASet.ReleaseIterator;
+  end;
 end;
 
 function CloneArrayBuffer(const ABuf: TGocciaArrayBufferValue;

source/units/Goccia.Evaluator.Comparison.pas

@@ -79,6 +79,8 @@ function IsDeepEqualInternal(const AActual, AExpected: TGocciaValue;
   ActualKeys, ExpectedKeys: TArray<string>;
   I: Integer;
   Key: string;
+  CursorA, CursorB: Integer;
+  LeftKey, LeftValue, RightKey, RightValue: TGocciaValue;
 begin
   // Base case: strict equality (handles primitives and same object references)
   if IsStrictEqual(AActual, AExpected) then
@@ -142,10 +144,10 @@ function IsDeepEqualInternal(const AActual, AExpected: TGocciaValue;
     Exit;
   end;
 
-  // Handle Sets
+  // Handle Sets — compared by insertion order, element for element.
   if (AActual is TGocciaSetValue) and (AExpected is TGocciaSetValue) then
   begin
-    if TGocciaSetValue(AActual).Items.Count <> TGocciaSetValue(AExpected).Items.Count then
+    if TGocciaSetValue(AActual).Count <> TGocciaSetValue(AExpected).Count then
     begin
       Result := False;
       Exit;
@@ -156,23 +158,39 @@ function IsDeepEqualInternal(const AActual, AExpected: TGocciaValue;
       Exit;
     end;
     AddComparedPair(AComparedPairs, AActual, AExpected);
-    for I := 0 to TGocciaSetValue(AActual).Items.Count - 1 do
-    begin
-      if not IsDeepEqualInternal(TGocciaSetValue(AActual).Items[I],
-        TGocciaSetValue(AExpected).Items[I], AComparedPairs) then
+    CursorA := 0;
+    CursorB := 0;
+    // Recursive comparison can run user getters that mutate either set; retain
+    // both so cursors stay valid, and confirm the expected side advances before
+    // comparing (avoids comparing stale out values).
+    TGocciaSetValue(AActual).RetainIterator;
+    TGocciaSetValue(AExpected).RetainIterator;
+    try
+      while TGocciaSetValue(AActual).NextItem(CursorA, LeftValue) do
       begin
-        Result := False;
-        Exit;
+        if not TGocciaSetValue(AExpected).NextItem(CursorB, RightValue) then
+        begin
+          Result := False;
+          Exit;
+        end;
+        if not IsDeepEqualInternal(LeftValue, RightValue, AComparedPairs) then
+        begin
+          Result := False;
+          Exit;
+        end;
       end;
+    finally
+      TGocciaSetValue(AExpected).ReleaseIterator;
+      TGocciaSetValue(AActual).ReleaseIterator;
     end;
     Result := True;
     Exit;
   end;
 
-  // Handle Maps
+  // Handle Maps — compared by insertion order, entry for entry.
   if (AActual is TGocciaMapValue) and (AExpected is TGocciaMapValue) then
   begin
-    if TGocciaMapValue(AActual).Entries.Count <> TGocciaMapValue(AExpected).Entries.Count then
+    if TGocciaMapValue(AActual).Count <> TGocciaMapValue(AExpected).Count then
     begin
       Result := False;
       Exit;
@@ -183,20 +201,35 @@ function IsDeepEqualInternal(const AActual, AExpected: TGocciaValue;
       Exit;
     end;
     AddComparedPair(AComparedPairs, AActual, AExpected);
-    for I := 0 to TGocciaMapValue(AActual).Entries.Count - 1 do
-    begin
-      if not IsDeepEqualInternal(TGocciaMapValue(AActual).Entries[I].Key,
-        TGocciaMapValue(AExpected).Entries[I].Key, AComparedPairs) then
-      begin
-        Result := False;
-        Exit;
-      end;
-      if not IsDeepEqualInternal(TGocciaMapValue(AActual).Entries[I].Value,
-        TGocciaMapValue(AExpected).Entries[I].Value, AComparedPairs) then
+    CursorA := 0;
+    CursorB := 0;
+    // Recursive comparison can run user getters that mutate either map; retain
+    // both so cursors stay valid, and confirm the expected side advances before
+    // comparing (avoids comparing stale out values).
+    TGocciaMapValue(AActual).RetainIterator;
+    TGocciaMapValue(AExpected).RetainIterator;
+    try
+      while TGocciaMapValue(AActual).NextEntry(CursorA, LeftKey, LeftValue) do
       begin
-        Result := False;
-        Exit;
+        if not TGocciaMapValue(AExpected).NextEntry(CursorB, RightKey, RightValue) then
+        begin
+          Result := False;
+          Exit;
+        end;
+        if not IsDeepEqualInternal(LeftKey, RightKey, AComparedPairs) then
+        begin
+          Result := False;
+          Exit;
+        end;
+        if not IsDeepEqualInternal(LeftValue, RightValue, AComparedPairs) then
+        begin
+          Result := False;
+          Exit;
+        end;
       end;
+    finally
+      TGocciaMapValue(AExpected).ReleaseIterator;
+      TGocciaMapValue(AActual).ReleaseIterator;
     end;
     Result := True;
     Exit;