Merge pull request #1 from dojoengine/ponderingdemocritus/recs-perf-review

perf(recs): O(N) hotspots, nested schemas, diagnostics → @dojoengine/recs 2.1.0

Author: ponderingdemocritus

packages/recs/CHANGELOG.md

@@ -1,5 +1,83 @@
 # Change Log
 
+## 2.1.0 (`@dojoengine/recs`)
+
+Republished as `@dojoengine/recs` from this repository. Backwards-compatible
+with `@dojoengine/recs@2.0.13` at the runtime API level; the Schema type
+widening is additive (every old flat schema still satisfies `Type | Schema`).
+
+### Nested schema support (ported from the previous fork)
+
+- `Schema` is now `{ [key: string]: Type | Schema }` so sub-struct fields
+  like Eternum's `WorldConfig.season_addresses_config` type-check without
+  casts. `ComponentValue` and `Component.values` handle the recursion.
+
+### Query correctness
+
+- `HasValue` / `NotValue` read only the keys present in the partial value,
+  avoiding a full `getComponentValue` materialization. On a 216-field
+  `Resource` component this drops per-check cost from 216 `Map.get` to 1.
+- `valueEquals` recurses into plain objects and arrays. Fixes silent
+  `HasValue(WorldConfig, { season_addresses_config: { … } })` failures on
+  fresh object/array references with identical contents — previously
+  compared by reference and always returned `false`.
+
+### Performance
+
+- `removeOverride` is now O(K_entity) via a per-entity override list
+  (was O(N log N) over all overrides in the world).
+- `hasComponent` and `Component.entities()` cache the first values-Map per
+  component — no more `Object.values(component.values)[0]` allocation per
+  query iteration.
+- `runQuery` no longer defensively spread-copies the entity set per
+  fragment; mutations are collected into `toDelete` / `toAdd` arrays.
+  `getChildEntities` is memoized per `runQuery` call so shared proxy
+  ancestors aren't re-walked.
+- `defineQuery` deduplicates component subscriptions (a fragment list with
+  the same component twice used to double-process every emission) and
+  pre-buckets fragments by component id.
+- `setComponent` skips the previous-value read when
+  `skipUpdateStream: true` (used by bulk-hydration syncs).
+- `createLocalCache` flushes are coalesced via `throttleTime` — a burst
+  of 200 updates does 2 storage writes instead of 200.
+
+### Indexer
+
+- `getValueKey` uses a schema-ordered, delimiter-separated
+  `JSON.stringify(value[k], bigIntReplacer)` key. Fixes collisions like
+  `{x:"1/2",y:"3"}` vs `{x:"1",y:"2/3"}`, and fixes the
+  `TypeError: Do not know how to serialize a BigInt` crash on any indexed
+  component with `Type.BigInt` / `Type.BigIntArray` fields.
+- Empty value-buckets are GC'd on remove so components with many unique
+  values (positions, balances) no longer leak a bucket entry per distinct
+  value forever.
+- No-op re-indexes are skipped when the value didn't actually change
+  between emissions.
+
+### Bug fixes
+
+- `setComponent(OverridableComponent, …)` no longer throws
+  `TypeError: Method Map.prototype.set called on incompatible receiver`;
+  `Map.prototype` methods are bound to the target on the per-key proxy.
+- The per-key `valueProxyHandler.get` honors `null` overrides correctly by
+  checking `key in override` instead of `override[key] != null`.
+
+### New APIs
+
+- `getDiagnostics(world)` — returns `{ entityCount, componentCount,
+components: [{ id, entitiesWithValue, indexerBuckets? }] }`. For
+  long-running-session health metrics.
+- `getIndexerStats(component)` — returns `{ bucketCount }` for indexed
+  components (undefined otherwise).
+
+### Tooling
+
+- 29 benchmarks (`pnpm test:bench`) with committed `baseline.json`.
+  B1–B18 cover generic hot paths, B19–B24 cover Eternum-shape workloads
+  (216-field Resource, 48-array ResourceArrival, BigInt-indexed Guild),
+  B25 is a soak test that asserts no linear-growth leaks across 10 churn
+  cycles, B26 covers nested-schema `HasValue`.
+
 ## 2.2.23
 
 ### Patch Changes

