Skip to content

Conversation

@camchenry
Copy link
Member

@camchenry camchenry commented Oct 20, 2025

In the current implementation, we collect all rules, and then remove rules that only run on specific node types and that have no relevant node types in the current file. This leads to a larger allocation size because we only remove the rules after we've already collected all of the rules.

This PR switches the order so that we filter out the rules before we actually collect them into a Vec. The downside of this is I can't see an easy way currently to make this work with the debug assertions we have for ensuring the runtime optimizations don't affect the output. The benefit of this is that the constant overhead for small files is much less, as shown by the benchmarks.

@github-actions github-actions bot added A-linter Area - Linter C-performance Category - Solution not expected to change functional behavior, only performance labels Oct 20, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 20, 2025

CodSpeed Performance Report

Merging #14822 will improve performances by 3.55%

Comparing 10-20-perf_linter_move_up_rule_retainment_into_initial_collection_to_reduce_allocation_size (0fc7f8b) with main (ee68089)

Summary

⚡ 1 improvement
✅ 3 untouched
⏩ 33 skipped1

Benchmarks breakdown

Mode Benchmark BASE HEAD Change
Simulation linter[RadixUIAdoptionSection.jsx] 729 µs 704 µs +3.55%

Footnotes

  1. 33 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.

@camchenry
Copy link
Member Author

@camc314 I wanted to avoid doing this initially because I couldn't think of an easy way to make this work well with the runtime optimization toggling in debug. Maybe AI can come up with something better, but this appears to have quite a large effect for small files! Maybe we could run two sets of rules in debug and compare them?

@graphite-app graphite-app bot changed the base branch from 10-20-perf_linter_id-length_reduce_allocations_and_add_more_ascii_checks to graphite-base/14822 October 20, 2025 16:55
@graphite-app graphite-app bot force-pushed the 10-20-perf_linter_move_up_rule_retainment_into_initial_collection_to_reduce_allocation_size branch from 65262b1 to 633a1d0 Compare October 20, 2025 17:00
@graphite-app graphite-app bot force-pushed the graphite-base/14822 branch from 04a78ee to b8f8ce5 Compare October 20, 2025 17:00
@graphite-app graphite-app bot changed the base branch from graphite-base/14822 to main October 20, 2025 17:00
@graphite-app graphite-app bot force-pushed the 10-20-perf_linter_move_up_rule_retainment_into_initial_collection_to_reduce_allocation_size branch from 633a1d0 to b149e71 Compare October 20, 2025 17:01
@camchenry camchenry force-pushed the 10-20-perf_linter_move_up_rule_retainment_into_initial_collection_to_reduce_allocation_size branch from b149e71 to 7fb7e5d Compare October 21, 2025 03:45
Copy link
Member Author


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.

@camchenry camchenry marked this pull request as ready for review October 21, 2025 18:41
@camchenry camchenry requested a review from camc314 as a code owner October 21, 2025 18:41
Copilot AI review requested due to automatic review settings October 21, 2025 18:41
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 optimizes the linter's rule filtering by moving the retention logic earlier in the processing pipeline. Instead of collecting all rules first and then filtering them, rules are now filtered before collection, reducing memory allocation overhead for small files.

Key Changes:

  • Moved AST node type checking into the initial filter predicate
  • Eliminated the separate retain operation that occurred after rule collection
  • Removed runtime optimization parameter from the execute_rules closure

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@camc314 camc314 self-assigned this Oct 22, 2025
@camchenry camchenry force-pushed the 10-20-perf_linter_move_up_rule_retainment_into_initial_collection_to_reduce_allocation_size branch 2 times, most recently from 1b01f4c to 8664d92 Compare October 29, 2025 00:48
@camchenry camchenry force-pushed the 10-20-perf_linter_move_up_rule_retainment_into_initial_collection_to_reduce_allocation_size branch from 8664d92 to 91d086d Compare October 29, 2025 01:20
@camc314 camc314 force-pushed the 10-20-perf_linter_move_up_rule_retainment_into_initial_collection_to_reduce_allocation_size branch from 91d086d to 0fc7f8b Compare October 29, 2025 13:48
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Oct 29, 2025
Copy link
Contributor

camc314 commented Oct 29, 2025

Merge activity

