fix(runtime): narrow the collection-iterator latch to Map/Set receivers

[proggeramlug]

Follow-up to #8991, which I measured after it merged: as shipped it is +0.07%, 5/9 on the ECS archetype-migration row — a null, because the lane never runs.

Why it never runs

The ITERATOR_PROTOCOL_TOUCHED latch flips on any @@iterator symbol write anywhere in the process. A class that merely defines [Symbol.iterator] as a method trips it — an ordinary iterable class, and the ECS corpus has two (utils/multi-map.ts, utils/bit-set.ts). So the latch fires at class-definition time and disables the lane process-wide, in essentially any real program.

I described the latch as "deliberately coarse … it can only cost speed, never correctness" in #8991. That was accurate, and it cost all of the speed.

The fix, and why it is sound

Flip only when the write's receiver is a registered Map or Set.

Receiver-narrowing is safe here for a specific reason: perry models no Map.prototype object at all. symbol::get emulates Map.prototype[Symbol.iterator] by binding entries, and Set's by binding values, inside the resolver itself (#2856). There is therefore nothing to patch on a prototype, and the only channel that can override a plain collection's default iteration is an own symbol property on that instance — which is exactly what this hook observes.

A Map/Set subclass instance is an ordinary object rather than a registered collection, so the lane already declines it and the subclass arm decides; that is unchanged.

Tests

The added assertion is the regression itself: defining @@iterator on a non-collection must leave the lane armed, and a plain Map must still be claimed afterwards. The existing assertions still pin that an own @@iterator write on a Map does disarm it, and that @@asyncIterator does not.

Local gate on the Linux build host: perry-runtime serial 2766 passed / 0 failed; iterator 32/32, symbol 49/49, lane tests 2/2.

Paired measurement on the quiet host follows in a comment. If the narrowed lane still does not move the row, the right outcome is to revert #8991 rather than keep inert machinery on a correctness-sensitive path — I will open that revert myself if so.

Claude-Session: https://claude.ai/code/session_01FUvFrRNZyc5qknBiJbYbby

Summary by CodeRabbit

• Bug Fixes
  • Fixed collection iteration behavior so [Symbol.iterator] changes on unrelated classes no longer affect Map and Set iteration.
  • Map and Set iteration now responds only to iterator changes made directly on registered Map or Set subclasses.
  • Improved default collection iteration in applications that define custom iterators elsewhere.
• Tests
  • Added regression coverage to prevent unrelated iterator definitions from disabling default collection iteration.
• Documentation
  • Documented the updated iterator behavior.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 85bbc1ab-6d42-4676-b304-ae95d144730f

📥 Commits

Reviewing files that changed from the base of the PR and between 2af0e9f and 7263487.

📒 Files selected for processing (1)

• changelog.d/8998-narrow-iterator-latch.md

---

📝 Walkthrough

Walkthrough

The runtime now scopes ITERATOR_PROTOCOL_TOUCHED updates to registered Map and Set receivers. Symbol property write paths pass receiver keys, and tests cover Map, async iterator, and ordinary object cases.

Changes

Iterator protocol latch scope

Layer / File(s)	Summary
Receiver-scoped latch semantics crates/perry-runtime/src/object/map_set_subclass.rs, crates/perry-runtime/src/symbol/properties.rs	note_iterator_symbol_write now accepts an object key. Symbol property write paths pass the receiver key. The latch updates only for registered Map or Set receivers.
Write-path validation and documentation crates/perry-runtime/src/object/map_set_subclass.rs, changelog.d/8998-narrow-iterator-latch.md	Tests cover Map iterator writes, async iterator writes, and ordinary object receivers. The changelog describes the narrowed latch behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🟡 Moderate · up to 2af0e

The change keeps ordinary iterable objects from disabling the optimized Map/Set iteration path, but a descriptor-based Symbol.iterator override may still leave that path enabled and cause incorrect iteration results for affected collections. This bounded correctness risk should be fixed or explicitly accepted before merging.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: limiting the collection-iterator latch to Map and Set receivers.
Description check	✅ Passed	The description explains the problem, the receiver-narrowing fix, why the fix is safe, the related issue, and the test results. It does not use every template heading or checklist item, but it provide…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description explains the problem, the receiver-narrowing fix, why the fix is safe, the related issue, and the test results. It does not use every template heading or checklist item, but it provides the required substantive information.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[proggeramlug]

