Conversation
…imize lexer perf (~34% faster)
… skip dollar/comma passes
tiye
force-pushed
the
perf
branch
2 times, most recently
from
a838967 to
e3919cc
Compare
There was a problem hiding this comment.
Pull request overview
This PR focuses on improving parser performance by eliminating intermediate allocations during lex/parse and by merging the $ and , tree-transform passes, while also adding a simple benchmark harness and updating project tooling/CI for Yarn 4.
Changes:
- Replace the previous token-buffer-based pipeline with a single-pass
lexAndBuild()parser insrc/index.ts. - Optimize tree transforms in
src/tree.ts(no-slice helpers + new combinedresolveDollarComma) and add equivalence tests. - Add benchmarking support (
src/bench.ts,tsconfig.bench.json,yarn bench) and update Yarn/CI/docs.
Reviewed changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/index.ts |
New single-pass lexer+builder and conditional $/, transform invocation. |
src/tree.ts |
No-slice comma/dollar helpers + new combined resolveDollarComma pass. |
src/parser.test.ts |
Adds equivalence tests to validate combined $/, behavior. |
src/bench.ts |
Adds a benchmark runner for local perf testing. |
tsconfig.bench.json |
Adds a bench-specific TS config (Node types, ES2022 target). |
package.json |
Adds bench script, bumps version, updates dev deps, pins Yarn via packageManager. |
Agents.md |
Adds contributor/agent perf/testing/tooling notes. |
.yarnrc.yml |
Configures Yarn to use node-modules linker. |
.gitignore |
Ignores bench artifacts and Yarn state files. |
.github/workflows/upload.yaml |
Updates Node setup + Yarn caching/immutable installs + uses yarn compile. |
.github/workflows/npm-publish.yaml |
Updates Node setup + Yarn caching/immutable installs + uses yarn compile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ```bash | ||
| yarn test # run all 22 Jest tests | ||
| yarn bench # benchmark on bundled test fixtures (×20, ~16 KB) | ||
| BENCH_FILE=/absolute/path/to/large.cirru yarn bench # benchmark on a real file |
There was a problem hiding this comment.
This doc line hard-codes "run all 22 Jest tests", but src/parser.test.ts currently defines more than 22 tests (and later in this document it also mentions 27). Consider avoiding a specific count here (or update it) so it doesn't go stale.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: tiye <449224+tiye@users.noreply.github.com>
docs: remove stale hard-coded Jest test count from Agents.md
No description provided.