perf(linter/jsx-no-comment-textnodes): remove regex for checking comment patterns #6534

camchenry · 2024-10-13T20:20:09Z

Improves performance of this rule by replacing Regex with simpler string methods. On the RadixUIAdoptionSection.jsx file alone, this improves performance by ~5%:

hyperfine --warmup 100 --shell=none --runs 3000 './oxlint-main -W jsx-no-comment-textnodes --silent ./RadixUIAdoptionSection.jsx' './oxlint-jsx-no-comment-textnodes-no-regex -W jsx-no-comment-textnodes --silent ./RadixUIAdoptionSection.jsx'
Benchmark 1: ./oxlint-main -W jsx-no-comment-textnodes --silent ./RadixUIAdoptionSection.jsx
  Time (mean ± σ):       4.4 ms ±   0.1 ms    [User: 1.7 ms, System: 1.9 ms]
  Range (min … max):     4.1 ms …   5.3 ms    3000 runs

Benchmark 2: ./oxlint-jsx-no-comment-textnodes-no-regex -W jsx-no-comment-textnodes --silent ./RadixUIAdoptionSection.jsx
  Time (mean ± σ):       4.2 ms ±   0.1 ms    [User: 1.5 ms, System: 1.8 ms]
  Range (min … max):     3.9 ms …   5.0 ms    3000 runs

Summary
  ./oxlint-jsx-no-comment-textnodes-no-regex -W jsx-no-comment-textnodes --silent ./RadixUIAdoptionSection.jsx ran
    1.06 ± 0.05 times faster than ./oxlint-main -W jsx-no-comment-textnodes --silent ./RadixUIAdoptionSection.jsx

graphite-app · 2024-10-13T20:20:14Z

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

camchenry · 2024-10-13T20:20:22Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @camchenry and the rest of your teammates on Graphite

codspeed-hq · 2024-10-13T20:26:54Z

CodSpeed Performance Report

Merging #6534 will not alter performance

_{Comparing 10-13-perf_linter_jsx-no-comment-textnodes_remove_regex_for_checking_comment_patterns (62afaa9) with main (8d9a2da)}

Summary

✅ 30 untouched benchmarks

camchenry · 2024-10-14T19:56:31Z

@DonIsaac i think this is ready to be review. I was going to wait until we had the small file benchmark, but I think there's enough data to show this is most likely a perf win.

crates/oxc_linter/src/rules/react/jsx_no_comment_textnodes.rs

camchenry · 2024-10-15T04:09:17Z

Merge activity

Oct 15, 12:09 AM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
Oct 15, 12:09 AM EDT: A user added this pull request to the Graphite merge queue.
Oct 15, 12:09 AM EDT: The Graphite merge queue couldn't merge this PR because it failed for an unknown reason.
Oct 15, 1:02 AM EDT: A user added this pull request to the Graphite merge queue.
Oct 15, 1:07 AM EDT: A user merged this pull request with the Graphite merge queue.