packages/recs/benchmarks/README.md

@@ -0,0 +1,115 @@
+# `recs` benchmarks
+
+Phase 0 baseline benchmarks for the optimization work tracked in
+`~/.claude/plans/build-a-detailed-plan-prancy-wave.md`.
+
+## Run
+
+```bash
+pnpm --filter @latticexyz/recs test:bench
+```
+
+Each case prints a `[BENCH] {...json...}` line with `id`, `name`, `iterations`,
+`totalMs`, `avgMs`, `opsPerSec`, `heapDeltaBytes`. The full set is persisted to
+`baseline.json` in this directory.
+
+## Files
+
+- `baseline.json` — last captured run. **Update with every Phase PR** that
+  improves any benchmark, so reviewers can `git diff` the perf change.
+- `../src/Benchmark.spec.ts` — the suite (one `describe` per group).
+- `../src/test-utils/bench.ts` — `bench(id, name, fn, opts)` helper.
+
+## Benchmarks
+
+| ID                   | Hot path                                                                  | Plan issue                       |
+| -------------------- | ------------------------------------------------------------------------- | -------------------------------- |
+| B1                   | `hasComponent` × 100k                                                     | #8 `Object.values()[0]` per call |
+| B2                   | `Component.entities()` iteration over 100k                                | #8 same                          |
+| B3                   | Indexer add+remove of 10k unique values                                   | #11 empty-bucket leak            |
+| B4                   | `Indexer.getEntitiesWithValue` × 10k, 100 matches each                    | #10 fresh-Set per call           |
+| B5                   | Indexer no-op `setComponent` × 10k                                        | #12 wasted re-index              |
+| B6                   | Indexer key-collision regression (`{x:"1/2",y:"3"}` vs `{x:"1",y:"2/3"}`) | #12 correctness                  |
+| B7-100, B7-1k, B7-5k | `removeOverride` × K, single entity                                       | #1 O(N log N) sort               |
+| B8                   | Overridable `entities()` × 1k calls                                       | #2 fresh-Set alloc               |
+| B9                   | Overridable `values[x].keys()` × 1k calls                                 | #3 fresh-Set + correctness       |
+| B10-10k, B10-100k    | `runQuery` 4 `Has` fragments                                              | #9 defensive copies              |
+| B11                  | `runQuery` `Has` + `HasValue` (non-indexed)                               | #6 O(N·K) scan                   |
+| B12                  | `getChildEntities` depth=4 branch=10 (non-indexed)                        | #5 no memoization                |
+| B13                  | `getChildEntities` depth=4 branch=10 (indexed)                            | reference: indexer path          |
+| B14                  | `defineQuery` proxy, 100 updates on 10k matched set                       | #4 full re-eval                  |
+| B15                  | `defineQuery` same component in 2 fragments, 1k updates                   | #13 double-subscribe             |
+| B16                  | `setComponent` × 100k with `skipUpdateStream: true`                       | #15 wasted prevValue read        |
+| B17                  | `componentValueEquals` × 1M                                               | #18 cleanup                      |
+| B18                  | `createLocalCache` 200 updates on 1k-entity component                     | #7 O(N) serialize per write      |
+
+## Conventions
+
+- Warmup runs equal to `iterations / 10` (or 1) before each measured loop.
+- Heap delta is measured with `process.memoryUsage().heapUsed` before/after; if
+  `--expose-gc` is available, `global.gc()` runs first. Treat as coarse signal.
+- Use `--runInBand` (set in `test:bench` script) so concurrent test workers
+  don't poison timings.
+- B6 also logs `[BENCH-NOTE] B6 indexer key-collisions ...` — expect 0 after
+  Phase 1.
+- B14/B15 log emitted-event counts as `[BENCH-NOTE]` so we can verify the
+  algorithmic improvements (Phase 3 should drop B14 emitted events
+  drastically; Phase 1 dedupe should halve B15).
+
+## Updating the baseline
+
+When a phase PR improves a benchmark:
+
+1. Re-run `pnpm --filter @latticexyz/recs test:bench`.
+2. Commit the updated `baseline.json`.
+3. Paste a before/after table in the PR description quoting the affected `id`s.
+4. Tighten the (loose) regression assertions in `Benchmark.spec.ts` for the
+   metrics you improved.
+
+## Phase 1+2 deltas vs Phase 0 baseline
+
+`darwin / node v20.9.0`. Negative deltas = faster.
+
+| ID       | Hot path                                 | P0 avgMs | P1+P2 avgMs |            Δ |
+| -------- | ---------------------------------------- | -------: | ----------: | -----------: |
+| B1       | `hasComponent` x 100k                    |     0.37 |        0.13 |     **−64%** |
+| B2       | `Component.entities()` iter 100k         |    17.18 |       11.29 |     **−34%** |
+| B3       | Indexer add+remove 10k unique            |    51.95 |       70.70 |   **+36%** ¹ |
+| B4       | Indexer `getEntitiesWithValue` x 10k     |   160.27 |      142.34 |         −11% |
+| B5       | Indexer no-op setComponent x 10k         |    24.28 |       23.39 |          −4% |
+| B7-1k    | `removeOverride` x 1000                  |    20.37 |        5.72 |     **−72%** |
+| B7-5k    | `removeOverride` x 5000                  |   259.91 |       59.47 |     **−77%** |
+| B8       | Overridable `entities()` x 1k            |  2017.95 |     1718.54 |         −15% |
+| B9       | Overridable `keys()` x 1k                |   537.19 |      474.05 |         −12% |
+| B10-10k  | `runQuery` 4 Has on 10k                  |    13.06 |        6.94 |     **−47%** |
+| B10-100k | `runQuery` 4 Has on 100k                 |   156.44 |       88.11 |     **−44%** |
+| B11      | `runQuery` Has + HasValue (non-indexed)  |     8.12 |        7.30 |         −10% |
+| B12      | `getChildEntities` d=4 b=10 (non-idx)    |  8663.57 |     7422.31 |         −14% |
+| B13      | `getChildEntities` d=4 b=10 (indexed)    |     7.86 |        3.67 |     **−53%** |
+| B14      | `defineQuery` proxy, 100 updates / 10k   |  1416.28 |      917.53 |     **−35%** |
+| B15      | `defineQuery` same-component 2 fragments |     7.31 |        4.45 |     **−39%** |
+| B16      | `setComponent` skip-stream x 100k        |   133.93 |       63.26 |     **−53%** |
+| B17      | `componentValueEquals` x 1M              |   180.91 |      134.18 |         −26% |
+| B18      | `createLocalCache` 200 updates / 1k      |   116.30 |        0.32 | **−99.7%** ² |
+
+¹ B3 regressed because Phase 1 now (a) GCs empty buckets via an extra
+`Map.delete` per remove and (b) uses a JSON-stringified key (more allocation
+per write than the old `Object.values().join('/')`). The trade-off is
+correctness — the old key collided across values like `{x:"1/2",y:"3"}` vs
+`{x:"1",y:"2/3"}` and leaked an empty `Set` per distinct value forever.
+
+² B18 is a `throttleTime(leading+trailing)` win: the leading write fires
+synchronously (so the bench's `localStorage` check still passes), and the
+remaining 199 writes in the burst collapse to a single trailing emission
+that fires after the bench window. Real-world: 200 rapid updates → 2
+storage writes instead of 200.
+
+Phase 3 (localized `defineQuery` proxy re-evaluation) is deferred — see plan.
+
+## Notes from Phase 0 baseline run
+
+A Map-proxy bug also surfaced while writing the suite: calling
+`setComponent(Overridable, ...)` directly threw `TypeError: Method
+Map.prototype.set called on incompatible receiver #<Map>`. Existing call sites
+worked around this by setting on the underlying component. Fixed in Phase 1 by
+binding `Map.prototype` methods to the underlying target.