feat(linter): add --debug options and add per-rule timing info#22282
Conversation
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. |
Merging this PR will not alter performance
Comparing Footnotes
|
236f062 to
27a21e4
Compare
|
I can't distinguish the timed/untimed versions. I tested this branch against |
--debug options and add per-rule timing info
15c2e99 to
8ad82f4
Compare
--debug options and add per-rule timing info--debug options and add per-rule timing info
cb8f5b9 to
b83a8f9
Compare
|
@camc314 See the above analysis for the performance results: #22282 (comment), but I think this is finally a version that doesn't have the negative performance impact on the hot path. Thanks for your analysis in the original issue. Since I think this could be easily reversed in a future PR, I'm inclined to go ahead and merge this so we can hopefully iterate more and see if it's valuable for us. At least so far, I've found one significant perf optimization just based on these timings information: #22393 Let me know what your thoughts are on this direction. |
Merge activity
|
) - part of #22138 This PR implements two separate but related things: - A new `--debug` option for the `oxlint` CLI that accepts a comma separated list of flags like `--debug foo,bar,baz`. This allows adding more debug functionality in the future without needing to add lots of individual CLI options. - A `--debug timings` option that enables per-rule timing information to be recorded and output. This enables determining what the slowest rules are in a single run of the linter in the CLI. ### Rule timings The primary goal of this altered approach to timing linter rules was: **when timings are not enabled, it doesn't cost any performance.**. To achieve that, we've added a `TIMINGS` const generic argument to all of the core `run`, `run_on_jest_node`, `run_once`, etc. functions in the linter. This creates more code overall, because instead of having a single version of each function, now we have two: one where the compiler optimized it with `TIMINGS = false` and one where `TIMINGS = true`. We then switch between these two paths at runtime by passing `--debug timings`. So, was this successful? In short, yes. There is no significant difference between `oxlint` in `main` and this PR when it comes to performance, unless you enable per-rule timings (see #22282 (comment)). When you enable timing information, there is a 20-30% overhead in performance. This is because we have to record lots of time information and bookkeeping in hash maps to output the final information. This could be optimized down the line possibly, but it's not a primary goal. This is also kept fairly generic so that in future PRs we can add support for JS plugin timings and `tsgolint` rule timings as well, if those are available. Overall, I think the increased maintenance burden is worth it. Even looking at the example output below, I can already see ways to improve specific lint rules for improved performance. It will be invaluable for users to be able to record timing information and submit it to us without requiring them to pull up a profiler and submit a flamegraph profile. This doesn't entirely replace a profiler, but it allows us to get useful information without needing to recompile a binary or install additional tools. ### Example output Example output from `--debug timings` on the vscode repository (using dev mode, not release build, in case this seems really slow): ``` Found 7790 warnings and 19 errors. Finished in 16.6s on 6911 files with 95 rules using 8 threads. Rule timings: Rule Time (ms) Relative Calls Source ------------------------------------------------------- ---------- -------- ------- ------ eslint/no-unused-vars 4119.700 22.5% 6681 native eslint/no-unreachable 4116.910 22.5% 6900 native eslint/no-loss-of-precision 1329.986 7.3% 168327 native oxc/bad-array-method-on-arguments 849.118 4.6% 9749545 native eslint/no-useless-escape 633.403 3.5% 469909 native eslint/no-nonoctal-decimal-escape 602.055 3.3% 447866 native eslint/no-this-before-super 467.429 2.6% 3912 native typescript/no-unsafe-declaration-merging 429.882 2.4% 20334 native eslint/no-obj-calls 423.232 2.3% 602268 native eslint/no-useless-backreference 319.706 1.7% 605544 native eslint/no-dupe-keys 284.629 1.6% 109038 native eslint/no-unsafe-optional-chaining 246.120 1.3% 2036313 native eslint/no-control-regex 216.737 1.2% 605544 native eslint/no-misleading-character-class 212.035 1.2% 605544 native eslint/constructor-super 172.978 0.9% 11088 native eslint/no-caller 171.227 0.9% 946489 native eslint/no-const-assign 161.911 0.9% 167786 native unicorn/no-thenable 160.821 0.9% 851478 native eslint/no-iterator 155.057 0.8% 975576 native eslint/no-shadow-restricted-names 146.540 0.8% 6900 native eslint/no-dupe-class-members 135.977 0.7% 4144 native unicorn/prefer-set-size 135.732 0.7% 946489 native typescript/no-extra-non-null-assertion 134.364 0.7% 1523512 native oxc/bad-replace-all-arg 123.456 0.7% 551933 native eslint/no-eval 117.968 0.6% 774258 native oxc/only-used-in-recursion 108.291 0.6% 150511 native unicorn/no-await-in-promise-methods 103.633 0.6% 551933 native oxc/bad-min-max-func 102.570 0.6% 551933 native eslint/no-invalid-regexp 101.664 0.6% 602268 native eslint/no-dupe-else-if 97.380 0.5% 84274 native unicorn/prefer-string-starts-ends-with 94.667 0.5% 551933 native unicorn/no-invalid-remove-event-listener 93.116 0.5% 551933 native oxc/bad-char-at-comparison 91.417 0.5% 551933 native unicorn/no-single-promise-in-promise-methods 90.413 0.5% 551933 native eslint/no-extra-boolean-cast 90.250 0.5% 613702 native oxc/number-arg-out-of-range 87.906 0.5% 551933 native eslint/no-import-assign 82.042 0.4% 73720 native unicorn/no-useless-length-check 81.488 0.4% 42367 native unicorn/no-useless-spread 78.767 0.4% 178577 native typescript/no-unnecessary-parameter-property-assignment 76.019 0.4% 62949 native eslint/use-isnan 75.399 0.4% 650300 native eslint/no-unsafe-finally 70.008 0.4% 93520 native unicorn/no-invalid-fetch-options 68.221 0.4% 602268 native oxc/const-comparisons 54.044 0.3% 131693 native typescript/no-wrapper-object-types 52.855 0.3% 221028 native eslint/no-constant-binary-expression 48.784 0.3% 131693 native eslint/no-irregular-whitespace 39.979 0.2% 476825 native eslint/no-unassigned-vars 34.773 0.2% 167786 native eslint/no-unused-expressions 34.643 0.2% 329941 native eslint/no-sparse-arrays 31.352 0.2% 69539 native typescript/no-this-alias 29.074 0.2% 233601 native eslint/no-useless-rename 28.954 0.2% 142726 native eslint/no-constant-condition 25.769 0.1% 103602 native typescript/triple-slash-reference 22.440 0.1% 6798 native eslint/no-setter-return 22.301 0.1% 83251 native eslint/no-cond-assign 22.198 0.1% 172385 native eslint/no-compare-neg-zero 22.107 0.1% 89326 native eslint/no-global-assign 21.885 0.1% 6900 native typescript/prefer-as-const 21.868 0.1% 204458 native eslint/no-self-assign 18.860 0.1% 68783 native eslint/require-yield 17.908 0.1% 80796 native eslint/no-unused-private-class-members 17.573 0.1% 4144 native eslint/no-func-assign 17.390 0.1% 80796 native typescript/prefer-namespace-keyword 17.112 0.1% 783 native typescript/no-misused-new 15.415 0.1% 30084 native eslint/no-async-promise-executor 15.132 0.1% 50335 native oxc/missing-throw 14.423 0.1% 50335 native oxc/erasing-op 13.472 0.1% 89326 native oxc/bad-comparison-sequence 11.513 0.1% 89326 native eslint/no-delete-var 9.966 0.1% 43952 native oxc/bad-object-literal-comparison 9.699 0.1% 89326 native unicorn/no-new-array 9.666 0.1% 50335 native eslint/no-duplicate-case 9.498 0.1% 1520 native oxc/double-comparisons 9.466 0.1% 42367 native eslint/no-unsafe-negation 9.424 0.1% 89326 native eslint/no-class-assign 8.982 0.0% 11088 native unicorn/no-useless-fallback-in-spread 8.865 0.0% 42367 native oxc/uninvoked-array-callback 8.857 0.0% 50335 native eslint/no-ex-assign 8.852 0.0% 2729 native typescript/no-duplicate-enum-values 8.704 0.0% 1507 native eslint/valid-typeof 8.612 0.0% 43952 native eslint/no-new-native-nonconstructor 7.647 0.0% 50335 native eslint/no-empty-character-class 5.828 0.0% 3276 native typescript/no-useless-empty-export 4.494 0.0% 27137 native unicorn/no-empty-file 4.235 0.0% 6900 native unicorn/no-unnecessary-await 3.885 0.0% 31924 native eslint/no-unused-labels 2.308 0.0% 6900 native eslint/for-direction 1.938 0.0% 2837 native eslint/no-useless-catch 1.788 0.0% 3853 native eslint/getter-return 1.754 0.0% 1019 native typescript/no-non-null-asserted-optional-chain 1.734 0.0% 7549 native eslint/no-empty-pattern 1.226 0.0% 5829 native eslint/no-debugger 0.074 0.0% 1 native ```
b83a8f9 to
b46d4de
Compare
# Oxlint ### 🚀 Features - 5478fb5 linter/jsdoc: Implement `require-throws-description` rule (#22386) (Mikhail Baev) - b46d4de linter: Add `--debug` options and add per-rule timing info (#22282) (camchenry) - c73225e linter/eslint: Implement `prefer-arrow-callback` rule (#22312) (박천(Cheon Park)) - de82b59 linter: Add support for `eslint-plugin-jsx-a11y-x` (#22356) (mehm8128) - b170da3 linter: Implement no-implicit-globals (#22249) (Jovi De Croock) - f44b6c8 linter: Fill schemas `DummyRuleMap` with built-in rules (#22288) (Sysix) - 5cdb80d linter/jsx-a11y/: Implement no-interactive-element-to-noninteractive-role (#22332) (anarefolio) - 2749422 linter/jsx-a11y: Add no-noninteractive-element-interactions (#22337) (Pablo Tovar) - ba2a1d3 linter/jsdoc: Implement `require-throws-type` rule (#22358) (Mikhail Baev) - d90729d linter/jsx-a11y: Implement control-has-associated-label (#21985) (mehm8128) - 1d04903 linter/jsdoc: Implement `require-yields-type` rule (#22331) (Mikhail Baev) ### 🐛 Bug Fixes - 04c4609 linter/no-nullable-type-assertion-style: Mark as suggestion (#22450) (camc314) - 1c2b7ec linter/no-unused-vars: Handle shadowed self assignments (#22387) (camc314) - 9faa1d5 linter/no-noninteractive-tabindex: Check conditional expressions (#22435) (camc314) - 0854b3a linter/prefer-arrow-callback: Preserve TSX generic arrows in fixer (#22434) (camc314) - 410b814 linter: Supply `source_type` to codegen fixer (#22433) (camc314) - 3c1bb6f linter: Skip per-node dispatch for run_once-only rules in large files (#22398) (Connor Shea) - 5206cde linter/no-unused-vars: Improve type-only rest parameters diagnostic (#22385) (camc314) - c9a22b5 linter/consistent-function-scoping: Allow imported bindings (#22384) (camc314) - c1e966d linter: Report type-only unused parameters in no-unused-vars (#22368) (camchenry) - 4818d98 linter: Check whether path is under root before ignoring it (#20101) (Leonabcd123) - 41fcdcf linter: Fix rule count not including override rules (#19898) (Daniel Osmond) - 59b4f0e linter: Fix 'explicit-module-boundary-types' false positive with 'allowOverloadFunctions' (#22341) (camchenry) ### ⚡ Performance - 6d42395 linter: Narrow no-unsafe-optional-chaining dispatch (#22437) (camchenry) - 08595fb linter: Optimize no-unreachable (#22397) (camchenry) - 3b46a8d linter: Optimize `no-loss-of-precision` (#22395) (camchenry) - b3e2dc9 linter: Optimize `oxc/bad-array-method-on-arguments` (#22393) (camchenry) ### 📚 Documentation - dcbf62c linter: Remove some duplicate spaces (#22359) (camc314) # Oxfmt ### 💥 BREAKING CHANGES - 21bb5d1 oxfmt: [**BREAKING**] Avoid config pre-scan (#22258) (leaysgur) ### 🐛 Bug Fixes - 441d724 oxfmt: Fix "race probe" logic with unit tests (#22378) (leaysgur) - e49ee26 formatter: Respect `singleQuote` for jsdoc `import()` type paths (#22353) (Colin Lienard) - 43b9978 formatter/sort_imports: Treat subpath imports as internal (#22440) (leaysgur) - 7c5cfa0 formatter: Handle jsx trailing comment with parens (#22370) (leaysgur) - ac5f120 formatter: Fix erroneous formatting inside a template literal with parentheses (#22262) (Jovi De Croock) - 3c53a95 formatter/sort_imports: Handle ignore comment as boundary (#22369) (leaysgur) - 4dd83dd oxfmt: Send expandedStates variants as shared refs (#22366) (leaysgur) - 055cc61 formatter: Expand JSX logical chain with leading line comment (#22346) (leaysgur) - 8046222 formatter: Preserve type alias comment break (#22261) (Jovi De Croock) ### ⚡ Performance - 123c493 oxfmt: Reduce more syscalls (#22380) (leaysgur) Co-authored-by: overlookmotel <557937+overlookmotel@users.noreply.github.com>

