Skip to content

perf(string): single-alloc join/repeat/replace_once with safety fixes #5506

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
xcb3d wants to merge 3 commits into from
Closed

perf(string): single-alloc join/repeat/replace_once with safety fixes #5506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ccb5dd1
perf(string): single-alloc join/repeat/replace_once with safety fixes
xcb3d Sep 4, 2026
be98376
fix(docs): resolve broken intra-doc links for -D warnings
xcb3d Sep 4, 2026
a63a92f
fix(value): correct JsValue::as_str doctest and intra-doc links
xcb3d Sep 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 28 additions & 14 deletions core/engine/src/builtins/array/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1002,28 +1002,42 @@ impl Array {
};

// 5. Let R be the empty String.
let mut r = Vec::with_capacity(len as usize + len.saturating_sub(1) as usize);
if len == 0 {
return Ok(StaticJsStrings::EMPTY_STRING.into());
}

let mut elements = Vec::new();
// 6. Let k be 0.
// 7. Repeat, while k < len,
for k in 0..len {
// a. If k > 0, set R to the string-concatenation of R and sep.
if k > 0 {
r.push(separator.clone());
}
// b. Let element be ? Get(O, ! ToString(𝔽(k))).
let element = o.get(k, context)?;
// c. If element is undefined, null or the array itself, let next be the empty String; otherwise, let next be ? ToString(element).
let next = if element.is_null_or_undefined() || &element == this {
js_string!()
} else {
element.to_string(context)?
};
// d. Set R to the string-concatenation of R and next.
r.push(next.clone());
// c. If element is undefined or null, let next be the empty String;
// otherwise, let next be ? ToString(element).
// NOTE: Boa additionally maps a self-referential element (identity,
// object-only) to the empty string to avoid infinite recursion.
// This must be an identity check: value equality would wrongly
// match primitives, e.g. `Array.prototype.join.call("a", "-")`.
let next =
if element.is_null_or_undefined() || (element.is_object() && &element == this) {
StaticJsStrings::EMPTY_STRING
} else {
element.to_string(context)?
};
// d. Append element to list.
elements.push(next);
// e. Set k to k + 1.
}
// Fast path: avoid copying a single huge element through `join`.
if elements.len() == 1 {
// SAFETY: just checked len.
return Ok(elements.pop().expect("len == 1").into());
}
// `elements` outlives `str_refs`, so the borrowed `JsStr`s stay valid
// for the duration of `JsString::join`.
let str_refs: Vec<_> = elements.iter().map(JsString::as_str).collect();
// 8. Return R.
Ok(js_string!(&r[..]).into())
Ok(JsString::join(separator.as_str(), &str_refs).into())
}

/// `Array.prototype.toString( separator )`
Expand Down
208 changes: 158 additions & 50 deletions core/engine/src/builtins/string/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,18 +653,28 @@ impl String {
let this = this.require_object_coercible()?;

// 2. Let S be ? ToString(O).
let mut string = this.to_string(context)?;
let string = this.to_string(context)?;

if args.is_empty() {
return Ok(JsValue::new(string));
}

// 3. Let R be S.
// 4. For each element next of args, do
if args.len() == 1 {
let next_string = args[0].to_string(context)?;
return Ok(JsValue::new(js_string!(&string, &next_string)));
}

let mut strings = Vec::with_capacity(args.len() + 1);
strings.push(string);
for arg in args {
// a. Let nextString be ? ToString(next).
// b. Set R to the string-concatenation of R and nextString.
string = js_string!(&string, &arg.to_string(context)?);
strings.push(arg.to_string(context)?);
}

// `strings` outlives `str_refs`, so the borrowed `JsStr`s stay valid
// for the duration of `concat_array`.
let str_refs: Vec<_> = strings.iter().map(JsString::as_str).collect();
// 5. Return R.
Ok(JsValue::new(string))
Ok(JsValue::new(JsString::concat_array(&str_refs)))
}

/// `String.prototype.repeat( count )`
Expand Down Expand Up @@ -734,14 +744,12 @@ impl String {
let n = n as usize;

// Charge each repetition against the VM loop-iteration limit.
let mut result = Vec::with_capacity(n);
for _ in 0..n {
crate::vm::opcode::IncrementLoopIteration::operation((), context)?;
result.push(string.as_str());
}

// 6. Return the String value that is made from n copies of S appended together.
Ok(JsString::concat_array(&result).into())
Ok(string.repeat(n).into())
}

/// `String.prototype.slice( beginIndex [, endIndex] )`
Expand Down Expand Up @@ -1019,12 +1027,29 @@ impl String {
ReplaceValue(JsString),
}

// 1. Let O be ? RequireObjectCoercible(this value).
let o = this.require_object_coercible()?;
// Ultra-fast path: If this, searchValue, and replaceValue are all strings
// without '$', delegate directly to `replace_once` (no substitution possible).
//
// SAFETY of the ordering: this path requires all three `as_str()` to be
// `Some`, which implies `this` is a primitive string (so
// `RequireObjectCoercible` below would succeed without side effects) and
// both args are primitive strings (so neither is an object with
// `@@replace`, nor a callable functional-replace). Observable behavior
// is therefore identical to the slow path.
if let [arg0, arg1, ..] = args
&& let (Some(s), Some(search_str), Some(replace_str)) =
(this.as_str(), arg0.as_str(), arg1.as_str())
&& !replace_str.contains(b'$')
{
return Ok(JsString::replace_once(s, search_str, replace_str).into());
}

let search_value = args.get_or_undefined(0);
let replace_value = args.get_or_undefined(1);

// 1. Let O be ? RequireObjectCoercible(this value).
let o = this.require_object_coercible()?;

// 2. If searchValue is an Object, then
if search_value.is_object() {
// a. Let replacer be ? GetMethod(searchValue, @@replace).
Expand Down Expand Up @@ -1080,6 +1105,19 @@ impl String {
}
// 12. Else,
CallableOrString::ReplaceValue(replace_value) => {
// Fast path: If replace_value does not contain '$', no substitution is possible.
// Reuse the already-computed `position` via `replace_once_at`
// instead of searching a second time.
if !replace_value.contains(b'$') {
return Ok(JsString::replace_once_at(
string.as_str(),
search_length,
replace_value.as_str(),
position,
)
.into());
}

// a. Assert: Type(replaceValue) is String.
// b. Let captures be a new empty List.
let captures = Vec::new();
Expand Down Expand Up @@ -1231,16 +1269,22 @@ impl String {
// i. Assert: Type(replaceValue) is String.
// ii. Let captures be a new empty List.
// iii. Let replacement be ! GetSubstitution(searchString, string, p, captures, undefined, replaceValue).
Err(ref replace_str) => get_substitution(
&search_string,
&string,
p,
&[],
&JsValue::undefined(),
replace_str,
context,
)
.js_expect("GetSubstitution should never fail here.")?,
Err(ref replace_str) => {
if replace_str.contains(b'$') {
get_substitution(
&search_string,
&string,
p,
&[],
&JsValue::undefined(),
replace_str,
context,
)
.js_expect("GetSubstitution should never fail here.")?
} else {
replace_str.clone()
}
}
};

// d. Set result to the string-concatenation of result, preserved, and replacement.
Expand Down Expand Up @@ -1566,36 +1610,84 @@ impl String {
.into());
}

let filler_len = filler.len() as u64;
let total_len = int_max_length as usize;
let fill_len = fill_len as usize;
let filler_len = filler.len();
debug_assert!(filler_len > 0, "empty filler returns early above");

// 9. Let truncatedStringFiller be the String value consisting of repeated
// concatenations of filler truncated to length fillLen.
let repetitions = {
let q = fill_len / filler_len;
let r = fill_len % filler_len;
if r == 0 { q } else { q + 1 }
};
let filler_str = filler.as_str();
let s_str = string.as_str();

let latin1 = s_str.is_latin1() && filler_str.is_latin1();

let mut truncated_string_filler = Vec::with_capacity(fill_len as usize);
let filler_slice = filler.to_vec();
for _ in 0..repetitions {
let remaining = fill_len as usize - truncated_string_filler.len();
if remaining >= filler_slice.len() {
truncated_string_filler.extend_from_slice(&filler_slice);
let res = if latin1 {
let mut builder = boa_string::Latin1JsStringBuilder::with_capacity(total_len);
let s_bytes = s_str.as_latin1().expect("checked latin1");
let f_bytes = filler_str.as_latin1().expect("checked latin1");

let copy_filler = |builder: &mut boa_string::Latin1JsStringBuilder| {
let mut remaining = fill_len;
while remaining > 0 {
let chunk = remaining.min(filler_len);
debug_assert!(chunk > 0 && chunk <= filler_len);
builder.extend_from_slice(&f_bytes[..chunk]);
remaining -= chunk;
}
};

if placement == Placement::Start {
copy_filler(&mut builder);
builder.extend_from_slice(s_bytes);
} else {
truncated_string_filler.extend_from_slice(&filler_slice[..remaining]);
break;
builder.extend_from_slice(s_bytes);
copy_filler(&mut builder);
}
}
let truncated_string_filler = JsString::from(&truncated_string_filler[..]);

// 10. If placement is start, return the string-concatenation of truncatedStringFiller and S.
if placement == Placement::Start {
Ok(js_string!(&truncated_string_filler, &string).into())
// SAFETY: Both parts were checked Latin1 above, so one byte == one code unit.
unsafe { builder.build_as_latin1() }
} else {
// 11. Else, return the string-concatenation of S and truncatedStringFiller.
Ok(js_string!(&string, &truncated_string_filler).into())
}
use boa_string::{JsStr, JsStrVariant, Utf16JsStringBuilder};
let mut builder = Utf16JsStringBuilder::with_capacity(total_len);

let append_str =
|builder: &mut Utf16JsStringBuilder, js_s: &JsStr<'_>| match js_s.variant() {
JsStrVariant::Latin1(bytes) => {
builder.extend(bytes.iter().copied().map(u16::from));
}
JsStrVariant::Utf16(u16s) => {
builder.extend_from_slice(u16s);
}
};

// Hoist the filler variant out of the loop: it never changes.
let filler_variant = filler_str.variant();
let append_filler = |builder: &mut Utf16JsStringBuilder| {
let mut remaining = fill_len;
while remaining > 0 {
let chunk = remaining.min(filler_len);
debug_assert!(chunk > 0 && chunk <= filler_len);
match filler_variant {
JsStrVariant::Latin1(bytes) => {
builder.extend(bytes[..chunk].iter().copied().map(u16::from));
}
JsStrVariant::Utf16(u16s) => {
builder.extend_from_slice(&u16s[..chunk]);
}
}
remaining -= chunk;
}
};

if placement == Placement::Start {
append_filler(&mut builder);
append_str(&mut builder, &s_str);
} else {
append_str(&mut builder, &s_str);
append_filler(&mut builder);
}
builder.build()
};

Ok(res.into())
}

