Skip to content
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.

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

Add fast path for number to JsString conversion #4054

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ fast-float2 = "0.2.3"
hashbrown = "0.15.1"
indexmap = { version = "2.6.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"
Expand Down
11 changes: 3 additions & 8 deletions core/engine/src/builtins/json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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 {
Expand Down
13 changes: 3 additions & 10 deletions core/engine/src/builtins/number/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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. {
Expand Down
4 changes: 1 addition & 3 deletions core/engine/src/builtins/object/for_in_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(_) => {}
}
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/builtins/object/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
4 changes: 2 additions & 2 deletions core/engine/src/builtins/typed_array/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}

Expand Down
22 changes: 10 additions & 12 deletions core/engine/src/property/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,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(),
}
}
}
Expand All @@ -739,8 +739,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)
}
}

Expand All @@ -749,7 +748,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)
}
}

Expand All @@ -758,7 +757,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)
}
}

Expand All @@ -767,7 +766,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)
}
}

Expand All @@ -776,7 +775,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)
}
}

Expand All @@ -786,18 +785,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)
}
}

Expand Down
4 changes: 2 additions & 2 deletions core/engine/src/value/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 2 additions & 0 deletions core/string/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions core/string/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading