perf(runtime): add ASCII fast path to toLowerCase/toUpperCase - #10117
perf(runtime): add ASCII fast path to toLowerCase/toUpperCase#10117proggeramlug wants to merge 2 commits into
Conversation
case_convert ran every input, including pure ASCII, through a scalar wtf8_step decode / per-char to_lowercase()/to_uppercase() iterator / re-encode loop, costing 30-33x Node on a 1M-char all-ASCII string. Gate on a real per-byte bytes.is_ascii() scan (not the is_ascii_string byte_len==utf16_len aggregate proxy, which can lie for malformed WTF-8) and use to_ascii_lowercase()/to_ascii_uppercase() for a vectorizable byte-table transform instead. Non-ASCII input, locale-aware casing, and WTF-8/lone-surrogate handling are untouched. Claude-Session: https://claude.ai/code/session_013naeTjgijAXt8PwpQEKkbu
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthrough
ChangesASCII case conversion
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Refactor · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The ASCII optimization preserves the existing non-ASCII conversion path and is covered for the relevant Unicode and malformed-string boundaries. No actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The ASCII fast path uses
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Claude-Session: https://claude.ai/code/session_013naeTjgijAXt8PwpQEKkbu (cherry picked from commit aa1e9e1)
|
Landed on Your commits are on Closing this PR as landed — GitHub cannot auto-close it because the train merges as its own branch. |
Summary
toLowerCase()/toUpperCase()cost 30-33x Node on pure-ASCII input becausecase_convertran every input through a scalarwtf8_stepdecode / per-charchar::to_lowercase()/to_uppercase()iterator / re-encode loop, even when the input was pure ASCII.case_convertnow checksbytes.is_ascii()(a real per-byte scan) and, when true, produces the result with a singleto_ascii_lowercase()/to_ascii_uppercase()byte-table transform, skipping the scalar decode loop entirely. Everything else (Unicode special casing, WTF-8/lone-surrogate handling, locale-aware casing inlocale.rs) is unchanged.is_ascii_string(s)helper (byte_len == utf16_len) as the gate — that's only an aggregate proxy and can be true for malformed WTF-8 where a stray continuation byte and a truncated multi-byte lead cancel out in the unit count, even though the bytes aren't ASCII. A regression test (case_convert_rejects_the_aggregate_ascii_lie) locks this in; I verified it actually fails when the gate is swapped back tois_ascii_string(s).Testing
RUST_TEST_THREADS=1 cargo test --release -p perry-runtime string::— all 129 string-module tests pass, including the new ones covering: basic ASCII correctness (empty/single-char/already-in-target-case/flags/utf16_len), an ASCII prefix followed by a lone surrogate (must NOT take the fast path, flag preserved), and the aggregate-lie regression above.test-files/test_gap_10090_string_case_ascii_fastpath.tswith the release binary and diffed byte-for-byte againstnode --experimental-strip-types(Node v26.5.1, matching.node-version): identical output for ASCII fast-path cases,ß→SS/ẞ→ß, default-localei/I(not the Turkish rule), the default-localeİ→i+combining-dot special casing, Cherokee, Deseret (astral), an ASCII string with one trailing multi-byte character, and an ASCII prefix + lone surrogate.cargo fmt --all -- --checkclean.perry-runtime).Note
While writing the correctness gap test I found that Greek
Σ-at-word-end lowercasing doesn't apply the Unicode conditionalFinal_Sigmarule (toLowerCase()always produces medialσ, never finalς) — this is a pre-existing bug in the untouched scalar path (Rust'schar::to_lowercase()has no notion of that conditional rule), unaffected by this fix since Greek text is non-ASCII either way. Filed separately as #10116 and excluded from this PR's gap test with a comment pointing there.Per instruction, this PR does not bump
[workspace.package] versioninCargo.tomlor theCurrent Versionline inCLAUDE.md.https://claude.ai/code/session_013naeTjgijAXt8PwpQEKkbu
Summary by CodeRabbit
Performance
toLowerCase()andtoUpperCase()performance for pure-ASCII strings.Bug Fixes