/// `String.prototype.padEnd( targetLength[, padString] )`
Expand Down Expand Up @@ -1669,7 +1761,12 @@ impl String {
// 2. Return ? TrimString(S, start+end).
let object = this.require_object_coercible()?;
let string = object.to_string(context)?;
Ok(js_string!(string.trim()).into())
let trimmed = string.trim();
// Fold common results back to static strings instead of retaining
// a slice of a potentially huge parent.
Ok(StaticJsStrings::get_string(&trimmed.as_str())
.unwrap_or(trimmed)
.into())
}

/// `String.prototype.trimStart()`
Expand All @@ -1693,7 +1790,10 @@ impl String {
// 2. Return ? TrimString(S, start).
let object = this.require_object_coercible()?;
let string = object.to_string(context)?;
Ok(js_string!(string.trim_start()).into())
let trimmed = string.trim_start();
Ok(StaticJsStrings::get_string(&trimmed.as_str())
.unwrap_or(trimmed)
.into())
}

/// `String.prototype.trimEnd()`
Expand All @@ -1717,7 +1817,10 @@ impl String {
// 2. Return ? TrimString(S, end).
let object = this.require_object_coercible()?;
let string = object.to_string(context)?;
Ok(string.trim_end().into())
let trimmed = string.trim_end();
Ok(StaticJsStrings::get_string(&trimmed.as_str())
.unwrap_or(trimmed)
.into())
}

/// [`String.prototype.toUpperCase()`][upper] and [`String.prototype.toLowerCase()`][lower]
Expand Down Expand Up @@ -2667,6 +2770,11 @@ pub(crate) fn get_substitution(
// 8. Let tailPos be position + matchLength.
let tail_pos = position + match_length;

// Fast path: if replacement does not contain '$', no substitutions can occur.
if !replacement.contains(b'$') {
return Ok(replacement.clone());
}

// 10. Let result be the String value derived from replacement by copying code unit elements
// from replacement to result while performing replacements as specified in Table 58.
// These $ replacements are done left-to-right, and, once such a replacement is performed,
Expand Down
14 changes: 13 additions & 1 deletion core/engine/src/value/inner/legacy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::builtins::is_html_dda::IsHTMLDDA;
use crate::{JsBigInt, JsObject, JsSymbol, value::Type};
use boa_engine::JsVariant;
use boa_gc::{Finalize, Trace, custom_trace};
use boa_string::JsString;
use boa_string::{JsStr, JsString};

#[derive(Clone, Debug)]
pub(crate) enum EnumBasedValue {
Expand Down Expand Up @@ -267,6 +267,18 @@ impl EnumBasedValue {
}
}

/// Returns the value as a [`JsStr`] without cloning.
///
/// The returned slice borrows `self`; keep the [`crate::JsValue`] alive while using it.
#[must_use]
#[inline]
pub(crate) fn as_str(&self) -> Option<JsStr<'_>> {
match self {
Self::String(value) => Some(value.as_str()),
_ => None,
}
}

/// Converts the value to a boolean without cloning pointer types.
#[must_use]
#[inline]
Expand Down
Loading
Loading