Skip to content

perf(parser): remove const generic param from finish_next_inner#19684

Merged
graphite-app[bot] merged 1 commit intomainfrom
om/02-24-refactor_parser_remove_const_generic_param_from_finish_next_inner_
Feb 25, 2026
Merged

perf(parser): remove const generic param from finish_next_inner#19684
graphite-app[bot] merged 1 commit intomainfrom
om/02-24-refactor_parser_remove_const_generic_param_from_finish_next_inner_

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Feb 24, 2026

Replace const generic param from finish_next_inner.

- fn finish_next_inner<const REPLACE_SAME_START: bool>(&mut self, kind: Kind) -> Token {
+ fn finish_next_inner(&mut self, kind: Kind, mode: FinishTokenMode) -> Token {

The function is marked #[inline(always)] to ensure that mode can be statically known from each call site and acts as a const.

The perf improvement (5%-8% on parser_tokens benchmarks) surprised me. Turns out it's the result of the #[inline(always)] (see #19694 which made that change in isolation).

Aside from the perf improvement, it's easier to read, and may reduce compile time a bit (as this function is inlined into many call sites).

@github-actions github-actions bot added the A-parser Area - Parser label Feb 24, 2026
@github-actions github-actions bot added the C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior label Feb 24, 2026
Copy link
Member Author

overlookmotel commented Feb 24, 2026


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@overlookmotel overlookmotel marked this pull request as ready for review February 24, 2026 15:35
Copilot AI review requested due to automatic review settings February 24, 2026 15:35
@overlookmotel overlookmotel self-assigned this Feb 24, 2026
@overlookmotel overlookmotel marked this pull request as draft February 24, 2026 15:36
@overlookmotel overlookmotel force-pushed the om/02-24-refactor_parser_remove_const_generic_param_from_finish_next_inner_ branch from 3bccb19 to ac33e76 Compare February 24, 2026 19:06
@overlookmotel overlookmotel changed the title refactor(parser): remove const generic param from finish_next_inner perf(parser): remove const generic param from finish_next_inner Feb 24, 2026
@github-actions github-actions bot added the C-performance Category - Solution not expected to change functional behavior, only performance label Feb 24, 2026
@overlookmotel overlookmotel added A-linter-plugins Area - Linter JS plugins labels Feb 24, 2026
@overlookmotel overlookmotel marked this pull request as ready for review February 24, 2026 22:10
@graphite-app graphite-app bot added the 0-merge Merge with Graphite Merge Queue label Feb 25, 2026
@graphite-app
Copy link
Contributor

graphite-app bot commented Feb 25, 2026

Merge activity

…9684)

Replace const generic param from `finish_next_inner`.

```diff
- fn finish_next_inner<const REPLACE_SAME_START: bool>(&mut self, kind: Kind) -> Token {
+ fn finish_next_inner(&mut self, kind: Kind, mode: FinishTokenMode) -> Token {
```

The function is marked `#[inline(always)]` to ensure that `mode` can be statically known from each call site and acts as a const.

The perf improvement (5%-8% on `parser_tokens` benchmarks) surprised me. Turns out it's the result of the `#[inline(always)]` (see #19694 which made that change in isolation).

Aside from the perf improvement, it's easier to read, and may reduce compile time a bit (as this function is inlined into many call sites).
@graphite-app graphite-app bot force-pushed the om/02-24-refactor_parser_remove_const_generic_param_from_finish_next_inner_ branch from ac33e76 to b5d9845 Compare February 25, 2026 00:08
@graphite-app graphite-app bot merged commit b5d9845 into main Feb 25, 2026
22 checks passed
@graphite-app graphite-app bot deleted the om/02-24-refactor_parser_remove_const_generic_param_from_finish_next_inner_ branch February 25, 2026 00:14
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Feb 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter-plugins Area - Linter JS plugins A-parser Area - Parser C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior C-performance Category - Solution not expected to change functional behavior, only performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants