Skip to content

perf(linter/plugins): make tokens class instances#19980

Merged
graphite-app[bot] merged 1 commit intomainfrom
om/03-02-perf_linter_plugins_make_tokens_class_instances
Mar 3, 2026
Merged

perf(linter/plugins): make tokens class instances#19980
graphite-app[bot] merged 1 commit intomainfrom
om/03-02-perf_linter_plugins_make_tokens_class_instances

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Mar 3, 2026

#19978 introduced caching of tokens, and got a large speed-up. However, it had one major flaw.

Token objects (like AST nodes) are plain objects with a getter on the prototype for loc property. When loc getter is triggered, it adds a property to the token object itself, so that accessing loc again evaluates to the same Location object, rather than recalculating the location and creating a new object.

Now that we cache tokens objects, any tokens which had loc property added need to have that property deleted again when tokens are reset, ready for reuse.

Deleting the loc property has an unfortunate side effect. V8 transitions the object into "dictionary mode", which makes all property accesses much slower on that object for the rest of its lifecycle.

Worse, it means that token objects don't have a consistent object "shape", which inhibits inline caches and transitions deserializeTokenInto from monomorphic to polymorphic, which makes V8 deoptimize it. This is one of the hottest JS functions in Oxlint, so has a heavy perf impact.

Solve this problem by making tokens instances of a Token class. Token stores loc in a private #loc property, and the loc getter can fetch it if loc for that token was already accessed.

All Token objects now have a consistent object shape throughout, regardless of whether loc is accessed on them or not.

This does make the loc getter a little more expensive, but that cost is heavily outweighed by the improved performance yielded by consistent object shapes.

Copy link
Member Author

overlookmotel commented Mar 3, 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.

@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI A-linter-plugins Area - Linter JS plugins labels Mar 3, 2026
@github-actions github-actions bot added the C-performance Category - Solution not expected to change functional behavior, only performance label Mar 3, 2026
@overlookmotel overlookmotel marked this pull request as ready for review March 3, 2026 21:25
@overlookmotel overlookmotel requested a review from camc314 as a code owner March 3, 2026 21:25
Copilot AI review requested due to automatic review settings March 3, 2026 21:25
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 refactors token deserialization/caching to use Token class instances (with private-field loc caching) to avoid V8 hidden-class transitions introduced by deleting cached loc properties, improving hot-path performance in token reuse.

Changes:

  • Replace plain-object token pooling with pooled Token class instances that lazily compute/cache loc in #loc.
  • Introduce computeLoc(start, end) and simplify getLoc() to rely on node/token .loc getters for caching behavior.
  • Update token reset logic to clear cached loc via a private-field reset instead of delete token.loc.

Reviewed changes

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

File Description
apps/oxlint/src-js/plugins/tokens.ts Switch pooled tokens to Token class instances with private-field loc caching and reset logic changes.
apps/oxlint/src-js/plugins/location.ts Add computeLoc, simplify getLoc, and adjust getNodeLoc to reuse computeLoc and change loc define semantics.

@overlookmotel overlookmotel force-pushed the om/03-02-perf_linter_plugins_reduce_memory_copies_for_tokens branch from 5338b5c to c477205 Compare March 3, 2026 22:42
@overlookmotel overlookmotel force-pushed the om/03-02-perf_linter_plugins_make_tokens_class_instances branch from 8d4fb15 to 0501477 Compare March 3, 2026 22:42
@overlookmotel overlookmotel requested a review from Copilot March 3, 2026 22:43
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

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

@graphite-app graphite-app bot added the 0-merge Merge with Graphite Merge Queue label Mar 3, 2026
@graphite-app
Copy link
Contributor

graphite-app bot commented Mar 3, 2026

Merge activity

#19978 introduced caching of tokens, and got a large speed-up. However, it had one major flaw.

Token objects (like AST nodes) are plain objects with a getter on the prototype for `loc` property. When `loc` getter is triggered, it adds a property to the token object itself, so that accessing `loc` again evaluates to the same `Location` object, rather than recalculating the location and creating a new object.

