⚠️ No Changeset found

Latest commit: a77dd3f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Binary Sizes

Commit: 7fa6333

Merging this PR will not alter performance

✅ 200 untouched benchmarks
🆕 18 new benchmarks

Performance Changes

_{Comparing kdy1/swc-es-parser-bench-parity (a77dd3f) with main (f70a4b7)}

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

PR Review: expand benchmark corpus without swc_ecma dependencies

Overall

Good goal — decoupling swc_es_parser benchmarks from swc_ecma_parser paths. The test coverage for the no-dependency invariant is a nice addition. A few items worth considering:

1. File duplication (~12 MB of identical files)

The 11 benchmark fixture files are byte-identical copies of the files already in crates/swc_ecma_parser/benches/files/. This doubles the storage for these fixtures in the repo. Since these are checked into git (not gitignored), every clone carries the extra weight.

Alternatives to consider:

• Symlinks — ln -s ../../swc_ecma_parser/benches/files/*.js . would avoid duplication while keeping local paths. include_str! resolves symlinks fine. However, this would violate the "no swc_ecma references" invariant at the filesystem level, so it depends on how strictly that's meant.
• Shared top-level benches/files/ directory that both crates reference.
• Git LFS for these large JS fixtures (especially typescript.js at 171K lines).

If the intent is to keep the crates fully independent (so swc_ecma_parser could be removed someday), then the duplication is a reasonable trade-off — but it's worth documenting that rationale.

2. Benchmark naming inconsistency in parser.rs

The parser benchmark names use "es_parser/bootstrap/..." (e.g., es_parser/bootstrap/angular), while in swc_ecma_parser they use "es/parser/...". The "bootstrap" prefix is misleading — these are parser benchmarks, not bootstrap-related. The lexer benchmarks correctly use "es_parser/lexer/...".

Should be "es_parser/parser/..." for consistency with the lexer naming and with what these benchmarks actually measure.

3. Repetitive benchmark registration

Both lexer.rs and parser.rs have 11 nearly identical c.bench_function(...) blocks that differ only in name and file path. A helper like:

const BENCH_FILES: &[(&str, &str, Syntax)] = &[
    ("colors", include_str!("./files/colors.js"), Syntax::Es(EsSyntax::default())),
    // ...
];

for (name, src, syntax) in BENCH_FILES {
    c.bench_function(&format!("es_parser/lexer/{name}"), |b| {
        bench_lex(b, *syntax, src)
    });
}

This would reduce duplication and make adding future benchmarks easier. The swc_ecma_parser benchmarks have the same pattern, so this isn't a regression — just an improvement opportunity.

4. no_ecma_dependency test — contains_swc_ecma_reference is very broad

The new contains_swc_ecma_reference function matches on the substring "swc_ecma_" anywhere in the source. This could give false positives on:

• Comments explaining the relationship to swc_ecma_parser
• String literals in test assertions

The existing contains_swc_ecma_parser_import function is more precise. Consider making the new check slightly more targeted (e.g., excluding comments), or at least documenting that bench files shouldn't mention swc_ecma_* even in comments.

5. Minor nit

The colors.js benchmark in parser.rs was originally labeled with a comment // Copied from ratel-rust in the swc_ecma_parser version. That context is lost in the new file.

Summary

The core change is sound — expanding the benchmark corpus and removing cross-crate path dependencies. The main concern is the 12 MB of duplicated fixture files. The "bootstrap" naming in parser benchmarks should be fixed. Everything else is minor.

🤖 Generated with Claude Code