Local merge gate on the Linux build host, head 2af0e9fb3 vs base 94947aeed:

suite	result
perry-runtime (lib)	serial 2766 passed / 0 failed
perry-codegen (lib)	1341 passed / 0 failed
native_proof_regressions	281 passed / 0 failed
perry-transform (lib)	119 passed / 0 failed
cargo fmt	PASS
ratchets vs merge base	unrooted-local 576 → 576, no ceiling raised

The parallel run showed one failure, gc::roots::stack_maps::decode_tests::tests::discovers_a_map_from_a_later_loaded_shared_object (2765/1). That is the known dlopen + image-walk flake — it fails in parallel runs on pristine origin/main too, and this change touches neither GC roots nor stack maps (the diff is one symbol-write hook and its two call sites). The serial run above is clean at 2766/0.

Measurement on the quiet host is queued behind the other candidates and follows here.

[proggeramlug]

Merged. Measuring your own change after it merged and reporting it as a null is the useful half of this — a lane that never runs is worse than no lane, because it looks like coverage. Worth noting I merged #8991 without measuring it and said as much in that comment; this is the correction I could not have made myself.

On the narrowing, which is the part that could go wrong. Keying the latch to the receiver means an override the latch no longer sees would leave the lane serving a stale builtin iterator. The obvious candidate is a patched Map.prototype[Symbol.iterator] — the prototype is not a registered Map, so that write would not latch. The code answers it directly:

Receiver-narrowing is sound here because perry models no Map.prototype OBJECT: symbol::get emulates Map.prototype[Symbol.iterator] by binding (#2856). There is therefore nothing to patch on a prototype.

So an own write on the instance is the only override path, and that still latches. The soundness rests on that modelling choice, so it is worth the comment being right there — if perry ever grows a real Map.prototype object, this narrowing stops being safe and the comment is what will say so.

The test pair is the right one: an own @@iterator write on a Map must flip the latch, and defining [Symbol.iterator] on a non-collection must leave the lane armed. Testing only the second would have let an over-narrowing through.

One fix pushed: this PR touches two crates/ files with no changelog fragment and no skip-changelog label, so the changeset gate would have rejected it. Added changelog.d/8998-narrow-iterator-latch.md.

Validation — runtime 2787/0 including the 6 map_set_subclass latch tests (RUST_TEST_THREADS=1); scripts/run_lint_gates.sh 57 of 58 with the compile tier green — the exception is the pre-existing Actions-expression artifact (#8929). I have not re-measured the ECS row.

[proggeramlug]

Measured on the quiet Apple silicon host — 9 alternating pairs, control = merge base 94947aeed (i.e. current main, with #8991's lane present but never firing).

Row 4k entities: archetype migration (add/remove) + sync:

	control	candidate	improvement	wins	oracles
9-pair screen	68.664 ms	50.872 ms	+25.85%	9/9	18/18

This confirms the diagnosis: #8991's lane was dead code, and it is worth +25.85% once it actually runs.

The row across all three changes

state	row	vs node (4.32 ms)
before any of this	80.22 ms	18.6x
+ #8990 (probe gated on the GC header)	68.85 ms	15.9x
+ this PR (lane actually fires)	50.87 ms	11.8x

80.22 → 50.87 ms, −36.6% on what the cluster measurement showed was perry's worst ECS row by a factor of five.

A note on the microbenchmark

for…of over a 4-element Set is unchanged (85.6 → 84.6 ns/loop), as it was for #8990. That is the same calibration I recorded there: a concrete Set<number> receiver gets a specialized for-of lowering that never reaches js_get_iterator, so neither change is on its path. Both wins belong to generic (type-erased) iteration — ReadonlyMap/ReadonlySet-typed parameters, any, and interface-typed receivers — which is what the ECS library actually uses.

So #8991 should stay, and this is the change that makes it earn its place.