…ent patterns (#6534) Improves performance of this rule by replacing `Regex` with simpler string methods. On the `RadixUIAdoptionSection.jsx` file alone, this improves performance by ~5%: ``` hyperfine --warmup 100 --shell=none --runs 3000 './oxlint-main -W jsx-no-comment-textnodes --silent ./RadixUIAdoptionSection.jsx' './oxlint-jsx-no-comment-textnodes-no-regex -W jsx-no-comment-textnodes --silent ./RadixUIAdoptionSection.jsx' Benchmark 1: ./oxlint-main -W jsx-no-comment-textnodes --silent ./RadixUIAdoptionSection.jsx Time (mean ± σ): 4.4 ms ± 0.1 ms [User: 1.7 ms, System: 1.9 ms] Range (min … max): 4.1 ms … 5.3 ms 3000 runs Benchmark 2: ./oxlint-jsx-no-comment-textnodes-no-regex -W jsx-no-comment-textnodes --silent ./RadixUIAdoptionSection.jsx Time (mean ± σ): 4.2 ms ± 0.1 ms [User: 1.5 ms, System: 1.8 ms] Range (min … max): 3.9 ms … 5.0 ms 3000 runs Summary ./oxlint-jsx-no-comment-textnodes-no-regex -W jsx-no-comment-textnodes --silent ./RadixUIAdoptionSection.jsx ran 1.06 ± 0.05 times faster than ./oxlint-main -W jsx-no-comment-textnodes --silent ./RadixUIAdoptionSection.jsx ```

## [0.10.0] - 2024-10-18 - 7f6b219 editor/vscode: [**BREAKING**] Unify configuration logic (#6630) (DonIsaac) - 782f0a7 codegen: [**BREAKING**] Rename `print_char` method to `print_ascii_byte` (#6512) (overlookmotel) - 7645e5c codegen: [**BREAKING**] Remove CommentOptions API (#6451) (Boshen) - 5200960 oxc: [**BREAKING**] Remove passing `Trivias` around (#6446) (Boshen) - 80266d8 linter: [**BREAKING**] Support plugins in oxlint config files (#6088) (DonIsaac) ### Features - 6f22538 ecmascript: Add `ToBoolean`, `ToNumber`, `ToString` (#6502) (Boshen) - 1e7fab3 linter: Implement `no-callback-in-promise` (#6157) (dalaoshu) - c56343d linter: Promote `no_unsafe_optional_chaining` to correctness (#6491) (Boshen) - 454874a linter: Implement `react/iframe-missing-sandbox` (#6383) (Radu Baston) - c8174e2 linter: Add suggestions for `no-plusplus` (#6376) (camchenry) - 6e3224d linter: Configure by category in config files (#6120) (DonIsaac) - c5e66e1 linter/no-unused-vars: Report own type references within class, interface, and type alias declarations (#6557) (DonIsaac) - 8c78f97 linter/node: Implement no-new-require (#6165) (Jelle van der Waa) ### Bug Fixes - cf92730 editor: Use human-readable output channel names (#6629) (DonIsaac) - d9159a2 editor: Misaligned command prefixes (#6628) (DonIsaac) - b9c94bb editors/vscode: Temporarily solve oxc_language_server issue on windows (#6384) (dalaoshu) - e340424 linter: Support import type with namespaced import in `import/no-duplicates` (#6650) (Dmitry Zakharov) - a668397 linter: Panic in `no-else-return` (#6648) (dalaoshu) - 41dc8e3 linter: Stack overflow in `oxc/no-async-endpoint-handlers` (#6614) (DonIsaac) - d07a9b0 linter: Panic in `no-zero-fractions` (#6607) (dalaoshu) - d6a0d2e linter: Fix file name checking behavior of `unicorn/filename-case` (#6463) (camchenry) - 0784e74 linter: Error fixer of `switch-case-braces` (#6474) (dalaoshu) - e811812 linter: Error diagnostic message based on parameter length of valid-expect (#6455) (dalaoshu) - f71c91e linter: Move `eslint/sort-keys` to `style` category (#6377) (DonIsaac) - 2b86de9 linter/no-control-regex: False negative for flags in template literals (#6531) (DonIsaac) - 685a590 linter/no-control-regex: Better diagnostic messages (#6530) (DonIsaac) - 6d5a9f2 linter/no-control-regex: Allow capture group references (#6529) (DonIsaac) - ba53bc9 linter/no-unused-vars: False positives in TS type assertions (#6397) (DonIsaac) - d3e59c6 linter/no-unused-vars: False positive in some default export cases (#6395) (DonIsaac) - e08f956 linter/no-unused-vars: False positive for functions and classes in arrays (#6394) (DonIsaac) - b9d7c5f no-unused-vars: Consider functions within conditional expressions usable (#6553) (Brian Donovan) ### Performance - 0cbd4d0 linter: Avoid megamorphism in `RuleFixer` methods (#6606) (DonIsaac) - 725f9f6 linter: Get fewer parent nodes in `unicorn/prefer-dom-node-text-content` (#6467) (camchenry) - c00f669 linter: Use NonZeroUsize for pending module cache entries (#6439) (DonIsaac) - a1a2721 linter: Replace `ToString::to_string` with `CompactStr` in remaining rules (#6407) (camchenry) - c5c69d6 linter: Use `CompactStr` in `valid-title` (#6406) (camchenry) - d66e826 linter: Use `CompactStr` in `prefer-lowercase-title` (#6405) (camchenry) - 889400c linter: Use `CompactStr` for `get_node_name` in Jest rules (#6403) (camchenry) - 9906849 linter: Use `CompactStr` in `no-large-snapshots` (#6402) (camchenry) - c382ec4 linter: Use `CompactStr` in `no-hooks` (#6401) (camchenry) - 24a5d9b linter: Use `CompactStr` in `expect-expect` (#6400) (camchenry) - 71dbdad linter: Use `CompactStr` in `no-console` (#6399) (camchenry) - f5f00a1 linter: Use `CompactStr` in `no-bitwise` (#6398) (camchenry) - 62afaa9 linter/jsx-no-comment-textnodes: Remove regex for checking comment patterns (#6534) (camchenry) - b3d0cce linter/no-unescaped-entities: Add fast path to check if char should be replaced (#6594) (camchenry) - ee73f56 linter/no-unused-vars: Do not construct `Regex` for default ignore pattern (#6590) (camchenry) - 77ddab8 linter/numeric-separators-style: Replace regex with number parser (#6546) (camchenry) - 8f47cd0 linter/react: Remove regex patterns in `no-unknown-property` (#6536) (camchenry) ### Documentation - 557f941 linter: Add docs to no-unused-vars and Tester (#6558) (DonIsaac) ### Refactor - ecce5c5 linter: Improve recursive argument handling and diagnostics creation (#6513) (no-yan) - f960e9e linter: Add suggested file names for `unicorn/filename-case` (#6465) (camchenry) - 7240ee2 linter: Make advertised fix kinds consistent (#6461) (Alexander S.) - b48c368 linter: `no_global_assign` rule: reduce name lookups (#6460) (overlookmotel) - 2566ce7 linter: Remove OxlintOptions (#6098) (DonIsaac) - 002078a linter: Make Runtime's members private (#6440) (DonIsaac) - 6a0a533 linter: Move module cache logic out of Runtime (#6438) (DonIsaac) - c18c6e9 linter: Split service code into separate modules (#6437) (DonIsaac) - 5ea9ef7 linter: Improve labels and help message for `eslint/no-useless-constructor` (#6389) (DonIsaac) - 2c32dac linter/no-control-regex: Remove duplicate code (#6527) (DonIsaac) - 435a89c oxc: Remove useless `allocator.alloc(program)` calls (#6571) (Boshen) - f70e93b oxc: Ban index methods on std::str::Chars (#6075) (dalaoshu) ### Testing - a6cae98 linter: Make sure all auto-fixing rules have fixer test (#6378) (DonIsaac) - 06b09b2 linter/no-unused-vars: Enable now-passing tests (#6556) (DonIsaac) - badd11c linter/no-unused-vars: Ignored catch parameters (#6555) (DonIsaac) - 84aa2a2 linter/no-useless-constructor: Add cases for initializers in subclass constructors (#6390) (DonIsaac) --------- Co-authored-by: DonIsaac <22823424+DonIsaac@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

github-actions bot added the A-linter Area - Linter label Oct 13, 2024

camchenry mentioned this pull request Oct 13, 2024

perf(linter/react): remove regex patterns in no-unknown-property #6536

Merged

camchenry marked this pull request as ready for review October 13, 2024 22:18

camchenry marked this pull request as draft October 13, 2024 22:18

camchenry mentioned this pull request Oct 14, 2024

perf(linter/numeric-separators-style): replace regex with number parser #6546

Merged

camchenry marked this pull request as ready for review October 14, 2024 17:17

camchenry requested a review from DonIsaac October 14, 2024 19:55

DonIsaac approved these changes Oct 14, 2024

View reviewed changes

crates/oxc_linter/src/rules/react/jsx_no_comment_textnodes.rs Show resolved Hide resolved

camchenry force-pushed the 10-13-perf_linter_jsx-no-comment-textnodes_remove_regex_for_checking_comment_patterns branch from 63882d5 to 9f6ba32 Compare October 15, 2024 03:14

github-actions bot added the C-performance Category - Solution not expected to change functional behavior, only performance label Oct 15, 2024

camchenry added the 0-merge Merge with Graphite Merge Queue label Oct 15, 2024

graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Oct 15, 2024

Boshen added the 0-merge Merge with Graphite Merge Queue label Oct 15, 2024 — with Graphite App

Boshen force-pushed the 10-13-perf_linter_jsx-no-comment-textnodes_remove_regex_for_checking_comment_patterns branch from 9f6ba32 to 62afaa9 Compare October 15, 2024 05:02

graphite-app bot merged commit 62afaa9 into main Oct 15, 2024
26 checks passed

graphite-app bot deleted the 10-13-perf_linter_jsx-no-comment-textnodes_remove_regex_for_checking_comment_patterns branch October 15, 2024 05:07

oxc-bot mentioned this pull request Oct 18, 2024

release(oxlint): v0.10.0 #6661

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf(linter/jsx-no-comment-textnodes): remove regex for checking comment patterns #6534

perf(linter/jsx-no-comment-textnodes): remove regex for checking comment patterns #6534

camchenry commented Oct 13, 2024 •

edited

Loading

graphite-app bot commented Oct 13, 2024

camchenry commented Oct 13, 2024 •

edited

Loading

codspeed-hq bot commented Oct 13, 2024 •

edited

Loading

camchenry commented Oct 14, 2024

camchenry commented Oct 15, 2024 •

edited by Boshen

Loading

perf(linter/jsx-no-comment-textnodes): remove regex for checking comment patterns #6534

perf(linter/jsx-no-comment-textnodes): remove regex for checking comment patterns #6534

Conversation

camchenry commented Oct 13, 2024 • edited Loading

graphite-app bot commented Oct 13, 2024

Your org has enabled the Graphite merge queue for merging into main

camchenry commented Oct 13, 2024 • edited Loading

codspeed-hq bot commented Oct 13, 2024 • edited Loading

CodSpeed Performance Report

Merging #6534 will not alter performance

Summary

camchenry commented Oct 14, 2024

camchenry commented Oct 15, 2024 • edited by Boshen Loading

Merge activity

camchenry commented Oct 13, 2024 •

edited

Loading

camchenry commented Oct 13, 2024 •

edited

Loading

codspeed-hq bot commented Oct 13, 2024 •

edited

Loading

camchenry commented Oct 15, 2024 •

edited by Boshen

Loading