Now that we cache tokens objects, any tokens which had `loc` property added need to have that property deleted again when tokens are reset, ready for reuse.

Deleting the `loc` property has an unfortunate side effect. V8 transitions the object into "dictionary mode", which makes all property accesses much slower on that object for the rest of its lifecycle.

Worse, it means that token objects don't have a consistent object "shape", which inhibits inline caches and transitions `deserializeTokenInto` from monomorphic to polymorphic, which makes V8 deoptimize it. This is one of the hottest JS functions in Oxlint, so has a heavy perf impact.

Solve this problem by making tokens instances of a `Token` class. `Token` stores `loc` in a private `#loc` property, and the `loc` getter can fetch it if `loc` for that token was already accessed.

All `Token` objects now have a consistent object shape throughout, regardless of whether `loc` is accessed on them or not.

This does make the `loc` getter a little more expensive, but that cost is heavily outweighed by the improved performance yielded by consistent object shapes.
@graphite-app graphite-app bot force-pushed the om/03-02-perf_linter_plugins_reduce_memory_copies_for_tokens branch from c477205 to 758b424 Compare March 3, 2026 23:24
@graphite-app graphite-app bot force-pushed the om/03-02-perf_linter_plugins_make_tokens_class_instances branch from 0501477 to 77f1c71 Compare March 3, 2026 23:25
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Mar 3, 2026
Base automatically changed from om/03-02-perf_linter_plugins_reduce_memory_copies_for_tokens to main March 3, 2026 23:32
@graphite-app graphite-app bot merged commit 77f1c71 into main Mar 3, 2026
20 checks passed
@graphite-app graphite-app bot deleted the om/03-02-perf_linter_plugins_make_tokens_class_instances branch March 3, 2026 23:33
camc314 pushed a commit that referenced this pull request Mar 9, 2026
# Oxlint
### 🚀 Features