This PR implements two separate but related things:
--debugoption for theoxlintCLI that accepts a comma separated list of flags like--debug foo,bar,baz. This allows adding more debug functionality in the future without needing to add lots of individual CLI options.--debug timingsoption that enables per-rule timing information to be recorded and output. This enables determining what the slowest rules are in a single run of the linter in the CLI.Rule timings
The primary goal of this altered approach to timing linter rules was: when timings are not enabled, it doesn't cost any performance.. To achieve that, we've added a
TIMINGSconst generic argument to all of the corerun,run_on_jest_node,run_once, etc. functions in the linter. This creates more code overall, because instead of having a single version of each function, now we have two: one where the compiler optimized it withTIMINGS = falseand one whereTIMINGS = true. We then switch between these two paths at runtime by passing--debug timings.So, was this successful? In short, yes. There is no significant difference between
oxlintinmainand this PR when it comes to performance, unless you enable per-rule timings (see #22282 (comment)). When you enable timing information, there is a 20-30% overhead in performance. This is because we have to record lots of time information and bookkeeping in hash maps to output the final information. This could be optimized down the line possibly, but it's not a primary goal.This is also kept fairly generic so that in future PRs we can add support for JS plugin timings and
tsgolintrule timings as well, if those are available.Overall, I think the increased maintenance burden is worth it. Even looking at the example output below, I can already see ways to improve specific lint rules for improved performance. It will be invaluable for users to be able to record timing information and submit it to us without requiring them to pull up a profiler and submit a flamegraph profile. This doesn't entirely replace a profiler, but it allows us to get useful information without needing to recompile a binary or install additional tools.
Example output
Example output from
--debug timingson the vscode repository (using dev mode, not release build, in case this seems really slow):