perf(linter): add should_run based on node types (batch 1)
#12228
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jul 11, 2025
Member
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd either label to this PR to merge it via 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
bot
added
the
C-performance
CodSpeed Instrumentation Performance ReportMerging #12228 will not alter performanceComparing Summary
|
Member
Author
|
+2% perf improvement on the radix UI benchmark: 
|
added 4 commits
July 11, 2025 12:29
camchenry
force-pushed
the
07-11-feat_semantic_add_methods_to_check_if_ast_contains_any_all_node_types
branch
from
09291b4 to
cd7d601
Compare
camchenry
requested review from
Dunqing,
camc314 and
overlookmotel
as code owners
camchenry
force-pushed
the
07-11-perf_linter_add_should_run_based_on_node_types_batch_1_
branch
from
3b85248 to
f353586
Compare
This was referenced Jul 11, 2025
camchenry
force-pushed
the
07-11-feat_semantic_add_methods_to_check_if_ast_contains_any_all_node_types
branch
from
cd7d601 to
1449a81
Compare
Comment on lines
+114
to
+120
| fn should_run(&self, ctx: &crate::context::ContextHost) -> bool { | ||
| ctx.semantic().nodes().contains(oxc_ast::AstType::BinaryExpression) | ||
| && ctx | ||
| .semantic() | ||
| .nodes() | ||
| .contains_any(&[oxc_ast::AstType::NumericLiteral, oxc_ast::AstType::BigIntLiteral]) | ||
| } |
Member
There was a problem hiding this comment.
From benchmarks, obviously some of these changes are having a positive effect.
But some of these methods may be of limited value. How many ASTs don't contain a single BinaryExpression and not a single NumericLiteral?
I imagine most of the gain is from rarer types e.g. AstType::Class and AstType::DebuggerStatement.
You may want to "pick your fights".
camchenry
deleted the
07-11-perf_linter_add_should_run_based_on_node_types_batch_1_
branch
graphite-app bot
pushed a commit
that referenced
this pull request
Sep 8, 2025
…3138) - part of #12223 - supersedes stack: #12228 This PR adds the ability to add node type information to indicate what node types a lint rule acts on. The lint rule runner can use this information to optimize running lint rules by skipping nodes, files, or rules that don't apply. This is a simple version that is focused on creating the framework for automatically generating the rule runner code. This is not the end of optimizations. Currently only 102 rules have node type information, while 471 rules have no node type information and will not be skipped. Adding node type information for more rules will improve performance. We may also be able to skip more nodes and files by being clever about how we use this information. For example, filtering out any rules which do not run on any node type, if the file contains no relevant node types for the rule. - Adds a new `RuleRunner` trait which includes information needed for indicating whether a rule has node type info or not - Adds a `oxc_linter_codegen` task which is used to generate the implementation of `RuleRunner` trait for all lint rules. This is mostly written by AI, so worth a bit of skepticism, but I've tried to manually verify the results and it looks good (just lacking more complex detection). - Currently, only rules that contain a single top-level `if` statement (or chain of `else if`), or a single `match` statement will have their node type info generated. This should give us a high level of confidence, since these patterns are relatively easy to verify and identify, but it's only a subset of all rules. Next steps will be to add support for `let...else` - Adjusts the main linting loop to use the node type information to skip rules for node types that don't apply - Updates CI to include a check for any uncommitted linter codegen changes --- ## Benchmarks In the latest version of this PR, CodSpeed shows an up to +13% increase in linter performance across large and small files. In the real-world, I've seen a ~5-15% improvement in performance, but in some cases close to zero for very small files. In either case, there appears to be little downside and a lot of potential upside here. <img width="769" height="242" alt="Screenshot 2025-08-24 at 9 29 30 PM" src="https://github.com/user-attachments/assets/d2c430c4-17a9-4606-85d5-c66c734975d9" /> <img width="757" height="308" alt="Screenshot 2025-08-24 at 9 30 24 PM" src="https://github.com/user-attachments/assets/5508464f-de55-4f4e-9ddd-58f215c4ce85" /> <img width="976" height="368" alt="Screenshot 2025-08-24 at 9 34 52 PM" src="https://github.com/user-attachments/assets/203f83c7-46f3-49f1-a1a8-e18f7ec8483b" />
graphite-app bot
pushed a commit
that referenced
this pull request
Sep 8, 2025
…3138) - part of #12223 - supersedes stack: #12228 This PR adds the ability to add node type information to indicate what node types a lint rule acts on. The lint rule runner can use this information to optimize running lint rules by skipping nodes, files, or rules that don't apply. This is a simple version that is focused on creating the framework for automatically generating the rule runner code. This is not the end of optimizations. Currently only 102 rules have node type information, while 471 rules have no node type information and will not be skipped. Adding node type information for more rules will improve performance. We may also be able to skip more nodes and files by being clever about how we use this information. For example, filtering out any rules which do not run on any node type, if the file contains no relevant node types for the rule. - Adds a new `RuleRunner` trait which includes information needed for indicating whether a rule has node type info or not - Adds a `oxc_linter_codegen` task which is used to generate the implementation of `RuleRunner` trait for all lint rules. This is mostly written by AI, so worth a bit of skepticism, but I've tried to manually verify the results and it looks good (just lacking more complex detection). - Currently, only rules that contain a single top-level `if` statement (or chain of `else if`), or a single `match` statement will have their node type info generated. This should give us a high level of confidence, since these patterns are relatively easy to verify and identify, but it's only a subset of all rules. Next steps will be to add support for `let...else` - Adjusts the main linting loop to use the node type information to skip rules for node types that don't apply - Updates CI to include a check for any uncommitted linter codegen changes --- ## Benchmarks In the latest version of this PR, CodSpeed shows an up to +13% increase in linter performance across large and small files. In the real-world, I've seen a ~5-15% improvement in performance, but in some cases close to zero for very small files. In either case, there appears to be little downside and a lot of potential upside here. <img width="769" height="242" alt="Screenshot 2025-08-24 at 9 29 30 PM" src="https://github.com/user-attachments/assets/d2c430c4-17a9-4606-85d5-c66c734975d9" /> <img width="757" height="308" alt="Screenshot 2025-08-24 at 9 30 24 PM" src="https://github.com/user-attachments/assets/5508464f-de55-4f4e-9ddd-58f215c4ce85" /> <img width="976" height="368" alt="Screenshot 2025-08-24 at 9 34 52 PM" src="https://github.com/user-attachments/assets/203f83c7-46f3-49f1-a1a8-e18f7ec8483b" />
Copilot AI
pushed a commit
that referenced
this pull request
Sep 8, 2025
…3138) - part of #12223 - supersedes stack: #12228 This PR adds the ability to add node type information to indicate what node types a lint rule acts on. The lint rule runner can use this information to optimize running lint rules by skipping nodes, files, or rules that don't apply. This is a simple version that is focused on creating the framework for automatically generating the rule runner code. This is not the end of optimizations. Currently only 102 rules have node type information, while 471 rules have no node type information and will not be skipped. Adding node type information for more rules will improve performance. We may also be able to skip more nodes and files by being clever about how we use this information. For example, filtering out any rules which do not run on any node type, if the file contains no relevant node types for the rule. - Adds a new `RuleRunner` trait which includes information needed for indicating whether a rule has node type info or not - Adds a `oxc_linter_codegen` task which is used to generate the implementation of `RuleRunner` trait for all lint rules. This is mostly written by AI, so worth a bit of skepticism, but I've tried to manually verify the results and it looks good (just lacking more complex detection). - Currently, only rules that contain a single top-level `if` statement (or chain of `else if`), or a single `match` statement will have their node type info generated. This should give us a high level of confidence, since these patterns are relatively easy to verify and identify, but it's only a subset of all rules. Next steps will be to add support for `let...else` - Adjusts the main linting loop to use the node type information to skip rules for node types that don't apply - Updates CI to include a check for any uncommitted linter codegen changes --- ## Benchmarks In the latest version of this PR, CodSpeed shows an up to +13% increase in linter performance across large and small files. In the real-world, I've seen a ~5-15% improvement in performance, but in some cases close to zero for very small files. In either case, there appears to be little downside and a lot of potential upside here. <img width="769" height="242" alt="Screenshot 2025-08-24 at 9 29 30 PM" src="https://github.com/user-attachments/assets/d2c430c4-17a9-4606-85d5-c66c734975d9" /> <img width="757" height="308" alt="Screenshot 2025-08-24 at 9 30 24 PM" src="https://github.com/user-attachments/assets/5508464f-de55-4f4e-9ddd-58f215c4ce85" /> <img width="976" height="368" alt="Screenshot 2025-08-24 at 9 34 52 PM" src="https://github.com/user-attachments/assets/203f83c7-46f3-49f1-a1a8-e18f7ec8483b" />
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.