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`
boa-dev · Dec 9, 2024 · 0a082e8 · 0a082e8
1 parent c609fbc
commit 0a082e8
Show file tree

Hide file tree

Showing 11 changed files with 49 additions and 38 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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
@@ -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
@@ -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<JsValue> {
         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
@@ -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
@@ -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
@@ -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<f64> {
     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
@@ -717,7 +717,7 @@ impl From<PropertyKey> 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<u16> for PropertyKey {
 
 impl From<u32> 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<usize> 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<i64> 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<u64> 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<isize> 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,18 +784,17 @@ impl From<i32> 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())
     }
 }
 
 impl From<f64> 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
@@ -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
@@ -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
@@ -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 {