perf(semantic): use SIMD for finding backslashes in check_string_literal#17534
Conversation
github-actions
Bot
added
A-semantic
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. |
CodSpeed Performance ReportMerging #17534 will not alter performanceComparing Summary
Footnotes
|
b1708a6 to
203ee7f
Compare
camchenry
changed the title
check_string_literal
There was a problem hiding this comment.
Pull request overview
This PR optimizes the performance of check_string_literal by using SIMD instructions for detecting backslashes in string literals. Profiling identified this function as a hot path that was using a naive byte-by-byte scan.
Key changes:
- Introduces a two-path approach: SIMD-optimized
memchrfor strings ≥16 bytes, byte-by-byte fallback for smaller strings - Adds early return for non-strict mode to avoid unnecessary processing
- Changes from character-based to byte-based processing (safe for ASCII escape sequences)
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/oxc_semantic/src/checker/javascript.rs | Implements dual-path string literal checking with SIMD optimization for longer strings and maintains byte-by-byte fallback for shorter strings |
| crates/oxc_semantic/Cargo.toml | Adds memchr dependency for SIMD-optimized searching |
| Cargo.lock | Updates lock file with memchr dependency and incidental version bumps for cc and find-msvc-tools |
After thoroughly reviewing this PR, I found the implementation to be correct and well-designed:
Logic Correctness: I verified the escape sequence handling logic through extensive analysis of edge cases, including:
- Escaped backslashes (
\\) - Multiple consecutive backslashes
- Octal escape sequences (
\0through\7) - Non-octal decimal escapes (
\8and\9) - The
skip_next_backslashmechanism correctly handles escaped backslashes in the SIMD path
Design Decisions: The code duplication between the SIMD and fallback paths is intentional and acceptable for a performance optimization. The 16-byte threshold appears reasonable for SIMD overhead, and memchr is already used elsewhere in the codebase, demonstrating consistency with project practices.
Behavioral Equivalence: Both paths produce identical results, and the early return optimization for non-strict mode is a sensible improvement that avoids unnecessary work.
No issues requiring comments were identified.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merge activity
|
…eral` (#17534) Profiling shows that `check_string_literal` is called fairly often. We use a naive byte-by-byte check here for finding backslashes for octal escape sequences. This should be optimized to something wider than processing a single byte at a time, but it's worth ensuring this actually happens. This PR changes it so that when the minimum string length is met, we use the SIMD-optimized `memchr` crate to find backslashes instead. <img width="696" height="352" alt="image" src="https://github.com/user-attachments/assets/58c67f78-7b91-4ab1-aa45-68a175c38e6c" />
203ee7f to
04809d1
Compare
### 🚀 Features - 659c23e linter: Init note field boilerplate (#17589) (Shrey Sudhir) - 6870b64 parser: Add TS1363 error code (#17609) (Sysix) - 23680a3 mangler: Skip mangling only in scopes affected by direct eval (#17612) (camc314) - a7e1643 parser: Add TS2528 error code to duplicate_default_export diagnostic (#17558) (camc314) ### 🐛 Bug Fixes - 1044116 ecmascript: Mark `new Symbol` as non side-effect free (#17568) (camc314) - ab5e4ca isolated-declarations: Strip default values from rest parameter binding patterns (#17602) (camc314) - 68b2e54 minifier: Prevent incorrect ??= transformation when member base is mutated (#17472) (copilot-swe-agent) ### ⚡ Performance - 6067143 semantic: Remove hash when checking identifier (#17564) (camchenry) - a28ab3d semantic: Avoid bounds check when checking string literal (#17545) (camc314) - 04809d1 semantic: Use SIMD for finding backslashes in `check_string_literal` (#17534) (camchenry) - 49ad2f0 semantic: Mark all diagnostic functions as `#[cold]` (#17487) (camc314) - ea82b50 transformer: Mark all diagnostic functions as `#[cold]` (#17486) (camc314) - d968e51 semantic: Mark `checker::check` as `inline(always)` (#17459) (camc314)
Profiling shows that
check_string_literalis called fairly often. We use a naive byte-by-byte check here for finding backslashes for octal escape sequences. This should be optimized to something wider than processing a single byte at a time, but it's worth ensuring this actually happens.This PR changes it so that when the minimum string length is met, we use the SIMD-optimized
memchrcrate to find backslashes instead.