Skip to content

Copy the leading ASCII run in bulk when converting Latin-1 to rust::String - #8016

Merged
littledivy merged 1 commit into
mainfrom
littledivy/2026-10-26-fast-latin1-rust-string
Oct 27, 2026
Merged

littledivy merged 1 commit into
mainfrom
littledivy/2026-10-26-fast-latin1-rust-string

Conversation

@littledivy

Copy link
Copy Markdown
Member

rust::String::latin1() pushed every byte as a char. Rust JSG converts every one-byte string argument this way, so a 64 KiB ASCII string took 24.8 µs vs 2.0 µs in C++ JSG.

In this patch the leading ASCII run is found 32 bytes at a time and copied in bulk, and only the rest is encoded per character. The exact UTF-8 length is reserved up front.

CPU time per call (bench-jsg from #8013, --config=benchmark, 5 processes × 5 reps):

before after C++ JSG
ascii/16 46.9 ns 45.6 ns 28.5 ns
ascii/1024 445.5 ns 78.4 ns 54.8 ns
ascii/65536 24.8 µs 1.80 µs 2.04 µs
latin1/16 70.7 ns 51.3 ns 33.4 ns
latin1/65536 33.9 µs 34.9 µs 31.8 µs

@littledivy
littledivy requested review from a team as code owners October 26, 2026 22:56
@littledivy
littledivy requested a review from nathanwhit October 26, 2026 22:56
@clanky-the-clanker

clanky-the-clanker Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

✅ Approved — No issues found. The new Latin-1 to UTF-8 fast path is correct: the unchecked UTF-8 conversion only ever sees bytes that were checked to be ASCII, and the reserved capacity matches the exact output length.

The decision is advisory: this comment never approves, blocks or requests changes on the pull request.


✅ All checks passed

📋 Code Quality: ✅ LGTM

I found no problems in the new string_from_latin1 path.

  • Safe use of the unchecked conversion: leading_ascii_len only counts a 32-byte chunk when every byte in it is below 0x80. After the chunks, it finishes with a byte-by-byte is_ascii() scan. So the ascii slice holds only ASCII bytes, and str::from_utf8_unchecked is safe to call on it.
  • Exact capacity: a byte at or above 0x80 becomes 2 UTF-8 bytes and every other byte stays 1 byte. That makes len + count(rest >= 0x80) the exact output size, so the string never has to grow while it is filled. The prefix is all ASCII, so counting only over rest is correct.
  • Tests: the new test covers an empty input and ASCII prefixes just below, at and just above the 32-byte chunk size, and at several multiples of it. Each case is followed by non-ASCII bytes and an ASCII tail.

📝 Documentation: ✅ LGTM

I found no documentation problems. The new comments describe what the code does now, as the repository's comment guidelines ask.


Reviewed commit: 232dde4a

@ask-bonk

ask-bonk Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Since last review: 0 resolved, 0 still open, 0 new.
LGTM!


Reviewed commit: 232dde4a · github run

…tring

rust::String::latin1() pushed every byte as a char, one at a time. Rust
JSG converts every one-byte string argument this way, so a 64 KiB ASCII
string took 24.8 us, against 2.0 us in C++ JSG.

In this patch string_from_latin1 finds the leading ASCII run 32 bytes at
a time, copies it in bulk and encodes only the rest per character. It
also reserves the exact UTF-8 length, so non-ASCII strings no longer
regrow while being built.

CPU time per call to a method taking a string (bench-jsg from #8013,
--config=benchmark, 5 processes x 5 reps):

  case          before     after      C++ JSG
  ascii/16      46.9 ns    45.6 ns    28.5 ns
  ascii/1024    445.5 ns   78.4 ns    54.8 ns
  ascii/65536   24.8 us    1.80 us    2.04 us
  latin1/16     70.7 ns    51.3 ns    33.4 ns
  latin1/65536  33.9 us    34.9 us    31.8 us
@littledivy
littledivy force-pushed the littledivy/2026-10-26-fast-latin1-rust-string branch from 301b03e to 232dde4 Compare October 26, 2026 22:59
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 41.96%. Comparing base (2c37de0) to head (232dde4).
⚠️ Report is 40 commits behind head on main.

Files with missing lines Patch % Lines
src/rust/cxx/src/symbols/rust_string.rs 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8016      +/-   ##
==========================================
- Coverage   42.05%   41.96%   -0.10%     
==========================================
  Files        1061     1064       +3     
  Lines      301008   302174    +1166     
  Branches    29043    29082      +39     
==========================================
+ Hits       126599   126803     +204     
- Misses     157389   158340     +951     
- Partials    17020    17031      +11     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@littledivy
littledivy merged commit 4c3985e into main Oct 27, 2026
29 of 30 checks passed
@littledivy
littledivy deleted the littledivy/2026-10-26-fast-latin1-rust-string branch October 27, 2026 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants