perf(engine): back strong Map/Set with a SameValueZero-keyed ordered hash#890
Merged
Conversation
…hash Strong Map and Set stored entries in a flat list and located keys by a linear SameValueZero scan, making get/set/has/delete O(n), build/probe loops O(n^2), and set-algebra O(n*m). Back both with a single new TGocciaOrderedValueMap — the first production consumer of the generic insertion-ordered TOrderedMap (ADR 0019) — whose HashKey/KeysEqual are overridden with a hash exactly consistent with IsSameValueZero. Ops are now O(1) amortized and set-algebra O(n+m); insertion order is preserved. Deletes tombstone instead of shifting and iterators hold a physical cursor, matching the spec [[MapData]] model. Compaction (which renumbers entries) is gated by a per-store live-iterator counter via a new CanCompact virtual on TOrderedMap, so live cursors never observe a renumbering. Insertion canonicalizes -0 to +0 (ES2026 §24.5.1). Adopting the spec model fixed four pre-existing conformance bugs the flat-list code carried, none covered by the prior suite: -0 keys stored un-normalized; forEach/iterators not seeing entries appended during iteration; deleting during forEach throwing "Argument out of range"; and deleting the current key during for...of skipping the next entry. JS regression tests for all four ship here, plus a Pascal invariant test for the hash/equality consistency. Closes #807 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaces flat-list storage for strong ChangesSameValueZero-keyed ordered Map/Set store
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
Contributor
Suite TimingTest Runner (interpreted: 10,987 passed; bytecode: 10,987 passed)
MemoryGC rows aggregate the main thread plus all worker thread-local GCs. Test runner worker shutdown frees thread-local heaps in bulk; that shutdown reclamation is not counted as GC collections or collected objects.
Benchmarks (interpreted: 436; bytecode: 436)
MemoryGC rows aggregate the main thread plus all worker thread-local GCs. Benchmark runner performs explicit between-file collections, so collection and collected-object counts can be much higher than the test runner.
Measured on ubuntu-latest x64. |
Contributor
Benchmark Results436 benchmarks · PR vs same-runner Interpreted: 🟢 55 improved · 🔴 16 regressed · 365 unchanged · avg +2.3% Typical per-run noise (median variance): interpreted ±3.6%, bytecode ±2.6%. Deltas within noise overlap and read as unchanged. arraybuffer.js — Interp: 14 unch. · avg +1.6% · Bytecode: 🟢 1, 13 unch. · avg +0.1%
arrays.js — Interp: 🔴 2, 17 unch. · avg -1.1% · Bytecode: 🟢 1, 🔴 3, 15 unch. · avg -0.8%
async-await.js — Interp: 🟢 1, 5 unch. · avg -1.9% · Bytecode: 🔴 1, 5 unch. · avg +0.1%
async-generators.js — Interp: 2 unch. · avg -0.7% · Bytecode: 2 unch. · avg -2.1%
atomics.js — Interp: 6 unch. · avg -0.2% · Bytecode: 🟢 3, 3 unch. · avg +1.3%
base64.js — Interp: 🟢 1, 9 unch. · avg +1.0% · Bytecode: 🟢 1, 9 unch. · avg +1.4%
classes.js — Interp: 🟢 4, 🔴 2, 25 unch. · avg +0.9% · Bytecode: 🟢 2, 🔴 1, 28 unch. · avg +1.4%
closures.js — Interp: 🟢 1, 10 unch. · avg +4.5% · Bytecode: 🟢 1, 10 unch. · avg +1.0%
collections.js — Interp: 🟢 8, 4 unch. · avg +36.9% · Bytecode: 🟢 8, 4 unch. · avg +50.8%
csv.js — Interp: 🟢 3, 🔴 1, 9 unch. · avg -0.5% · Bytecode: 🔴 1, 12 unch. · avg -3.0%
destructuring.js — Interp: 🟢 2, 🔴 1, 19 unch. · avg -0.4% · Bytecode: 🟢 3, 🔴 2, 17 unch. · avg +1.8%
fibonacci.js — Interp: 8 unch. · avg +0.4% · Bytecode: 🟢 1, 7 unch. · avg +1.1%
float16array.js — Interp: 🟢 5, 27 unch. · avg +1.2% · Bytecode: 🟢 5, 27 unch. · avg +2.0%
for-in/for-in.js — Interp: 🟢 2, 1 unch. · avg +4.7% · Bytecode: 3 unch. · avg -0.3%
for-of.js — Interp: 🟢 2, 5 unch. · avg +2.7% · Bytecode: 7 unch. · avg -0.3%
generators.js — Interp: 🟢 1, 3 unch. · avg +2.3% · Bytecode: 🟢 1, 3 unch. · avg -0.2%
intl.js — Interp: 🔴 1, 5 unch. · avg -1.6% · Bytecode: 6 unch. · avg +1.7%
iterators.js — Interp: 🟢 2, 40 unch. · avg +3.1% · Bytecode: 🟢 2, 🔴 6, 34 unch. · avg -2.0%
json.js — Interp: 🟢 4, 19 unch. · avg +3.1% · Bytecode: 🟢 3, 🔴 2, 18 unch. · avg +0.6%
jsx.jsx — Interp: 🟢 1, 20 unch. · avg -0.6% · Bytecode: 🟢 3, 🔴 3, 15 unch. · avg -0.1%
modules.js — Interp: 9 unch. · avg +1.0% · Bytecode: 9 unch. · avg +0.5%
numbers.js — Interp: 11 unch. · avg -2.0% · Bytecode: 🟢 4, 7 unch. · avg +6.0%
objects.js — Interp: 🟢 1, 6 unch. · avg +1.1% · Bytecode: 7 unch. · avg -2.2%
promises.js — Interp: 12 unch. · avg -0.1% · Bytecode: 12 unch. · avg +0.5%
property-access.js — Interp: 🟢 1, 4 unch. · avg +7.0% · Bytecode: 5 unch. · avg -0.5%
regexp.js — Interp: 11 unch. · avg -4.0% · Bytecode: 🔴 1, 10 unch. · avg -1.1%
strings.js — Interp: 🟢 3, 16 unch. · avg +5.2% · Bytecode: 🔴 2, 17 unch. · avg -1.7%
temporal.js — Interp: 🟢 1, 5 unch. · avg +1.5% · Bytecode: 6 unch. · avg -1.9%
tsv.js — Interp: 9 unch. · avg -3.1% · Bytecode: 🟢 1, 🔴 2, 6 unch. · avg +0.0%
typed-arrays.js — Interp: 🟢 8, 🔴 1, 13 unch. · avg +23.1% · Bytecode: 🟢 4, 🔴 3, 15 unch. · avg -2.8%
uint8array-encoding.js — Interp: 🟢 4, 14 unch. · avg +3.9% · Bytecode: 🟢 6, 🔴 1, 11 unch. · avg +19.6%
weak-collections.js — Interp: 🔴 8, 7 unch. · avg -26.3% · Bytecode: 🟢 5, 🔴 1, 9 unch. · avg +18.2%
Deterministic profile diffDeterministic profile diff: no significant changes. Measured on ubuntu-latest x64. Each PR run also builds the |
Contributor
test262 Conformance
Areas closest to 100%
Per-test deltas (+13 / -0)Newly passing (13):
Steady-state failures are non-blocking; regressions vs the cached main baseline (lower total pass count, or any PASS → non-PASS transition) fail the conformance gate. Measured on ubuntu-latest x64, bytecode mode. Areas grouped by the first two test262 path components; minimum 25 attempted tests, areas already at 100% excluded. Δ vs main compares against the most recent cached |
coderabbitai
Bot
added
internal
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/units/Goccia.Values.SetValue.pas (1)
402-424: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle omitted Set arguments as
undefined. Lines 402, 423, and 442 skip the operation when no argument is passed, butSet.prototype.has,add, anddeleteshould process anundefinedvalue instead. That makesnew Set().add()a no-op and breaks the spec for omitted arguments.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/units/Goccia.Values.SetValue.pas` around lines 402 - 424, Update the Set prototype methods so omitted arguments are treated as undefined instead of being skipped. In TGocciaSetValue.SetAdd, TGocciaSetValue.SetHas, and TGocciaSetValue.SetDelete, remove the length checks that bypass the operation when AArgs is empty and instead read AArgs.GetElement(0) as undefined when no argument is provided. Ensure the SameValueZero/ContainsValue logic and the add/delete paths continue to run with that undefined value so the behavior matches the spec for Set.prototype.add, Set.prototype.has, and Set.prototype.delete.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/units/Goccia.Builtins.Globals.pas`:
- Around line 844-848: The Map/Set clone loop in StructuredCloneValue can be
invalidated by user code during accessor reads, so the active cursor must be
retained across the walk. Wrap the AMap.NextEntry and ASet.NextEntry iterations
with RetainIterator/ReleaseIterator so compaction cannot renumber entries
mid-clone, and keep the fix in the StructuredCloneValue code paths that populate
Result from AMap and ASet.
In `@source/units/Goccia.Evaluator.Comparison.pas`:
- Around line 161-166: The deep comparison loops in IsDeepEqualInternal need to
retain both collections’ cursor state and verify the expected-side advance
succeeds before comparing values. Update the Set and Map branches that use
TGocciaSetValue.NextItem and TGocciaMapValue.NextEntry so both cursors advance
in lockstep and the comparison only proceeds when the expected cursor call
returns true, avoiding use of stale RightValue/RightKey/RightValue after
recursive property reads mutate either store.
In `@source/units/Goccia.REPL.Formatter.pas`:
- Around line 88-100: The Map/Set cursor traversal in FormatREPLValue needs to
keep the underlying store alive while iterating, because recursive formatting
can trigger object/stringify reads that mutate the collection and invalidate
Cursor. Update the Map/Set formatting loops that use NextEntry in the REPL
formatter to retain the collection/store for the full traversal and release it
afterward, so compaction cannot occur mid-iteration; apply the same fix in both
affected formatting branches.
In `@source/units/Goccia.Values.Iterator.Concrete.pas`:
- Around line 52-57: The iterator classes are missing destruction-time cleanup,
so abandoned objects can leave retained stores unreleased and keep
ActiveIterators elevated. Add destructors to the affected iterator types and
have them call the existing idempotent ReleaseSource helper, alongside the
current Close/exhaustion paths, so cleanup still happens when an iterator object
is destroyed. Use the concrete iterator classes around DirectNext, AdvanceNext,
Close, and ReleaseSource to locate the affected implementations.
In `@source/units/Goccia.Values.MapValue.pas`:
- Line 306: The argument-length guard in the MapValue methods is preventing
valid undefined keys/values from being processed, which breaks m.get(), m.has(),
m.delete(), and m.set(). Update the relevant logic in
TGoccia.Values.MapValue.pas around the MapValue handlers to read arguments
directly from TGocciaArgumentsCollection.GetElement without checking
AArgs.Length first, since GetElement already yields undefined for out-of-range
indexes. Keep the existing TryGetValue-based flow in the affected methods, but
remove the premature length-based blocking so undefined is handled consistently.
In `@source/units/Goccia.Values.OrderedValueMap.pas`:
- Around line 46-54: Split the shared insertion path in OrderedValueMap so Map
and Set use separate helpers: keep SetEntry for Map-style key/value storage, and
add a dedicated Set insert method in the store that canonicalizes the key and
also stores the canonicalized value. Update the Set caller(s) to use this new
helper so Set.add(-0) preserves the +0 invariant through
values()/entries()/forEach without relying on callers to duplicate
canonicalization. Locate the change around CanonicalizeKey, SetEntry, and the
Set-backed insert path.
In `@source/units/Goccia.Values.SetValue.pas`:
- Around line 726-745: The set difference logic in Goccia.Values.SetValue.pas is
iterating ThisSet directly while SetRecordHas may execute user code, which can
mutate the set during traversal. Update the difference path that uses
ThisSet.NextItem and ResultSet.AddItem to iterate over a stable snapshot taken
before the callback runs, so only the pre-callback contents are considered. Keep
the existing RetainIterator/ReleaseIterator protection, but switch the source of
iteration in this branch away from ThisSet itself.
---
Outside diff comments:
In `@source/units/Goccia.Values.SetValue.pas`:
- Around line 402-424: Update the Set prototype methods so omitted arguments are
treated as undefined instead of being skipped. In TGocciaSetValue.SetAdd,
TGocciaSetValue.SetHas, and TGocciaSetValue.SetDelete, remove the length checks
that bypass the operation when AArgs is empty and instead read
AArgs.GetElement(0) as undefined when no argument is provided. Ensure the
SameValueZero/ContainsValue logic and the add/delete paths continue to run with
that undefined value so the behavior matches the spec for Set.prototype.add,
Set.prototype.has, and Set.prototype.delete.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a1301a0e-9c23-4cdf-b36b-80ba53dfdacf
📒 Files selected for processing (18)
docs/adr/0077-samevaluezero-ordered-collections.mddocs/adr/README.mdsource/shared/OrderedMap.passource/units/Goccia.Builtins.GlobalMap.passource/units/Goccia.Builtins.Globals.passource/units/Goccia.Evaluator.Comparison.passource/units/Goccia.REPL.Formatter.passource/units/Goccia.Values.Iterator.Concrete.passource/units/Goccia.Values.MapValue.passource/units/Goccia.Values.OrderedValueMap.Test.passource/units/Goccia.Values.OrderedValueMap.passource/units/Goccia.Values.SetValue.pastests/built-ins/Map/prototype/entries.jstests/built-ins/Map/prototype/forEach.jstests/built-ins/Map/prototype/set.jstests/built-ins/Set/prototype/add.jstests/built-ins/Set/prototype/forEach.jstests/built-ins/Set/prototype/values.js
Addresses review findings on the SameValueZero Map/Set store:
- Retain the source store across the structuredClone, deep-equal, and REPL
cursor walks: those recurse into user getters that can mutate the
collection mid-walk, which could otherwise let compaction renumber entries
and invalidate the cursor. Deep-equal now also confirms the expected-side
cursor advances before comparing.
- Treat an omitted Map/Set argument as the `undefined` key/value instead of a
no-op: Map get/set/has/delete and Set has/add/delete read the argument
directly (GetElement already yields undefined for out-of-range), so
`m.set("k")`, `m.get()`, and `s.add()` behave per spec.
- Move the Set canonicalization invariant into the store via AddSetMember, so
the element is stored as both key and value without callers pre-canonicalizing.
- Make Set difference/intersection/isSubsetOf/isDisjointFrom iterate a snapshot
taken before any set-like `has` callback runs (ES2026 §24.2.4): elements
appended during a mutating callback are no longer visited; the live ones
re-check membership so elements deleted during a callback are skipped.
Regression tests added for the argument handling and the mutating-callback
set operations, plus a Pascal test for AddSetMember.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
coderabbitai
Bot
added
bug
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/units/Goccia.Values.SetValue.pas`:
- Around line 119-133: SnapshotSetItems currently captures only TGocciaValue
data, so membership checks later in intersection, isSubsetOf, and isDisjointFrom
can mistake a delete-then-readd item for the original snapshot entry. Update
SnapshotSetItems and the related snapshot-using branches to preserve a stable
entry identity or physical slot reference from TGocciaSetValue rather than raw
values, and then compare against that identity when iterating so re-added values
are not treated as still-live snapshot members.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b55dc2f0-4c10-42b0-b6d3-ed85f55645b6
📒 Files selected for processing (11)
source/units/Goccia.Builtins.Globals.passource/units/Goccia.Evaluator.Comparison.passource/units/Goccia.REPL.Formatter.passource/units/Goccia.Values.MapValue.passource/units/Goccia.Values.OrderedValueMap.Test.passource/units/Goccia.Values.OrderedValueMap.passource/units/Goccia.Values.SetValue.pastests/built-ins/Map/prototype/set.jstests/built-ins/Set/prototype/add.jstests/built-ins/Set/prototype/difference.jstests/built-ins/Set/prototype/intersection.js
🚧 Files skipped from review as they are similar to previous changes (7)
- source/units/Goccia.Builtins.Globals.pas
- source/units/Goccia.REPL.Formatter.pas
- source/units/Goccia.Values.OrderedValueMap.pas
- tests/built-ins/Map/prototype/set.js
- source/units/Goccia.Evaluator.Comparison.pas
- source/units/Goccia.Values.OrderedValueMap.Test.pas
- source/units/Goccia.Values.MapValue.pas
…nded iteration The earlier snapshot+ContainsValue recheck for intersection/isSubsetOf/ isDisjointFrom could not distinguish a still-live original member from a delete-then-readd of the same value during a mutating `has` callback — membership flips back to true, so the re-added entry was wrongly visited. Replace it with bounded iteration over the original physical slots: capture the store's slot count before the callbacks (EntrySlotCount), retain the store so compaction cannot renumber slots, and walk with NextEntryBounded. Tombstones below the bound are skipped, appended members land at slots >= the bound and are ignored, and a delete-then-readd lands past the bound — matching ES2026 §24.2.4 exactly. `difference` keeps the value snapshot because the spec builds its result from a copy of O.[[SetData]]. Adds EntrySlotCount/NextEntryBounded to the shared TOrderedMap, a Pascal test for them, and a JS regression test for the delete-then-readd case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
coderabbitai
Bot
added
internal
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/units/Goccia.Values.SetValue.pas (1)
777-781: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRoot the copied members before invoking
other.has.
Snapshotis only a Pascal dynamic array, butSetRecordHasruns user code. If a callback deletes an unprocessed object fromThisSetand triggers GC, the laterSnapshot[I]can become a stale value. SinceResultSetis already temp-rooted, populate it with the snapshot first, then remove matches.🛡️ Proposed fix
Snapshot := SnapshotSetItems(ThisSet); for I := 0 to High(Snapshot) do - // Step 6a: If e is not in other, keep it. - if not SetRecordHas(OtherRecord, Snapshot[I]) then - ResultSet.AddItem(Snapshot[I]); + ResultSet.AddItem(Snapshot[I]); + for I := 0 to High(Snapshot) do + // Step 6a: If e is in other, remove it from the copied data. + if SetRecordHas(OtherRecord, Snapshot[I]) then + RemoveSetItem(ResultSet, Snapshot[I]);Based on learnings, GC-rooting issues should only be flagged around real safe points such as user-code/runtime transitions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/units/Goccia.Values.SetValue.pas` around lines 777 - 781, The loop in SetValue uses SnapshotSetItems(ThisSet) and then calls SetRecordHas, which can run user code before the snapshot member is copied into a rooted location. To avoid stale Snapshot[I] values if GC runs during SetRecordHas, change the logic in SetValue so the snapshot members are first added to the already temp-rooted ResultSet, and only then remove any items that are present in OtherRecord. Keep the fix localized around SnapshotSetItems, SetRecordHas, and ResultSet.AddItem.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@source/units/Goccia.Values.SetValue.pas`:
- Around line 777-781: The loop in SetValue uses SnapshotSetItems(ThisSet) and
then calls SetRecordHas, which can run user code before the snapshot member is
copied into a rooted location. To avoid stale Snapshot[I] values if GC runs
during SetRecordHas, change the logic in SetValue so the snapshot members are
first added to the already temp-rooted ResultSet, and only then remove any items
that are present in OtherRecord. Keep the fix localized around SnapshotSetItems,
SetRecordHas, and ResultSet.AddItem.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a2160ed4-4a61-4d56-ae0b-c5a78b6b6df1
📒 Files selected for processing (4)
source/shared/OrderedMap.passource/units/Goccia.Values.OrderedValueMap.Test.passource/units/Goccia.Values.SetValue.pastests/built-ins/Set/prototype/intersection.js
🚧 Files skipped from review as they are similar to previous changes (1)
- source/units/Goccia.Values.OrderedValueMap.Test.pas
…allbacks Set.difference's small branch called the set-like `has` (user code) on each snapshot member before copying it into the temp-rooted ResultSet. SnapshotSetItems returns a plain array, which is not a GC root, so a `has` callback that deleted a not-yet-processed member from the receiver and triggered a GC could free that member while it was still referenced only by the snapshot, dangling a later Snapshot[I]. Copy the snapshot into the temp-rooted ResultSet first, then remove the members present in other. The members stay reachable via ResultSet for the duration of the callbacks, and this matches the spec more literally (resultSetData is a copy of O.[[SetData]] that has matching elements removed, §24.2.4.5). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
frostney
added a commit
that referenced
this pull request
Jun 28, 2026
0077 was taken by #890 (SameValueZero-keyed ordered collections), which merged to main first. Renumber the thread-local cleanup registry ADR to 0078. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MapandSetwith a single newTGocciaOrderedValueMap— a SameValueZero-keyed, insertion-ordered hash store and the first production consumer of the generic insertion-orderedTOrderedMap(ADR 0019). It overridesHashKey/KeysEqualwith a hash exactly consistent withIsSameValueZero.get/set/has/deletego from O(n) (a linearIsSameValueZeroscan) to O(1) amortized; build/probe loops from O(n²) to O(n); set-algebra (union/intersection/difference/symmetricDifference/isSubsetOf/isSupersetOf/isDisjointFrom) from O(n·m) to O(n+m). Both Map and Set use the one store (Set stores key = value) — no per-collection subclass and no parallel structure.[[MapData]]model fixed real bugs, each with new regression tests that fail on the old flat-list code:-0keys are now canonicalized to+0(ES2026 §24.5.1 CanonicalizeKeyedCollectionKey).forEach/iterators now visit entries appended during iteration (ES2026 §24.1.3.5 re-reads the length each step).forEachno longer throwsRangeError: Argument out of range.for...ofno longer skips the next entry or leavessizewrong (§24.1.5.1).undefinedkey/value (m.set("k"),m.get(),s.add()) rather than a no-op.CanCompactvirtual onTOrderedMap(defaultTrue, behavior unchanged for existing users) gates it on a per-store live-iterator counter: while any iteration is in flight the store onlyGrows (which preserves indices) and never compacts, then reclaims tombstones once idle. The SameValueZero hash is the load-bearing invariant (an inconsistent hash silently loses entries) and is guarded by a Pascal invariant test.structuredClone, deep-equal (toEqual), and the REPL formatter. The Set operations are bounded to the members present when the operation began (ES2026 §24.2.4):intersection/isSubsetOf/isDisjointFromretain the store and iterate the original physical slots (EntrySlotCount+NextEntryBounded), so ahascallback that appends — or deletes-then-re-adds — a member cannot make it be revisited;differenceuses a value snapshot because the spec builds its result from a copy ofO.[[SetData]]. Set canonicalization moved into the store (AddSetMember) so callers cannot leak-0into the value slot.WeakMap/WeakSetkeepTHashMap; the content-hash vs identity-hash distinction is recorded in the ADR. No wider collections rewrite.for...of, spread,forEach, exhausted orreturn()-closed iterators — always release the gate), matching how engines pin a collection while an iterator references it. A destructor-based release is unsafe here (the GC sweep frees a Map before its iterators viaRecycle→Free, so the destructor would touch freed memory); the correct fix is GC-driven release during the mark phase, tracked as a follow-up and documented in ADR 0077.Testing
hasset operations: appended, deleted, and delete-then-readd); Map/Set re-validated under./build.pas --prod(the-O4/FASTMATH-sensitive path).Goccia.Values.OrderedValueMap.Test.pas(17 cases: hash/equality consistency over a value matrix incl. distinct-instance collapse,-0canonicalization,AddSetMember, tombstone cursor, bounded iteration, iterator-gate counter); all 39 Pascal unit-test binaries pass.benchmarks/collections.jsuses small N (≤50) where O(n²)≈O(n), so no benchmark number is headlined (consistent with the same-runner-benchmark caveat in ADR 0076).🤖 Generated with Claude Code