Skip to content

perf(linter/plugins): reduce allocations for tokens and comments with accessed loc#20480

Merged
graphite-app[bot] merged 1 commit intomainfrom
om/03-17-perf_linter_plugins_reduce_allocations_for_tokens_and_comments_with_accessed_loc_
Mar 21, 2026
Merged

perf(linter/plugins): reduce allocations for tokens and comments with accessed loc#20480
graphite-app[bot] merged 1 commit intomainfrom
om/03-17-perf_linter_plugins_reduce_allocations_for_tokens_and_comments_with_accessed_loc_

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Mar 17, 2026

Similar to #20479. tokensWithLoc and commentsWithLoc contain tokens/comments on which loc property has been accessed.

Previously these arrays were grown with .push(token), and shrunk at end of linting file with tokensWithLoc.length = 0;. Problem with that is that setting length to 0 frees the array's backing allocation, which means it has to reallocate again on next file when a token's loc is accessed.

Instead, never shrink these arrays, and track the active length in separate variables. After warm-up over first batch of files, these arrays will graduate to "old space" and just sit there without further allocations.

Unlike #20479, we don't use Uint32Arrays as loc is calculated lazily, and Token's / Comments don't know what their index is in the cachedTokens / cachedComments arrays. Adding an #index field to Token and Comment would bloat every instance of these classes by 8 bytes.

Copy link
Member Author

overlookmotel commented Mar 17, 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 changes, fast-track this PR to the front of the merge queue

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.

@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI A-linter-plugins Area - Linter JS plugins C-performance Category - Solution not expected to change functional behavior, only performance labels Mar 17, 2026
@overlookmotel overlookmotel self-assigned this Mar 18, 2026
@overlookmotel overlookmotel force-pushed the om/03-17-perf_linter_plugins_reduce_allocations_for_tokens_and_comments_with_accessed_loc_ branch 2 times, most recently from 5e9ff3a to c659d44 Compare March 18, 2026 09:45
@overlookmotel overlookmotel force-pushed the om/03-17-perf_linter_plugins_reduce_allocations_for_regex_tokens branch from 9e16a5a to 9254068 Compare March 18, 2026 09:45
@overlookmotel overlookmotel force-pushed the om/03-17-perf_linter_plugins_reduce_allocations_for_tokens_and_comments_with_accessed_loc_ branch from c659d44 to 66d228c Compare March 18, 2026 16:28
@overlookmotel overlookmotel force-pushed the om/03-17-perf_linter_plugins_reduce_allocations_for_regex_tokens branch from 9254068 to 1c54851 Compare March 18, 2026 16:28
@overlookmotel overlookmotel marked this pull request as ready for review March 18, 2026 19:03
@overlookmotel overlookmotel requested a review from camc314 as a code owner March 18, 2026 19:03
Copilot AI review requested due to automatic review settings March 18, 2026 19:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces per-file allocations in oxlint’s JS-side token/comment infrastructure by keeping the “loc-accessed” tracking arrays at a stable capacity and using an active-count cursor instead of shrinking them to length 0 between files.

Changes:

  • Track tokens/comments whose loc was accessed via activeTokensWithLocCount / activeCommentsWithLocCount.
  • Avoid array.length = 0 on reset; reuse existing backing stores by resetting only the active counters.
  • Update reset loops to iterate only over the active portion of each tracking array.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
apps/oxlint/src-js/plugins/tokens.ts Reuses tokensWithLoc backing store across files with an active-count cursor; updates reset logic accordingly.
apps/oxlint/src-js/plugins/comments.ts Mirrors the same active-count approach for commentsWithLoc and updates reset logic.

@graphite-app graphite-app bot added the 0-merge Merge with Graphite Merge Queue label Mar 20, 2026
@overlookmotel overlookmotel removed the 0-merge Merge with Graphite Merge Queue label Mar 20, 2026
@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Mar 20, 2026 — with Graphite App
@graphite-app
Copy link
Contributor

graphite-app bot commented Mar 20, 2026

Merge activity

@overlookmotel overlookmotel force-pushed the om/03-17-perf_linter_plugins_reduce_allocations_for_tokens_and_comments_with_accessed_loc_ branch from 847014b to d825833 Compare March 21, 2026 12:21
@overlookmotel overlookmotel force-pushed the om/03-17-perf_linter_plugins_reduce_allocations_for_regex_tokens branch 2 times, most recently from 3426e69 to dfd5e3f Compare March 21, 2026 12:32
@overlookmotel overlookmotel force-pushed the om/03-17-perf_linter_plugins_reduce_allocations_for_tokens_and_comments_with_accessed_loc_ branch from d825833 to 72d4da3 Compare March 21, 2026 12:32
… accessed `loc` (#20480)

Similar to #20479. `tokensWithLoc` and `commentsWithLoc` contain tokens/comments on which `loc` property has been accessed.

Previously these arrays were grown with `.push(token)`, and shrunk at end of linting file with `tokensWithLoc.length =  0;`. Problem with that is that setting length to 0 frees the array's backing allocation, which means it has to reallocate again on next file when a token's `loc` is accessed.

Instead, never shrink these arrays, and track the active length in separate variables. After warm-up over first batch of files, these arrays will graduate to "old space" and just sit there without further allocations.

Unlike #20479, we don't use `Uint32Array`s as `loc` is calculated lazily, and `Token`'s / `Comment`s don't know what their index is in the `cachedTokens` / `cachedComments` arrays. Adding an `#index` field to `Token` and `Comment` would bloat every instance of these classes by 8 bytes.
graphite-app bot pushed a commit that referenced this pull request Mar 21, 2026
Possibly the most absurd optimization I've ever encountered.

```diff
- if (regexObjects.length > regexIndex) {
+ if (regexIndex < regexObjects.length) {
    regex = regexObjects[regexIndex];
  }
```

There is no semantic difference whatsoever between `regexObjects.length > regexIndex` and `regexIndex < regexObjects.length`. But V8's 2nd-tier Maglev compiler treats them differently. In the`>` version, `regexObjects[regexIndex]` incurs a bounds check, whereas in the new `<` version, no bounds check.

This is a micro-optimization, but this function is extremely hot, so it may actually move the needle in code that includes a lot of regexes. The same optimization is also applied in #20480.

Claude found this oddity by reading V8's source code!
@graphite-app graphite-app bot force-pushed the om/03-17-perf_linter_plugins_reduce_allocations_for_regex_tokens branch from dfd5e3f to 9c7a267 Compare March 21, 2026 12:47
@graphite-app graphite-app bot force-pushed the om/03-17-perf_linter_plugins_reduce_allocations_for_tokens_and_comments_with_accessed_loc_ branch from 72d4da3 to 9cfc312 Compare March 21, 2026 12:48
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Mar 21, 2026
Base automatically changed from om/03-17-perf_linter_plugins_reduce_allocations_for_regex_tokens to main March 21, 2026 12:51
@graphite-app graphite-app bot merged commit 9cfc312 into main Mar 21, 2026
24 checks passed
@graphite-app graphite-app bot deleted the om/03-17-perf_linter_plugins_reduce_allocations_for_tokens_and_comments_with_accessed_loc_ branch March 21, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area - CLI A-linter Area - Linter A-linter-plugins Area - Linter JS plugins 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