…ce allocation size (#14822)

In the current implementation, we collect all rules, and then remove rules that only run on specific node types and that have no relevant node types in the current file. This leads to a larger allocation size because we only remove the rules after we've already collected all of the rules.

This PR switches the order so that we filter out the rules before we actually collect them into a `Vec`. The downside of this is I can't see an easy way currently to make this work with the debug assertions we have for ensuring the runtime optimizations don't affect the output. The benefit of this is that the constant overhead for small files is much less, as shown by the benchmarks.
@graphite-app graphite-app bot force-pushed the 10-20-perf_linter_move_up_rule_retainment_into_initial_collection_to_reduce_allocation_size branch from 0fc7f8b to a9f68b2 Compare October 29, 2025 13:56
@graphite-app graphite-app bot merged commit a9f68b2 into main Oct 29, 2025
20 checks passed
@graphite-app graphite-app bot deleted the 10-20-perf_linter_move_up_rule_retainment_into_initial_collection_to_reduce_allocation_size branch October 29, 2025 14:01
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Oct 29, 2025
camc314 added a commit that referenced this pull request Oct 30, 2025
## [1.25.0] - 2025-10-30

### 💥 BREAKING CHANGES

- 659fd37 linter: [**BREAKING**] `tsgolint`: request fixes when
necessary (#15048) (camchenry)

### 🚀 Features

- ed24d60 linter: Expose tsgolint program diagnostics (#15080) (camc314)
- 23660c9 linter: `tsgolint`: handle omitted fixes and suggestions
(#15047) (camchenry)
- f7bef73 linter/plugins: Scope manager API (#14890) (Arsh)
- 3e15cdd linter/strict-boolean-expression: Add rule (#14930) (camc314)
- bd74603 linter: Add support for vitest/valid-title rule (#12085)
(Tyler Earls)

### 🐛 Bug Fixes

- e41dee5 linter/consistent-type-definition: Skip comments when looking
for token (#14909) (camc314)
- 806f9ba linter: Search system PATH for tsgolint executable (#14861)
(magic-akari)
- ee68089 linter: Normalize JS plugin names (#15010) (Peter Wagenet)
- 597340e ast-tools: Use oxfmt to format generated code (#15064)
(camc314)
- 5eaaa8e linter: Prevent underflow in count_comment_lines for JSX files
(#15026) (ityuany)
- 88577a8 import/no-namespace: Remove dot special case (#15032) (Arsh)
- 55ee962 linter/vars-on-top: False positive with typescript declare
block (#14952) (Hamir Mahal)
- 2de9f39 linter/plugins: Fall back to package name if meta.name is
missing (#14938) (Peter Wagenet)
- 5ace84b linter/no-empty-object-type: Parse `"allowWithName"` as
regular expressions (#14943) (Arsh)
- 5a2832d editor: Stop client when delete .oxlintrc.json with
`oxc.requireConfig` (#14897) (Liang Mi)
- 3e29d23 linter: Use aliases when parsing cli rules (#14912) (Arsh)
- b94b6aa linter/explicit-module-boundary-types: False negative with
export default function (#14905) (camc314)
- 7060863 linter/no-standaline-expect: False positive with expect in
callback (#14902) (camc314)
- dc5a71b linter/no-accumulating-spread: False positive in nested
callbacks within reduce (#14898) (camc314)

### 🚜 Refactor

- 8d8d508 editor: Flatten `flags` options (#15006) (Sysix)
- b1e1531 language_server: Extract library interface from main.rs
(#15036) (Boshen)
- 5de99c2 formatter: Export unified way to get_parse_options (#15027)
(leaysgur)
- b55df7f language_server: Move sub option for `flags` to the root +
deprecate flags (#14933) (Sysix)

### 📚 Documentation

- e15c91c linter: Add configuration option docs for
eslint/init-declarations rule. (#15085) (Connor Shea)
- f4505bc linter: Add configuration option docs for eslint/id-length
rule. (#15083) (Connor Shea)
- dd4c9d2 linter: Add configuration option docs for eslint/getter-return
rule. (#15081) (Connor Shea)
- 008e67a linter: Add configuration option docs for
jest/no-large-snapshots rule. (#15079) (Connor Shea)
- 31daf79 linter: Add configuration option docs for import/no-commonjs
rule. (#15077) (Connor Shea)
- 9bf8ebe linter: Add configuration option docs for
jsdoc/check-tag-names rule. (#15076) (Connor Shea)
- 491ab5e linter: Add configuration option docs for jsdoc/no-defaults
rule. (#15074) (Connor Shea)
- 2602d7e linter: Add configuration option docs for jsdoc/empty-tags
rule. (#15072) (Connor Shea)
- c3a92e0 linter: Add configuration option docs for
oxc/no-rest-spread-properties rule. (#15070) (Connor Shea)
- c7c9213 linter: Add configuration option docs for
oxc/no-optional-chaining rule. (#15071) (Connor Shea)
- 47a616f linter: Add configuration option docs for
typescript/no-empty-object-type rule. (#14968) (Connor Shea)
- fcc1e25 linter: Add configuration option docs for
typescript/no-explicit-any rule. (#15051) (Connor Shea)
- 1b88934 linter: Add configuration option docs for
eslint/no-multi-assign rule. (#15052) (Connor Shea)
- 909d4b9 linter: Add configuration option docs for
eslint/no-unsafe-negation rule. (#15053) (Connor Shea)
- f60763b linter: Add configuration option docs for
eslint/no-useless-computed-key rule. (#15054) (Connor Shea)
- c8911d4 linter: Add configuration option docs for
eslint/no-unneeded-ternary rule. (#15056) (Connor Shea)
- 9fc6374 linter: Add configuration option docs for
eslint/no-misleading-character-class rule. (#15058) (Connor Shea)
- 7656e2b linter: Add configuration option docs for eslint/sort-imports
rule. (#15013) (Connor Shea)
- bb06039 linter: Add configuration option docs for
unicorn/no-instanceof-builtins rule. (#14999) (Connor Shea)
- a4af5ff linter: Add configuration option docs for
unicorn/no-typeof-undefined rule. (#15001) (Connor Shea)
- ac2ed39 linter: Add configuration option docs for
unicorn/numeric-separators-style rule. (#15002) (Connor Shea)
- 5ce3e33 linter: Add configuration option docs for
unicorn/prefer-object-from-entries rule. (#15003) (Connor Shea)
- 07aea72 linter: Add configuration option docs for eslint/sort-keys
rule. (#15014) (Connor Shea)
- 35b36e0 linter: Add configuration option docs for eslint/no-eval rule.
(#15015) (Connor Shea)
- d222b85 linter: Add configuration option docs for eslint/no-empty
rule. (#15017) (Connor Shea)
- 83bbc37 linter: Add configuration option docs for
eslint/no-extend-native rule. (#15018) (Connor Shea)
- aade2fe linter: Add configuration option docs for
eslint/no-extra-boolean-cast rule. (#15019) (Connor Shea)
- bae0c09 linter: Add configuration option docs for jsdoc/require-yields
rule. (#15024) (Connor Shea)
- 06c77b6 linter: Add configuration option docs for unicorn/prefer-at
rule. (#15023) (Connor Shea)
- 54b2cb4 linter: Add configuration option docs for
eslint/max-classes-per-file rule. (#15022) (Connor Shea)
- b5acda7 linter: Add configuration option docs for
typescript/explicit-module-boundary-types rule. (#15021) (Connor Shea)
- a0c6162 linter: Add configuration option docs for vue/max-props rule.
(#14966) (Connor Shea)
- 3ec59c0 linter: Add configuration option docs for
no-async-endpoint-handlers rule. (#14965) (Connor Shea)
- 7a275fd linter: Add configuration option docs for no-map-spread rule.
(#14964) (Connor Shea)
- 0002b7c linter: Add configuration option docs for no-barrel-file rule.
(#14963) (Connor Shea)
- fd4b7ab linter: Add configuration option docs for
unicorn/consistent-function-scoping rule. (#14969) (Connor Shea)
- aa339b3 linter: Add configuration option docs for eslint/no-void rule.
(#14970) (Connor Shea)
- af8bdae linter: Add configuration option docs for
import/no-anonymous-default-export rule. (#14971) (Connor Shea)
- 5a81953 linter: Add configuration option docs for jest/require-hook
rule. (#14972) (Connor Shea)
- 1be268d linter: Add configuration option docs for
jest/require-top-level-describe rule. (#14973) (Connor Shea)
- 860a58c linter: Add configuration option docs for jest/no-hooks rule.
(#14974) (Connor Shea)
- b69b586 linter: Add configuration option docs for jest/max-expects
rule. (#14975) (Connor Shea)
- 2e02fe0 linter: Add configuration option docs for
jest/no-deprecated-functions rule (#14976) (Connor Shea)
- cdc6c4f linter: Add configuration option docs for
unicorn/no-array-sort rule. (#14977) (Connor Shea)
- 40eb394 linter/no-unassigned-import-config: Add auto generated config
docs (#14918) (camc314)

### ⚡ Performance

- a9f68b2 linter: Move up rule retainment into initial collection to
reduce allocation size (#14822) (camchenry)
- b27c5b9 linter: Add codegen support for regex nodes (#14874)
(camchenry)

### 🧪 Testing

- bf898e5 linter: Increase stability of tsgolint test cases (#15063)
(camc314)
- 49da411 linter/no-standalone-expect: Fix tests (#14901) (camc314)

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

Labels

A-linter Area - Linter 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.

3 participants