From 0a082e8a65e90a18cad20fb69c3737fdb535c66b Mon Sep 17 00:00:00 2001 From: CrazyboyQCD <53971641+CrazyboyQCD@users.noreply.github.com> Date: Mon, 9 Dec 2024 21:24:30 +0800 Subject: [PATCH] Add fast path for number to `JsString` conversion (#4054) * chore: add fast path to convert number to `JsString` * chore: replace `Number::to_js_string` with `JsString::from` * chore: remove some unnecessary `to_string`s on number to `JsString` conversion * chore: mark conversion `inline` --- Cargo.lock | 2 ++ Cargo.toml | 1 + core/engine/src/builtins/json/mod.rs | 11 +++------- core/engine/src/builtins/number/mod.rs | 13 +++-------- .../src/builtins/object/for_in_iterator.rs | 4 +--- core/engine/src/builtins/object/mod.rs | 2 +- .../engine/src/builtins/typed_array/object.rs | 4 ++-- core/engine/src/property/mod.rs | 22 +++++++++---------- core/engine/src/value/mod.rs | 4 ++-- core/string/Cargo.toml | 2 ++ core/string/src/lib.rs | 22 +++++++++++++++++++ 11 files changed, 49 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c3feb302480..159dac59869 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -562,8 +562,10 @@ name = "boa_string" version = "0.20.0" dependencies = [ "fast-float2", + "itoa", "paste", "rustc-hash 2.1.0", + "ryu-js", "sptr", "static_assertions", ] diff --git a/Cargo.toml b/Cargo.toml index 9f8af784096..af499d7e7c3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,6 +54,7 @@ fast-float2 = "0.2.3" hashbrown = "0.15.2" indexmap = { version = "2.7.0", default-features = false } indoc = "2.0.5" +itoa = "1.0.13" jemallocator = "0.5.4" num-bigint = "0.4.6" num-traits = "0.2.19" diff --git a/core/engine/src/builtins/json/mod.rs b/core/engine/src/builtins/json/mod.rs index 8dd04fcf85b..78e9710f554 100644 --- a/core/engine/src/builtins/json/mod.rs +++ b/core/engine/src/builtins/json/mod.rs @@ -193,12 +193,8 @@ impl Json { for i in 0..len { // 1. Let prop be ! ToString(𝔽(I)). // 2. Let newElement be ? InternalizeJSONProperty(val, prop, reviver). - let new_element = Self::internalize_json_property( - obj, - i.to_string().into(), - reviver, - context, - )?; + let new_element = + Self::internalize_json_property(obj, i.into(), reviver, context)?; // 3. If newElement is undefined, then if new_element.is_undefined() { @@ -750,8 +746,7 @@ impl Json { // 8. Repeat, while index < len, while index < len { // a. Let strP be ? SerializeJSONProperty(state, ! ToString(𝔽(index)), value). - let str_p = - Self::serialize_json_property(state, index.to_string().into(), value, context)?; + let str_p = Self::serialize_json_property(state, index.into(), value, context)?; // b. If strP is undefined, then if let Some(str_p) = str_p { diff --git a/core/engine/src/builtins/number/mod.rs b/core/engine/src/builtins/number/mod.rs index d1b09d4ecfe..99147f903f2 100644 --- a/core/engine/src/builtins/number/mod.rs +++ b/core/engine/src/builtins/number/mod.rs @@ -230,7 +230,7 @@ impl Number { }; // 4. If x is not finite, return ! Number::toString(x). if !this_num.is_finite() { - return Ok(JsValue::new(Self::to_js_string(this_num))); + return Ok(JsValue::new(JsString::from(this_num))); } // Get rid of the '-' sign for -0.0 let this_num = if this_num == 0. { 0. } else { this_num }; @@ -308,8 +308,7 @@ impl Number { _: &mut Context, ) -> JsResult { let this_num = Self::this_number_value(this)?; - let this_str_num = this_num.to_string(); - Ok(JsValue::new(js_string!(this_str_num))) + Ok(JsValue::new(js_string!(this_num))) } /// `flt_str_to_exp` - used in `to_precision` @@ -644,12 +643,6 @@ impl Number { )) } - #[allow(clippy::wrong_self_convention)] - pub(crate) fn to_js_string(x: f64) -> JsString { - let mut buffer = ryu_js::Buffer::new(); - js_string!(buffer.format(x)) - } - /// `Number.prototype.toString( [radix] )` /// /// The `toString()` method returns a string representing the specified Number object. @@ -688,7 +681,7 @@ impl Number { // 5. If radixNumber = 10, return ! ToString(x). if radix_number == 10 { - return Ok(JsValue::new(Self::to_js_string(x))); + return Ok(JsValue::new(JsString::from(x))); } if x == -0. { diff --git a/core/engine/src/builtins/object/for_in_iterator.rs b/core/engine/src/builtins/object/for_in_iterator.rs index cbaf5adf0b7..e811685a07a 100644 --- a/core/engine/src/builtins/object/for_in_iterator.rs +++ b/core/engine/src/builtins/object/for_in_iterator.rs @@ -113,9 +113,7 @@ impl ForInIterator { iterator.remaining_keys.push_back(k.clone()); } PropertyKey::Index(i) => { - iterator - .remaining_keys - .push_back(i.get().to_string().into()); + iterator.remaining_keys.push_back(i.get().into()); } PropertyKey::Symbol(_) => {} } diff --git a/core/engine/src/builtins/object/mod.rs b/core/engine/src/builtins/object/mod.rs index 30a022875b7..7d1b18c4d16 100644 --- a/core/engine/src/builtins/object/mod.rs +++ b/core/engine/src/builtins/object/mod.rs @@ -1533,7 +1533,7 @@ fn get_own_property_keys( (PropertyKeyType::String, PropertyKey::String(_)) | (PropertyKeyType::Symbol, PropertyKey::Symbol(_)) => Some(next_key.into()), (PropertyKeyType::String, PropertyKey::Index(index)) => { - Some(js_string!(index.get().to_string()).into()) + Some(js_string!(index.get()).into()) } _ => None, } diff --git a/core/engine/src/builtins/typed_array/object.rs b/core/engine/src/builtins/typed_array/object.rs index 0389956c5e4..1e6bf2477de 100644 --- a/core/engine/src/builtins/typed_array/object.rs +++ b/core/engine/src/builtins/typed_array/object.rs @@ -3,7 +3,7 @@ use std::sync::atomic::Ordering; use crate::{ - builtins::{array_buffer::BufferObject, Number}, + builtins::array_buffer::BufferObject, object::{ internal_methods::{ ordinary_define_own_property, ordinary_delete, ordinary_get, ordinary_get_own_property, @@ -272,7 +272,7 @@ fn canonical_numeric_index_string(argument: &JsString) -> Option { let n = argument.to_number(); // 3. If ! ToString(n) is argument, return n. - if &Number::to_js_string(n) == argument { + if &JsString::from(n) == argument { return Some(n); } diff --git a/core/engine/src/property/mod.rs b/core/engine/src/property/mod.rs index 2ef676abd39..f8c293da8b4 100644 --- a/core/engine/src/property/mod.rs +++ b/core/engine/src/property/mod.rs @@ -717,7 +717,7 @@ impl From for JsValue { match property_key { PropertyKey::String(ref string) => string.clone().into(), PropertyKey::Symbol(ref symbol) => symbol.clone().into(), - PropertyKey::Index(index) => js_string!(index.get().to_string()).into(), + PropertyKey::Index(index) => js_string!(index.get()).into(), } } } @@ -738,8 +738,7 @@ impl From for PropertyKey { impl From for PropertyKey { fn from(value: u32) -> Self { - NonMaxU32::new(value) - .map_or_else(|| Self::String(js_string!(value.to_string())), Self::Index) + NonMaxU32::new(value).map_or_else(|| Self::String(value.into()), Self::Index) } } @@ -748,7 +747,7 @@ impl From for PropertyKey { u32::try_from(value) .ok() .and_then(NonMaxU32::new) - .map_or_else(|| Self::String(js_string!(value.to_string())), Self::Index) + .map_or_else(|| Self::String(value.into()), Self::Index) } } @@ -757,7 +756,7 @@ impl From for PropertyKey { u32::try_from(value) .ok() .and_then(NonMaxU32::new) - .map_or_else(|| Self::String(js_string!(value.to_string())), Self::Index) + .map_or_else(|| Self::String(value.into()), Self::Index) } } @@ -766,7 +765,7 @@ impl From for PropertyKey { u32::try_from(value) .ok() .and_then(NonMaxU32::new) - .map_or_else(|| Self::String(js_string!(value.to_string())), Self::Index) + .map_or_else(|| Self::String(value.into()), Self::Index) } } @@ -775,7 +774,7 @@ impl From for PropertyKey { u32::try_from(value) .ok() .and_then(NonMaxU32::new) - .map_or_else(|| Self::String(js_string!(value.to_string())), Self::Index) + .map_or_else(|| Self::String(value.into()), Self::Index) } } @@ -785,7 +784,7 @@ impl From for PropertyKey { // Safety: A positive i32 value fits in 31 bits, so it can never be u32::MAX. return Self::Index(unsafe { NonMaxU32::new_unchecked(value as u32) }); } - Self::String(js_string!(value.to_string())) + Self::String(value.into()) } } @@ -793,10 +792,9 @@ impl From for PropertyKey { fn from(value: f64) -> Self { use num_traits::cast::FromPrimitive; - u32::from_f64(value).and_then(NonMaxU32::new).map_or_else( - || Self::String(ryu_js::Buffer::new().format(value).into()), - Self::Index, - ) + u32::from_f64(value) + .and_then(NonMaxU32::new) + .map_or_else(|| Self::String(value.into()), Self::Index) } } diff --git a/core/engine/src/value/mod.rs b/core/engine/src/value/mod.rs index 6d19592028e..d44b12ee9d0 100644 --- a/core/engine/src/value/mod.rs +++ b/core/engine/src/value/mod.rs @@ -540,8 +540,8 @@ impl JsValue { } else { js_string!("false") }), - Self::Rational(rational) => Ok(Number::to_js_string(*rational)), - Self::Integer(integer) => Ok(integer.to_string().into()), + Self::Rational(rational) => Ok(JsString::from(*rational)), + Self::Integer(integer) => Ok(JsString::from(*integer)), Self::String(string) => Ok(string.clone()), Self::Symbol(_) => Err(JsNativeError::typ() .with_message("can't convert symbol to string") diff --git a/core/string/Cargo.toml b/core/string/Cargo.toml index 60268b4cc00..4203ab51495 100644 --- a/core/string/Cargo.toml +++ b/core/string/Cargo.toml @@ -12,7 +12,9 @@ repository.workspace = true rust-version.workspace = true [dependencies] +itoa.workspace = true rustc-hash = { workspace = true, features = ["std"] } +ryu-js.workspace = true sptr.workspace = true static_assertions.workspace = true paste.workspace = true diff --git a/core/string/src/lib.rs b/core/string/src/lib.rs index f39288289ce..b64004a008f 100644 --- a/core/string/src/lib.rs +++ b/core/string/src/lib.rs @@ -1092,6 +1092,28 @@ impl std::fmt::Debug for JsString { impl Eq for JsString {} +macro_rules! impl_from_number_for_js_string { + ($($module: ident => $($ty:ty),+)+) => { + $( + $( + impl From<$ty> for JsString { + #[inline] + fn from(value: $ty) -> Self { + JsString::from_slice_skip_interning(JsStr::latin1( + $module::Buffer::new().format(value).as_bytes(), + )) + } + } + )+ + )+ + }; +} + +impl_from_number_for_js_string!( + itoa => i8, i16, i32, i64, i128, u8, u16, u32, u64, u128, isize, usize + ryu_js => f32, f64 +); + impl From<&[u16]> for JsString { #[inline] fn from(s: &[u16]) -> Self {