Skip to content

Commit e79a5d6

Browse files
committed
fix(typedarray): optimize toLocaleString using single NumberFormat and ListFormatter
Avoid creating a NumberFormat per element by reusing a single NumberFormat and ListFormatter instance for the entire typed array. Falls back to the spec path when: - Number.prototype.toLocaleString is overridden - elements contain NaN or Infinity - the typed array uses BigInt Signed-off-by: mrhapile <allinonegaming3456@gmail.com>
1 parent b78e36f commit e79a5d6

File tree

1 file changed

+104
-69
lines changed

1 file changed

+104
-69
lines changed

core/engine/src/builtins/typed_array/builtin.rs

Lines changed: 104 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -2476,9 +2476,6 @@ impl BuiltinTypedArray {
24762476
args: &[JsValue],
24772477
context: &mut Context,
24782478
) -> JsResult<JsValue> {
2479-
// This is a distinct method that implements the same algorithm as Array.prototype.toLocaleString as defined in
2480-
// 23.1.3.32 except that TypedArrayLength is called in place of performing a [[Get]] of "length".
2481-
24822479
let array = this.as_object().ok_or_else(|| {
24832480
JsNativeError::typ().with_message("Value is not a typed array object")
24842481
})?;
@@ -2497,7 +2494,6 @@ impl BuiltinTypedArray {
24972494
.with_message("typed array is outside the bounds of its inner buffer")
24982495
.into());
24992496
};
2500-
25012497
(
25022498
o.array_length(buf_len),
25032499
o.kind().content_type() == ContentType::BigInt,
@@ -2510,91 +2506,130 @@ impl BuiltinTypedArray {
25102506

25112507
let locales = args.get_or_undefined(0).clone();
25122508
let options = args.get_or_undefined(1).clone();
2509+
let call_args = [locales.clone(), options.clone()];
25132510

25142511
#[cfg(feature = "intl")]
2515-
let number_format = if _is_bigint {
2516-
None
2517-
} else {
2518-
use crate::builtins::intl::number_format::NumberFormat;
2519-
Some(NumberFormat::new(&locales, &options, context)?)
2520-
};
2521-
2522-
// 4. Let R be the empty String.
2523-
let mut r = Vec::<JsString>::with_capacity(len as usize);
2524-
2525-
let call_args = [locales, options];
2526-
2527-
#[cfg(feature = "intl")]
2528-
let is_unmodified = {
2529-
let number_proto = context.intrinsics().constructors().number().prototype();
2530-
2531-
let current_tls = number_proto.get(js_string!("toLocaleString"), context)?;
2512+
{
2513+
use crate::builtins::intl::number_format::{NumberFormat, to_intl_mathematical_value};
2514+
use icu_list::{
2515+
ListFormatter, ListFormatterPreferences,
2516+
options::{ListFormatterOptions, ListLength},
2517+
};
25322518

2519+
// Retrieve the stored intrinsic built-in for Number.prototype.toLocaleString.
2520+
// From number/mod.rs: it is registered with `callable_with_object` using
2521+
// the pre-allocated object from `realm.intrinsics().objects()
2522+
// .number_prototype_to_locale_string()`.
2523+
// From intrinsics.rs: `number_prototype_to_locale_string()` returns JsFunction.
2524+
//
2525+
// We compare the current value on Number.prototype against this stored
2526+
// intrinsic by pointer identity (JsObject::equals). If they match, the
2527+
// built-in has not been replaced and we can safely use the fast path.
2528+
// If a test has replaced it (e.g. Test262 observability tests that do
2529+
// `Number.prototype.toLocaleString = function() { callCount++; }`),
2530+
// the comparison fails and we fall through to the slow path which
2531+
// calls invoke("toLocaleString"), hitting the replacement.
25332532
let builtin_tls = context
25342533
.intrinsics()
25352534
.objects()
25362535
.number_prototype_to_locale_string();
25372536

2538-
current_tls
2537+
let current_tls = context
2538+
.intrinsics()
2539+
.constructors()
2540+
.number()
2541+
.prototype()
2542+
.get(js_string!("toLocaleString"), context)?;
2543+
2544+
let tls_is_builtin = current_tls
25392545
.as_object()
2540-
.is_some_and(|o| JsObject::equals(&o, &builtin_tls.into()))
2541-
};
2546+
.is_some_and(|o| JsObject::equals(&o, &builtin_tls.clone().into()));
2547+
2548+
// Use the fast path only for numeric (non-BigInt) typed arrays where
2549+
// the built-in toLocaleString is still intact.
2550+
if !_is_bigint && tls_is_builtin {
2551+
// ONE NumberFormat for the entire array — the core optimization.
2552+
let number_format = NumberFormat::new(&locales, &options, context)?;
2553+
2554+
// ONE ListFormatter to join all formatted strings.
2555+
// ListLength::Narrow produces comma-separated output without
2556+
// locale-specific conjunctions like "and".
2557+
// From list_format/mod.rs: the correct API is
2558+
// ListFormatterOptions::default().with_length(style)
2559+
// where style is a ListLength variant.
2560+
let prefs = ListFormatterPreferences::default();
2561+
let list_formatter = ListFormatter::try_new_unit_with_buffer_provider(
2562+
context.intl_provider().erased_provider(),
2563+
prefs,
2564+
ListFormatterOptions::default().with_length(ListLength::Narrow),
2565+
)
2566+
.map_err(|e| JsNativeError::typ().with_message(e.to_string()))?;
25422567

2543-
// 5. Let k be 0.
2544-
// 6. Repeat, while k < len,
2545-
for k in 0..len {
2546-
// b. Let nextElement be ? Get(O, ! ToString(𝔽(k))).
2547-
let next_element = array.get(k, context)?;
2568+
// Format each element. Abort the entire fast path if any element
2569+
// is non-finite (NaN, Infinity) because to_intl_mathematical_value
2570+
// cannot represent those — fall through to the slow path instead.
2571+
let mut formatted: Vec<String> = Vec::with_capacity(len as usize);
2572+
let mut use_fast = true;
25482573

2549-
// c. If nextElement is not undefined or null, then
2550-
// i. Let S be ? ToString(? Invoke(nextElement, "toLocaleString", « locales, options »)).
2551-
if next_element.is_null_or_undefined() {
2552-
r.push(js_string!(""));
2553-
continue;
2554-
}
2574+
for k in 0..len {
2575+
let elem = array.get(k, context)?;
25552576

2556-
#[cfg(feature = "intl")]
2557-
{
2558-
use crate::{
2559-
builtins::intl::number_format::to_intl_mathematical_value, value::JsVariant,
2560-
};
2577+
// Spec: null/undefined elements become empty string.
2578+
if elem.is_null_or_undefined() {
2579+
formatted.push(String::new());
2580+
continue;
2581+
}
25612582

2562-
// Fast path: finite primitive numbers only
2563-
let use_fast_path = is_unmodified
2564-
&& match next_element.variant() {
2565-
JsVariant::Integer32(_) => !_is_bigint,
2566-
JsVariant::Float64(f) => !_is_bigint && f.is_finite(),
2567-
_ => false,
2568-
};
2583+
match elem.variant() {
2584+
// Finite integers — always safe.
2585+
JsVariant::Integer32(_) => {
2586+
let mut x = to_intl_mathematical_value(&elem, context)?;
2587+
formatted.push(number_format.format(&mut x).to_string());
2588+
}
2589+
// Finite floats — safe.
2590+
JsVariant::Float64(f) if f.is_finite() => {
2591+
let mut x = to_intl_mathematical_value(&elem, context)?;
2592+
formatted.push(number_format.format(&mut x).to_string());
2593+
}
2594+
// NaN, Infinity, -Infinity — abort fast path.
2595+
_ => {
2596+
use_fast = false;
2597+
break;
2598+
}
2599+
}
2600+
}
25692601

2570-
if use_fast_path {
2571-
let mut x = to_intl_mathematical_value(&next_element, context)?;
2572-
r.push(js_string!(
2573-
number_format
2574-
.as_ref()
2575-
.expect("number_format is Some for numeric typed arrays")
2576-
.format(&mut x)
2577-
.to_string()
2578-
));
2579-
} else {
2580-
// Slow path: delegate to element.toLocaleString()
2581-
let s = next_element
2582-
.invoke(js_string!("toLocaleString"), &call_args, context)?
2583-
.to_string(context)?;
2584-
r.push(s);
2602+
if use_fast {
2603+
// Join all formatted strings with the ListFormatter in one call.
2604+
let joined = list_formatter.format_to_string(formatted.into_iter());
2605+
return Ok(js_string!(joined.as_str()).into());
25852606
}
2607+
// If use_fast is false, fall through to the slow path below.
25862608
}
2609+
}
25872610

2588-
#[cfg(not(feature = "intl"))]
2589-
{
2590-
let s = next_element
2591-
.invoke(js_string!("toLocaleString"), &call_args, context)?
2592-
.to_string(context)?;
2593-
r.push(s);
2611+
// Slow path: always spec-correct.
2612+
// Used when:
2613+
// - intl feature is disabled
2614+
// - typed array is BigInt (BigInt64Array / BigUint64Array)
2615+
// - Number.prototype.toLocaleString has been replaced
2616+
// - array contains NaN or Infinity
2617+
let mut r = Vec::<JsString>::with_capacity(len as usize);
2618+
2619+
for k in 0..len {
2620+
let next_element = array.get(k, context)?;
2621+
2622+
if next_element.is_null_or_undefined() {
2623+
r.push(js_string!(""));
2624+
continue;
25942625
}
2626+
2627+
let s = next_element
2628+
.invoke(js_string!("toLocaleString"), &call_args, context)?
2629+
.to_string(context)?;
2630+
r.push(s);
25952631
}
25962632

2597-
// 7. Return R.
25982633
let separator = js_string!(", ");
25992634
let mut result = Vec::with_capacity(
26002635
r.iter().map(JsString::len).sum::<usize>()

0 commit comments

Comments
 (0)