- e6b604f oxlint: Auto-enable gitlab formatter on GitLab (#20076)
(camc314)
- 2488a68 linter: Add .oxlintrc.jsonc config file support (#19870)
(Scott S.)
- 61bf388 linter: Add `options.reportUnusedDisableDirectives` to config
file (#19799) (Peter Wagenet)
- c92422b oxlint: Auto-enable github formatter on GitHub Actions
(#19944) (Boshen)
- 0337c6d linter: Implement typescript/no-unecessary-type-conversion
(#19955) (camc314)
- 2919313 linter: Introduce denyWarnings config options (#19926)
(camc314)
- a607119 linter: Introduce maxWarnings config option (#19777) (camc314)

### 🐛 Bug Fixes

- 0861d9a linter/plugins: Remove getters from `Context` (#20115)
(overlookmotel)
- 92cfb14 linter/plugins: Fix types for `walkProgram` and
`walkProgramWithCfg` (#20081) (overlookmotel)
- 10e211f oxlint/lsp: Send other code actions besides `source.fixAll` if
requested (#20042) (Sysix)
- 602daaa linter/plugins: Fix type definition for `VisitorObject`
(#20065) (overlookmotel)
- ee0491e apps,napi: Explicitly specify libs in tsconfigs (#20071)
(camc314)
- b6e9499 linter: Fix the logic for `unicorn/prefer-dom-node-remove` to
handle literal callees as well as arguments. (#20059) (connorshea)
- 3874ae1 linter: Update `unicorn/prefer-query-selector` to also catch
`getElementsByName()`. (#20060) (connorshea)
- 77c93fb linter: Handle array-type shorthand inside union members
(#20034) (camc314)
- 50eb160 linter/no-unused-vars: Allow unused type params in ambient
module blocks (#19615) (Don Isaac)
- 1dd0d21 linter/no-restricted-imports: Apply regex pattern checks to
side-effect imports (#20028) (camc314)
- 7f3d735 linter: Error when --type-check is used without --type-aware
(#20025) (camc314)
- eea201c unicorn/prefer-string-slice: Avoid unsafe autofix for
substr-to-slice (#20010) (camc314)
- 50359dc oxlint/lsp: Detect `reportUnusedDisableDirectives` from oxlint
config, change lsp `unusedDisableDirectives` default value to `None`
(#20011) (Sysix)
- 4bc84b1 linter/plugins: Allow `null` and `undefined` for
`rule.meta.fixable` (#20008) (overlookmotel)
- 753e27e linter/role-supports-aria-props: Add `aria-posinset` to
supported `option` ARIA properties (#20003) (JongKyung Lee)
- f57b2c9 linter/plugins: Fix return types of tokens methods (#19985)
(overlookmotel)
- 27ee4fc linter/no-loss-of-precision: Avoid double rounding for
negative exponents (#19999) (camc314)
- 77a94bb linter: Avoid no-loss-of-precision false positive for 3e-308
(#19992) (camc314)
- 6245c56 linter/no-unused-private-class-members: Treat logical lhs
access as usage (#19991) (camc314)
- 65891e3 linter: Avoid prefer-const false positive for mixed-scope
destructuring (#19982) (camc314)
- 89991fe linter: Avoid prefer-const false positive for operator
reassignments (#19975) (camc314)
- 87318e7 oxlint/lsp: Load js config with reforcing fs read (#19551)
(Sysix)
- d40a942 linter/no-useless-constructor: Mark fixer as suggestion
(#19961) (camc314)
- ccbd959 linter/prefer-code-point: Report String.fromCharCode member
references (#19931) (camc314)
- 14fbbfc linter: Add help text to oxc/no-rest-spread-properties rule
(#19900) (Subin Kim)

### ⚡ Performance

- 2baa5fb napi: Unify build-test profile to coverage for cache sharing
(#20090) (Boshen)
- 77f1c71 linter/plugins: Make tokens class instances (#19980)
(overlookmotel)
- 758b424 linter/plugins: Reduce memory copies for tokens (#19979)
(overlookmotel)
- 236847f linter/plugins: Cache token objects (#19978) (overlookmotel)
- 94b597a linter/plugins: Store tokens as a `Box<[Token]>` (#19969)
(overlookmotel)

### 📚 Documentation

- 2c0010a linter/plugins: Move comment about "bivariance hack" into
generated code (#20082) (overlookmotel)
- 7538f09 linter: Improve `import/extensions` and
`import/no-named-as-default` rule docs. (#20053) (connorshea)
- 1f909cf linter: Improve docs for
`unicorn/require-post-message-target-origin` rule. (#20061) (connorshea)
- 12ae35c oxlint/lsp: Remove outdated ToDo for `LintOptions.run`
(#20012) (Sysix)
- 3be73e6 linter/plugins: Fix JSDoc comments for tokens methods (#20004)
(overlookmotel)
- 48ef285 linter: Update `--config` docs (#19965) (camc314)
- 6ea49a0 linter: Fix some identation issues for the generated types
used in `oxlint.config.ts`. (#19942) (connorshea)
- 6c0e0b5 linter: Add oxlint.config.ts to the config docs. (#19941)
(connorshea)
- 160e423 linter: Add a note that the typeAware and typeCheck options
require oxlint-tsgolint (#19940) (connorshea)
- d54c275 linter: Improve rule docs for 27 unicorn rules (#19903)
(connorshea)
# Oxfmt
### 🚀 Features

- ee26215 oxfmt: Support css-in-js substitution (#20019) (leaysgur)
- 0f0ff51 oxfmt: Display default settings was used message in cli stats
(#19939) (leaysgur)
- 88815b8 oxfmt: Reintroduce stats line for write mode (#19938)
(leaysgur)

### 🐛 Bug Fixes

- ee0491e apps,napi: Explicitly specify libs in tsconfigs (#20071)
(camc314)
- 92f4490 oxfmt: Apply `is_ignored_dir` for glob paths too (#20056)
(leaysgur)
- 114f974 oxfmt/lsp: Prefer language_id over file extension when
formatting (#19977) (copilot-swe-agent)

### ⚡ Performance

- 2baa5fb napi: Unify build-test profile to coverage for cache sharing
(#20090) (Boshen)

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-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