perf(json): index large replacer key sets

[mizchi]

Prepare a lookup Map for Replacer::keep and Replacer::exclude when more than eight keys are supplied, avoiding an O(properties × keys) linear scan. Small key lists retain the existing array path.

The index is rebuilt once at the start of each stringify or transform, so mutations to the source key array remain observable between calls. The existing Replacer debug output is also preserved.

Native benchmark (Json::stringify, 5,000 object properties):

keep keys	before	after
2,500	11.83 ms	307.97 µs
3	54.63 µs	54.13 µs

Validation: 216 JSON tests pass on wasm, wasm-gc, JS, and native; moon check --target all and moon info pass.

[Copilot]

Pull request overview

This PR optimizes @json.Replacer::keep / ::exclude for large key lists by building a lookup Map (when > 8 keys) once per Json::stringify / Json::transform call, avoiding repeated linear scans across object properties. It preserves the existing behavior for small key lists and keeps Replacer’s debug output stable.

Changes:

• Refactors Replacer to store a ReplacerKind and introduces Replacer::prepare() to produce an efficient (String, Json) -> Json? function per call site.
• Updates Json::stringify and Json::transform to use the prepared replacer function, ensuring indexing happens once per call.
• Adds a regression test to ensure key-array mutations remain observable between successive stringify calls, plus a targeted benchmark for large keep key sets.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
json/json.mbt	Refactors Replacer internals and uses a prepared/indexed replacer in stringify and transform.
json/json_test.mbt	Adds a test asserting large-key-path behavior still observes key-array mutations between calls.
json/replacer_bench_test.mbt	Adds benchmarks covering Replacer::keep with large (2500) vs small (3) key sets.
json/pkg.generated.mbti	Updates the generated public interface to reflect the Debug impl change for Replacer.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[bobzhang]

Thanks for digging into this — the O(properties × keys) scan is a real problem and indexing it is the right instinct. But I benchmarked the patch as it stands and it regresses badly in a case the current bench suite doesn't cover, plus there are a few smaller things. Details below.

The index is rebuilt on every call, which is a 14× regression for small documents

Replacer::prepare runs at the start of every stringify/transform, so a caller that stringifies small documents with a large keep-list now pays O(keys) map inserts — each with a to_owned() String allocation — instead of O(properties × keys) cheap StringView comparisons.

Measured on native, Json::stringify with 2,500 keep keys:

object fields	main	this PR
5	7.39 µs	105.21 µs	14.2× slower
50	93.26 µs	108.81 µs	1.17× slower
5,000	24.30 ms	469.00 µs	51× faster

Break-even is around 55–60 properties. The bench suite here only covers the 5,000-field case, so CI would never catch this.

Two things are worth separating out. The <= 8 threshold is on the wrong quantity — the trade-off depends on the ratio of properties looked up to keys indexed, not on the key count alone. And the deeper issue is that the index can't be cached, because keep/exclude hold a live ArrayView and the patch deliberately keeps mutations to it observable.

The hand-written Debug impl describes a field that no longer exists

pub impl @debug.Debug for Replacer with fn to_repr(_) {
  @debug.Repr::record(Map([("f", @debug.Repr::literal("<function: ...>"))]))
}

The struct's field is kind now, not f. This exists only to keep the snapshot in json/types_test.mbt:53 green, and it churns pkg.generated.mbti (} derive(@debug.Debug) becomes a separate pub impl line).

Putting derive(@debug.Debug) on both Replacer and ReplacerKind works — it prints { kind: Custom(<function: ...>) }, which is truthful and more informative, and leaves the .mbti byte-identical to main. The only cost is updating that one snapshot.

Smaller things

• The Map-building loop is duplicated verbatim in the Keep and Exclude arms of prepare. Worth factoring out.
• 8 is a bare magic number — no named constant, no comment on where it came from.
• key.to_owned() copies every key. StringView has impl Hash in builtin, so Map[StringView, Unit] avoids the allocation entirely.
• The bench suite should include the small-document case so a future regression here is caught.

Where this leaves us

I've opened #4157 with an implementation that keeps your indexing idea but caches the index in the Replacer itself rather than rebuilding it per call. That removes the regression in every pattern I measured and is also less code — no prepare, no PreparedReplacer, no transform_with:

pattern	main	this PR	#4157
reuse replacer, 2500 keys / 5 fields	7.39 µs	105.21 µs	0.40 µs
reuse, 2500 / 50	93.26 µs	108.81 µs	3.13 µs
reuse, 2500 / 5000	24.30 ms	469.00 µs	397.39 µs
construct per call, 2500 / 5	7.39 µs	105.21 µs	7.43 µs
3 keys / 5000 fields	93.89 µs	92.40 µs	95.96 µs

The one behaviour change is that mutating the key array after the replacer has been used heavily is no longer observed. That aliasing was an accident of closing over the view rather than an intended contract, and the test added here would cement it — so that PR documents the key set as being read when the replacer is used and drops the guarantee.

Since it supersedes this one I'd suggest we land that instead, but the diagnosis and the benchmark table above are all from your work here — thank you for finding it.