perf(string): single-alloc join/repeat/replace_once with safety fixes - #5506
Closed
xcb3d wants to merge 3 commits into
Closed
perf(string): single-alloc join/repeat/replace_once with safety fixes#5506xcb3d wants to merge 3 commits into
xcb3d wants to merge 3 commits into
Conversation
- Add JsString::repeat/repeat_str with exponential doubling and
checked overflow handling; repeat(1) shares via clone
- Add JsString::join with single allocation, empty-neutral Latin1
detection, zero-len early return and hoisted separator variant
- Add JsString::replace_once/replace_once_at reusing index_of,
checked total_len and empty-neutral encoding
- Dedupe JsStr::index_of into shared same/mixed helpers with
first-byte skip and debug asserts
- Fix Array.prototype.join self-check to object identity only
(join.call('a') no longer returns empty)
- Fix unsafe pointer writes via &raw mut, document nan-boxed
as_str lifetime, harden build_as_latin1 truncation docs
- Intern trim results back to static strings; hoist pad filler
variant; reuse replace position to avoid double search
- Add coverage for join/index_of/replace_once/repeat/trim
Use fully-qualified paths (crate::JsString, crate::JsValue) in new as_str/repeat docs so cargo doc --document-private-items passes with denied warnings.
Test262 conformance changes
Tested main commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5506 +/- ##
===========================================
+ Coverage 47.24% 62.81% +15.57%
===========================================
Files 476 536 +60
Lines 46892 60616 +13724
===========================================
+ Hits 22154 38077 +15923
+ Misses 24738 22539 -2199 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
|
This is fixing too many things at once; 800+ lines of new code (some of them being unsafe). Please split your optimizations into multiple PRs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Single-allocation fast paths for the
boa_stringhot paths (join/repeat/replace_once/index_of), plus the safety fixes found during maintainer review.It changes the following:
JsString::repeat/repeat_strwith exponential doubling and checked overflow handling;repeat(1)shares viacloneJsString::joinwith single allocation, empty-neutral Latin1 detection, zero-len early return and hoisted separator variantJsString::replace_once/replace_once_atreusingindex_of, checkedtotal_lenand empty-neutral encodingJsStr::index_ofinto shared same/mixed helpers with first-byte skip and debug assertsArray.prototype.joinself-check to object identity only (join.call('a')no longer returns empty)&raw mut, document nan-boxedas_strlifetime, hardenbuild_as_latin1truncation docsjoin/index_of/replace_once/repeat/trimPerformance
Criterion
boa_benches --bench scripts -- strings(release), same machine, baseline (upstream HEAD665f0392) vs patched — 2 runs each, median:strings/splitstrings/replacestrings/concat+=concatenationsstrings/sliceWhy:
split("a")calls the 1-charindex_offast path (position()directly instead ofwindows().position()) thousands of times;replacebenefits from the single-allocreplace_once+ no double search (replace_once_at);concat/slicegain indirectly via zero-copyas_strand doublingrepeatin setup.Targeted JS micro-bench (debug binary,
Date.now()):repeat-100k3 ms → 1 ms (~3x),replace-match-10k97 ms → 73 ms (~25%),concat-5k44 ms → 38 ms (~14%).Verification
cargo test -p boa_string --lib: 30/30 ✅ (4 new test fns)cargo test -p boa_engine --lib: 1120/0 ✅cargo clippy -p boa_string --all-targets,cargo clippy -p boa_engine --lib: clean ✅cargo fmt --check,git diff --check: clean ✅cargo +nightly miri test -p boa_string --lib: 30/30 ✅cargo make run-ci(fmt + lint all/no-features): pass ✅Machine config