Skip to content

perf(parser): skip parse_call_expression_rest when no call follows#23063

Merged
graphite-app[bot] merged 1 commit into
mainfrom
perf/parser-skip-call-rest
Jun 7, 2026
Merged

perf(parser): skip parse_call_expression_rest when no call follows#23063
graphite-app[bot] merged 1 commit into
mainfrom
perf/parser-skip-call-rest

Conversation

@Boshen

@Boshen Boshen commented Jun 7, 2026

Copy link
Copy Markdown
Member

What

In parse_lhs_expression_or_higher, skip parse_call_expression_rest unless the current token is ( or ?., and inline the single-use parse_member_expression_or_higher so the LeftHandSideExpression parse reads top-to-bottom (primarymember_expressionlhs).

Why

parse_member_expression_rest already consumes every MemberExpression continuation (., ?., [], tagged template, TS ! / <T>). The only ways to extend a fully-parsed MemberExpression into an LHS are Arguments (() or an OptionalChain (?.) — the ./[]/template extensions live under CallExpression, so they only apply after a call.

Previously parse_lhs_expression_or_higher always called parse_call_expression_rest, whose loop re-runs parse_member_expression_rest (the #1 hotspot per profiling) on every primary before checking for ?./(. So every leaf expression paid for a redundant member-rest scan plus call-rest overhead. A sample profile showed the expression LHS chain at ~50% of parse time.

Correctness

Behavior-preserving. When the current token is neither ( nor ?., parse_call_expression_rest is a no-op. a<T>() is unaffected (<T> consumed in member-rest, leaving cur == (). The inlined helper had a single caller and identical span.

estree (AST + spans + tokens) byte-identical to main; allocs_parser.snap byte-identical (pure work reduction, no memory effect).

Reference

typescript-go (and TS's own JS parser) use the same shape with the same redundant member-rest re-entry — the reference leaves this unoptimized. Grammar excerpt + parser.go cross-reference in the PR comment.

Benchmark

Interleaved A/B, two saved release binaries, 2.5 MB expression-dense file, 12 alternating reps:

  • main: ~22.6 ms/parse (median)
  • this branch: ~19.7 ms/parse (median)

13% faster on expression-heavy code; every branch run beat every main run. Neutral on mixed workloads (saving is per-primary).

@github-actions github-actions Bot added the A-parser Area - Parser label Jun 7, 2026
@codspeed-hq

codspeed-hq Bot commented Jun 7, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 4.45%

⚡ 4 improved benchmarks
✅ 53 untouched benchmarks
⏩ 9 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation parser[kitchen-sink.tsx] 22.9 ms 21.7 ms +5.62%
Simulation parser[binder.ts] 3.2 ms 3.1 ms +4.48%
Simulation parser[react.development.js] 1.3 ms 1.2 ms +4.14%
Simulation parser[App.tsx] 8 ms 7.7 ms +3.59%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing perf/parser-skip-call-rest (6e7903f) with main (d16f456)

Open in CodSpeed

Footnotes

  1. 9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@Boshen Boshen force-pushed the perf/parser-skip-call-rest branch from 959239b to 416db10 Compare June 7, 2026 10:34
@Boshen

Boshen commented Jun 7, 2026

Copy link
Copy Markdown
Member Author

Why the ( / ?. guard is spec-faithful (not a heuristic)

ECMAScript grammar for LeftHandSideExpression:

LeftHandSideExpression : NewExpression | CallExpression | OptionalExpression

CallExpression :
    MemberExpression Arguments        // (...)   ← first way to extend a MemberExpression
    CallExpression Arguments
    CallExpression [ Expression ]
    CallExpression . IdentifierName
    CallExpression TemplateLiteral
    super... | import...

OptionalExpression :
    MemberExpression OptionalChain    // ?. ...  ← the other way
    CallExpression  OptionalChain
    OptionalExpression OptionalChain

The . / [] / TemplateLiteral extensions live under CallExpression, so they only apply after an Arguments. A fully-parsed MemberExpression can therefore only be extended into an LHS by Arguments (() or OptionalChain (?.). parse_member_expression_or_higher already consumes every MemberExpression continuation (., ?., [], tagged template, and the TS ! / <T> extensions), so guarding parse_call_expression_rest on LParen | QuestionDot is exactly that grammar boundary.

Reference cross-check (typescript-go)

The official TS Go port has the identical shape and the same redundancy this PR removes — parseLeftHandSideExpressionOrHigher ends with an unconditional parseCallExpressionRest, whose loop re-runs parseMemberExpressionRest on every primary before checking for ?. / (:

// parseLeftHandSideExpressionOrHigher
return p.parseCallExpressionRest(pos, expression)

// parseCallExpressionRest
for {
    expression = p.parseMemberExpressionRest(pos, expression, true) // redundant re-scan
    questionDotToken := p.parseOptionalToken(KindQuestionDotToken)
    ...
    if typeArguments != nil || p.token == KindOpenParenToken { /* call */; continue }
    break
}

So TS-go (and TS's own JS parser) confirm the member/call boundary is precisely ( / ?., and leave the redundant re-scan unoptimized. The a<T>() instantiation-then-call case is unaffected: <T> is consumed inside member-rest, leaving cur == (, so the guard still routes it into parse_call_expression_rest. This is why estree output is byte-identical.

@Boshen Boshen force-pushed the perf/parser-skip-call-rest branch 3 times, most recently from 0c14dbe to 6e7903f Compare June 7, 2026 10:54
@Boshen

Boshen commented Jun 7, 2026

Copy link
Copy Markdown
Member Author

Side effect: ~halves system allocations on TypeScript files

allocs_parser.snap (committed in this PR) vs main:

file Sys allocs (main → PR)
checker.ts 5303 → 2725 (-49%)
kitchen-sink.tsx 5027 → 2991 (-41%)
binder.ts 281 → 141 (-50%)
App.tsx 632 → 520 (-18%)
antd.js / pdf.mjs / *.jsx unchanged

Root cause: the removed redundant parse_member_expression_rest re-scan re-ran the speculative TS parse_type_arguments_in_expression on every < / << token. When the < is not type arguments (e.g. a comparison a < b), that speculation parses a type and then expect(KindRAngle) fails, which builds an OxcDiagnostic (heap: message String + boxed diagnostic) before rewind discards it — the allocation is already counted. This happened twice per failing < (member-rest in parse_member_expression_or_higher, then again in parse_call_expression_rest's loop); the guard removes the second attempt.

Matches the data: TS-only (no type args in JS → JS files unchanged), ~halved (was done twice), and sys drops ~2× arena (each failed < ≈ 2 heap allocs for the diagnostic + ~1 arena type node).

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Jun 7, 2026

Boshen commented Jun 7, 2026

Copy link
Copy Markdown
Member Author

Merge activity

…23063)

## What

In `parse_lhs_expression_or_higher`, skip `parse_call_expression_rest` unless the current token is `(` or `?.`, and inline the single-use `parse_member_expression_or_higher` so the `LeftHandSideExpression` parse reads top-to-bottom (`primary` → `member_expression` → `lhs`).

## Why

`parse_member_expression_rest` already consumes every `MemberExpression` continuation (`.`, `?.`, `[]`, tagged template, TS `!` / `<T>`). The only ways to extend a fully-parsed `MemberExpression` into an LHS are `Arguments` (`(`) or an `OptionalChain` (`?.`) — the `.`/`[]`/template extensions live under `CallExpression`, so they only apply after a call.

Previously `parse_lhs_expression_or_higher` always called `parse_call_expression_rest`, whose loop re-runs `parse_member_expression_rest` (the #1 hotspot per profiling) on every primary before checking for `?.`/`(`. So every leaf expression paid for a redundant member-rest scan plus call-rest overhead. A `sample` profile showed the expression LHS chain at ~50% of parse time.

## Correctness

Behavior-preserving. When the current token is neither `(` nor `?.`, `parse_call_expression_rest` is a no-op. `a<T>()` is unaffected (`<T>` consumed in member-rest, leaving `cur == (`). The inlined helper had a single caller and identical `span`.

estree (AST + spans + tokens) byte-identical to `main`; `allocs_parser.snap` byte-identical (pure work reduction, no memory effect).

## Reference

typescript-go (and TS's own JS parser) use the same shape with the same redundant member-rest re-entry — the reference leaves this unoptimized. Grammar excerpt + `parser.go` cross-reference in the PR comment.

## Benchmark

Interleaved A/B, two saved release binaries, 2.5 MB expression-dense file, 12 alternating reps:

- `main`: ~22.6 ms/parse (median)
- this branch: ~19.7 ms/parse (median)

≈ **13% faster on expression-heavy code**; every branch run beat every main run. Neutral on mixed workloads (saving is per-primary).
@graphite-app graphite-app Bot force-pushed the perf/parser-skip-call-rest branch from 6e7903f to 883dfc1 Compare June 7, 2026 12:13
@graphite-app graphite-app Bot merged commit 883dfc1 into main Jun 7, 2026
30 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jun 7, 2026
@graphite-app graphite-app Bot deleted the perf/parser-skip-call-rest branch June 7, 2026 12:16
graphite-app Bot pushed a commit that referenced this pull request Jun 8, 2026
Follow-up to #23063.

`parse_lhs_expression_or_higher` parses the `MemberExpression` to completion before entering `parse_call_expression_rest`, and (since #23063) only enters it when the current token is `(` (Arguments) or `?.` (OptionalChain). The loop in `parse_call_expression_rest` then re-ran `parse_member_expression_rest` — the hottest function in the parser — as its first step on every iteration.

For a `(` entry that first call is the identity: `(` is not in the member-expression continuation FIRST set (`?.`, `.`, `[`, template, TS `!`/`<`). So skip it on the `(` first-entry. The `?.` entry and every later iteration (after `Arguments`) still run it.

Behavior-preserving:
- `cargo coverage -- parser` — Test262 / Babel / TypeScript / ESTree snapshots byte-identical (no `.snap` changes).
- `just allocs` — allocation snapshot unchanged.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Boshen added a commit that referenced this pull request Jun 8, 2026
### 💥 BREAKING CHANGES

- ee4dc73 ast: [**BREAKING**] Add `#[non_exhaustive]` to AST nodes
(#23046) (overlookmotel)
- 4c35362 ast: [**BREAKING**] Add
`AstBuilder::template_element_escape_raw` and
`template_element_escape_raw_with_lone_surrogates` methods (#23047)
(overlookmotel)

### 🚀 Features

- b846ab2 react_compiler: Integrate the Rust port of the React Compiler
(#22942) (Boshen)
- 5b8dd68 parser: Report TS1255 for invalid class definite assertions
(#22917) (camc314)
- 85efabf semantic: Make building the class table optional, off by
default (#22862) (Boshen)

### 🐛 Bug Fixes

- 556acdc codegen: Parenthesize TS-cast assignment targets (#23112)
(Boshen)
- 37169ff codegen: Don't emit space between postfix `--` and `>` when
minifying (#23036) (Boshen)
- a4b1bf7 codegen: Drop redundant whitespace in minified TypeScript
output (#23038) (Boshen)
- cf53285 parser: Report reserved type-declaration names in the parser
(#23035) (Boshen)
- 4e44969 ast: Fix UB in `escape_template_element_raw` (#23052)
(overlookmotel)
- c543154 parser: Report comma operator in JSX expression in the parser
(#23030) (Boshen)
- 325c94f codegen: Tighten conditional-type and constructor-type
whitespace when minifying (#23033) (Boshen)
- 95dd3a2 parser: Report `import type` alias to a non-external reference
in the parser (#23032) (Boshen)
- 90180b8 codegen: Drop space after `:` in function return type when
minifying (#23028) (Boshen)
- 6da876e parser: Report `abstract` private class field in the parser
(#23029) (Boshen)
- 28467ce codegen: Don't emit space before a postfix update operand when
minifying (#23027) (Boshen)
- cb29926 codegen: Drop redundant space after `export default` when
minifying (#23024) (Boshen)
- 62965ae codegen: Drop redundant space after `else` when minifying
(#23025) (Boshen)
- 989230a parser: Report compound assignment to non-simple target in the
parser (#23022) (Boshen)
- 06f367c parser: Report `super.#field` private access in the parser
(#23014) (Boshen)
- 184edef codegen: Print space before `const`/`declare` enum modifier
(#23013) (Boshen)
- 4d722e0 parser: Report duplicate switch `default` clause in the parser
(#23012) (Boshen)
- 597ed85 codegen: Parenthesize `let`/`async` for-of head target
(#23008) (Boshen)
- 8b631bf codegen: Remove stray space before mapped type value colon
(#23010) (Boshen)
- c08407e codegen: Don't over-parenthesize `in` inside an arrow in a
for-init (#23009) (Boshen)
- 600cd6f codegen: Parenthesize lower-precedence
`TSInstantiationExpression` operand (#23007) (Boshen)
- 187e1a5 codegen: Don't leak space after comment-only JSX expression
container (#23006) (Boshen)
- 294c473 codegen: Don't over-parenthesize `TSTypeAssertion` operand
(#23004) (Boshen)
- 786d96f codegen: Give `TSTypeAssertion` unary precedence (#23002)
(Boshen)
- 1295882 parser: Report `new.target` and `import.meta` syntax errors in
the parser (#23003) (Boshen)
- d727b6b codegen: Parenthesize `await` expression as base of `**`
(#23001) (Boshen)
- 67dfa08 codegen: Keep parentheses around `new` callees containing a
call (#22997) (Boshen)
- 17e7cf3 parser: Disallow unerasable `as`/`satisfies` assertions
(#22986) (Boshen)
- beb46d3 parser: Commit to module goal on decorated exports (#22941)
(Boshen)
- 49e63f7 isolated-declarations: Require annotations for satisfies
initializers (#22898) (camc314)
- 8c93601 isolated-declarations: Allow unknown enum initializer in
non-const enum (#22900) (camc314)

### ⚡ Performance

- 7d89909 parser: Peek instead of lookahead for yield disambiguation
(#23071) (Boshen)
- bf872f0 parser: Skip arrow lookahead for a parenthesized literal
(#23070) (Boshen)
- d19fc54 parser: Guard type-argument speculation behind an angle-token
check (#23069) (Boshen)
- 8eb5507 parser: Skip redundant member-rest re-scan on call entry
(#23068) (Boshen)
- 883dfc1 parser: Skip parse_call_expression_rest when no call follows
(#23063) (Boshen)
- b171153 parser: Peek before the await-using lookahead (#23059)
(Boshen)
- 56f21bd parser: Use peek_token for the TS `asserts` type predicate
(#23058) (Boshen)
- 68805ac parser: Use peek_token instead of checkpoint/rewind for
single-token decisions (#23056) (Boshen)
- 1f9d8eb ast: `AstBuilder::template_element_escape_raw` avoid
allocation if no escape required (#23053) (overlookmotel)
- 502b04d semantic: Move cold function redeclaration handling into
`#[cold]` function (#22973) (overlookmotel)

### 📚 Documentation

- 275d318 napi/minifier: Point `target` to oxc docs (#23102) (camc314)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-parser Area - Parser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant