Skip to content

perf(linter/only-used-in-recursion): improve skip_to_next_char slicing#17374

Merged
graphite-app[bot] merged 1 commit intomainfrom
12-25-perf_linter_only-used-in-recursion_improve_skip_to_next_char_slicing
Dec 27, 2025
Merged

perf(linter/only-used-in-recursion): improve skip_to_next_char slicing#17374
graphite-app[bot] merged 1 commit intomainfrom
12-25-perf_linter_only-used-in-recursion_improve_skip_to_next_char_slicing

Conversation

@camchenry
Copy link
Member

@camchenry camchenry commented Dec 26, 2025

This only really has an effect on the react.development.js benchmark, where I noticed we end up spending a lot of time in skip_to_next_char.

Instead of iterating over the entire source, we slice from the start index and use char_indices from there. The start index should always be at a char boundary, so there shouldn't be any risk of UTF-8 issues at slicing in middle of a character.

I also took the opportunity to improve how trailing commas are handled and make the fix a little bit better and add another test case.

image

Copy link
Member Author

camchenry commented Dec 26, 2025


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 C-performance Category - Solution not expected to change functional behavior, only performance labels Dec 26, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 26, 2025

CodSpeed Performance Report

Merging #17374 will not alter performance

Comparing 12-25-perf_linter_only-used-in-recursion_improve_skip_to_next_char_slicing (75df67b) with 12-24-perf_linter_inline_is_function_node_into_run_functions (23857c2)

Summary

✅ 4 untouched
⏩ 41 skipped1

Footnotes

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

@graphite-app graphite-app bot force-pushed the 12-24-perf_linter_inline_is_function_node_into_run_functions branch 2 times, most recently from 571e6dc to e188d8c Compare December 26, 2025 16:39
@graphite-app graphite-app bot force-pushed the 12-25-perf_linter_only-used-in-recursion_improve_skip_to_next_char_slicing branch from 79e0eff to 43466f1 Compare December 26, 2025 16:39
@camchenry camchenry marked this pull request as ready for review December 27, 2025 01:59
@camchenry camchenry requested a review from camc314 as a code owner December 27, 2025 01:59
@camchenry camchenry force-pushed the 12-24-perf_linter_inline_is_function_node_into_run_functions branch from e188d8c to 23857c2 Compare December 27, 2025 02:42
@camchenry camchenry force-pushed the 12-25-perf_linter_only-used-in-recursion_improve_skip_to_next_char_slicing branch from 43466f1 to 75df67b Compare December 27, 2025 02:42
Copy link
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍💪

@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Dec 27, 2025
@camc314 camc314 self-assigned this Dec 27, 2025
Copy link
Contributor

camc314 commented Dec 27, 2025

Merge activity

…ing (#17374)

This only really has an effect on the `react.development.js` benchmark, where I noticed we end up spending a lot of time in `skip_to_next_char`.

Instead of iterating over the entire source, we slice from the start index and use `char_indices` from there. The start index _should_ always be at a char boundary, so there shouldn't be any risk of UTF-8 issues at slicing in middle of a character.

I also took the opportunity to improve how trailing commas are handled and make the fix a little bit better and add another test case.

<img width="730" height="441" alt="image" src="https://github.com/user-attachments/assets/680c5cfe-7cf1-4760-926f-fb2a1dc25a8f" />
@graphite-app graphite-app bot force-pushed the 12-24-perf_linter_inline_is_function_node_into_run_functions branch from 23857c2 to eecee5d Compare December 27, 2025 11:52
@graphite-app graphite-app bot force-pushed the 12-25-perf_linter_only-used-in-recursion_improve_skip_to_next_char_slicing branch from 75df67b to c27514c Compare December 27, 2025 11:52
Base automatically changed from 12-24-perf_linter_inline_is_function_node_into_run_functions to main December 27, 2025 11:57
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Dec 27, 2025
@graphite-app graphite-app bot merged commit c27514c into main Dec 27, 2025
21 checks passed
@graphite-app graphite-app bot deleted the 12-25-perf_linter_only-used-in-recursion_improve_skip_to_next_char_slicing branch December 27, 2025 11:57
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.

2 participants