feat: compile() — the ergonomic .each path, codegen'd to bindColumns speed#86
Conversation
…speed `query.compile(body)` reads an `.each`-style callback's own source, rewrites each `e.<comp>.<field>` to direct typed-array column indexing, and codegens a specialized per-archetype loop — so the readable accessor syntax lands near eachChunk/bindColumns (~1.6 ns/entity, ~6x faster than the proxy .each) while you keep writing `e.position.x += e.velocity.dx * ctx.dt`. Unlike bindColumns it PRESERVES reactivity: a written component is recorded in the write log exactly as the accessor setter would (component-granular, gated on tracking.active, so free when no .changed/observer consumer exists). The tracking branch is hoisted OUT of the loop, so the common no-consumer path is a clean kernel V8 compiles like bindColumns. Pure speedup that can never change results: the analyzer is conservative and falls back to the unchanged proxy .each for anything it can't prove safe — non-straight-line bodies, strings/comments/regex literals, destructuring-assign, non-numeric-scalar fields (vec/bool/eid/bigint/rich), non-required components, any `e` use beyond `e.comp.field`, per-row ctx writes, row-filtered queries, `__`-prefixed locals (no generated-name collision), and runtimes that block `new Function`. Every generated identifier is `__`-prefixed and the body is rejected if it contains `__`, so a user local can never silently shadow one. A scratch pre-flight runs the runner once on throwaway arrays, so a body that closes over an outer variable falls back instead of crashing the first frame. Correctness is property-tested byte-identical to proxy .each under random spawn/despawn/write/growth churn, with and without a .changed consumer. - new: packages/core/src/query/compile-each.ts (the source transform) - LiveQuery.compile() + public Query/LooseQuery.compile type - bench bucket (ecsia compile) + CI regression-ratio lane (≤1.0x bitECS) - docs: performance.md "Compile the ergonomic path" section - bundle budget ratcheted (+~2.3KB gz: compile lives in the query kernel)
Greptile SummaryAdds
Confidence Score: 4/5Safe to merge; the only behavioral issue is a narrow false positive in write detection that causes over-reporting of The compiler is well-designed with conservative fallback semantics — any analysis failure degrades to the proxy, never to wrong data. Three small gaps in packages/core/src/query/compile-each.ts — the three findings above are all in the analyzer; live-query.ts and the test files look solid. Important Files Changed
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/core/src/query/compile-each.ts:208
The `preInc` check tests whether the full block text up to this match ends with `++\s*`. This can fire falsely across statement boundaries. For a block like `e.position.x++
e.position.x` (the second access is just a read), `block.slice(0, m.index)` is `e.position.x++
`, which ends with `++
` — `
` satisfies `\s*` — so `preInc` fires and `position` is falsely added to `writtenIds`. This causes spurious `.changed()` notifications for bodies that post-increment one access and then read the same field later as a standalone expression.
```suggestion
// Only consider the text between the previous match end (or start of block) and this match start.
const preInc = /(?:\+\+|--)\s*$/.test(block.slice(last, m.index)) // prefix ++/-- (any whitespace)
```
### Issue 2 of 3
packages/core/src/query/compile-each.ts:171-174
The `idByName` map is populated inside the access loop but never read after it. This is dead code, likely a leftover from an earlier version of the factory-source builder that referenced component ids by name.
```suggestion
const specs: EachViewSpec[] = []
const specIndex = new Map<string, number>() // "comp.field" -> views[] index (dedups repeat accesses)
const writtenIds = new Set<number>()
```
### Issue 3 of 3
packages/core/src/query/compile-each.ts:79
**Regex literal at block start not caught**
`REGEX_LITERAL` only fires when `/` is preceded by an operator character, so a regex literal at the very start of a block (`/e\.position\.x/ .test(...)`) would not be caught. In that contrived case `accessRe` would match the literal text `e.position.x` inside the regex pattern and corrupt the generated source. A simple additional guard — bail if the trimmed block starts with `/` — closes this gap.
Reviews (1): Last reviewed commit: "feat: compile() — the ergonomic .each pa..." | Re-trigger Greptile |
| // written. Straight-line bodies guarantee the write always runs, so the after-body trackWrite is exact. | ||
| const rest = block.slice(accessRe.lastIndex).replace(/^\s+/, '') | ||
| const postInc = rest.startsWith('++') || rest.startsWith('--') | ||
| const preInc = /(?:\+\+|--)\s*$/.test(block.slice(0, m.index)) // prefix ++/-- (any whitespace) |
There was a problem hiding this comment.
The
preInc check tests whether the full block text up to this match ends with ++\s*. This can fire falsely across statement boundaries. For a block like e.position.x++ e.position.x (the second access is just a read), block.slice(0, m.index) is e.position.x++ , which ends with ++ — satisfies \s* — so preInc fires and position is falsely added to writtenIds. This causes spurious .changed() notifications for bodies that post-increment one access and then read the same field later as a standalone expression.
| const preInc = /(?:\+\+|--)\s*$/.test(block.slice(0, m.index)) // prefix ++/-- (any whitespace) | |
| // Only consider the text between the previous match end (or start of block) and this match start. | |
| const preInc = /(?:\+\+|--)\s*$/.test(block.slice(last, m.index)) // prefix ++/-- (any whitespace) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/query/compile-each.ts
Line: 208
Comment:
The `preInc` check tests whether the full block text up to this match ends with `++\s*`. This can fire falsely across statement boundaries. For a block like `e.position.x++
e.position.x` (the second access is just a read), `block.slice(0, m.index)` is `e.position.x++
`, which ends with `++
` — `
` satisfies `\s*` — so `preInc` fires and `position` is falsely added to `writtenIds`. This causes spurious `.changed()` notifications for bodies that post-increment one access and then read the same field later as a standalone expression.
```suggestion
// Only consider the text between the previous match end (or start of block) and this match start.
const preInc = /(?:\+\+|--)\s*$/.test(block.slice(last, m.index)) // prefix ++/-- (any whitespace)
```
How can I resolve this? If you propose a fix, please make it concise.| const specs: EachViewSpec[] = [] | ||
| const specIndex = new Map<string, number>() // "comp.field" -> views[] index (dedups repeat accesses) | ||
| const writtenIds = new Set<number>() | ||
| const idByName = new Map<string, number>() |
There was a problem hiding this comment.
The
idByName map is populated inside the access loop but never read after it. This is dead code, likely a leftover from an earlier version of the factory-source builder that referenced component ids by name.
| const specs: EachViewSpec[] = [] | |
| const specIndex = new Map<string, number>() // "comp.field" -> views[] index (dedups repeat accesses) | |
| const writtenIds = new Set<number>() | |
| const idByName = new Map<string, number>() | |
| const specs: EachViewSpec[] = [] | |
| const specIndex = new Map<string, number>() // "comp.field" -> views[] index (dedups repeat accesses) | |
| const writtenIds = new Set<number>() |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/query/compile-each.ts
Line: 171-174
Comment:
The `idByName` map is populated inside the access loop but never read after it. This is dead code, likely a leftover from an earlier version of the factory-source builder that referenced component ids by name.
```suggestion
const specs: EachViewSpec[] = []
const specIndex = new Map<string, number>() // "comp.field" -> views[] index (dedups repeat accesses)
const writtenIds = new Set<number>()
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| // A regex literal could spell a real `e.comp.field` that string-rewriting would corrupt. A `/` right after | ||
| // an operator/open-bracket starts a regex (division's `/` follows a value: identifier/`)`/`]`/number), so | ||
| // this flags regex literals without bailing on division. | ||
| const REGEX_LITERAL = /[=(,:[!&|?{;+\-*%<>~^]\s*\// |
There was a problem hiding this comment.
Regex literal at block start not caught
REGEX_LITERAL only fires when / is preceded by an operator character, so a regex literal at the very start of a block (/e\.position\.x/ .test(...)) would not be caught. In that contrived case accessRe would match the literal text e.position.x inside the regex pattern and corrupt the generated source. A simple additional guard — bail if the trimmed block starts with / — closes this gap.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/query/compile-each.ts
Line: 79
Comment:
**Regex literal at block start not caught**
`REGEX_LITERAL` only fires when `/` is preceded by an operator character, so a regex literal at the very start of a block (`/e\.position\.x/ .test(...)`) would not be caught. In that contrived case `accessRe` would match the literal text `e.position.x` inside the regex pattern and corrupt the generated source. A simple additional guard — bail if the trimmed block starts with `/` — closes this gap.
How can I resolve this? If you propose a fix, please make it concise.
What
Adds
query.compile(body)— a source-rewriting query compiler for the ergonomic iteration path. It reads an.each-style callback's own.toString(), rewrites everye.<component>.<field>into direct typed-array column indexing, and codegens a specialized per-archetype loop (reusing the PR1new Functionmechanism). The readable accessor syntax now lands neareachChunk/bindColumnsinstead of paying the per-row proxy tax.Measured (50k integrate): compiled ~1.6 ns/entity vs proxy
.each~10 — a ~6× speedup, matchingbindColumns. CI regression lane assertscompilestays ≤ 1.0× a same-run bitECS control.Why it's safe (never changes results)
.eachfor anything it can't prove safe (control flow /?/&&/return/ loops / nested fns, strings / comments / regex literals, destructuring-assignment, non-numeric-scalar fieldsvec/bool/eid/bigint/rich, non-required components, anyeuse beyonde.comp.field, per-rowctxwrites, row-filtered queries, runtimes that blocknew Function).__-prefixed and the body is rejected if it contains__, so a user local (v0,trackWrite, …) can never silently shadow a generated one.ReferenceError) → proxy fallback, instead of crashing the first real frame.tracking.active), with the branch hoisted out of the loop so the no-consumer path is a clean kernel..changed()/observers see compiled writes identically..eachunder random spawn/despawn/write/growth churn, with and without a.changedconsumer.An adversarial review caught the generated-name-collision (silent shadow), destructuring-write-miss, and prefix-
++write-detection gaps — all fixed and regression-tested here.Changes
packages/core/src/query/compile-each.ts— the analyzer/transformLiveQuery.compile()+ publicQuery/LooseQuery.compiletypeecsia compile) wired intobench:report+ the CI regression-ratio laneperformance.md"Compile the ergonomic path" sectioncompileis a query-kernel method, not tree-shakeable)Test plan
pnpm test(1269 passed)pnpm typecheck:tests/typecheck:extraspnpm docs:checkpnpm sizeBENCH_REGRESSION=1regression lane (4/4 ratios under ceiling)