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

Optimize number to PropertyKey conversion #3769

Merged
merged 1 commit into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
33 changes: 17 additions & 16 deletions core/engine/src/builtins/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl IntrinsicObject for Array {
Attribute::CONFIGURABLE,
)
.property(
utf16!("length"),
StaticJsStrings::LENGTH,
0,
Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
)
Expand Down Expand Up @@ -258,7 +258,7 @@ impl BuiltInConstructor for Array {
};
// e. Perform ! Set(array, "length", intLen, true).
array
.set(utf16!("length"), int_len, true, context)
.set(StaticJsStrings::LENGTH, int_len, true, context)
.expect("this Set call must not fail");
// f. Return array.
Ok(array.into())
Expand Down Expand Up @@ -307,7 +307,7 @@ impl Array {
}
}

o.set(utf16!("length"), len, true, context)?;
o.set(StaticJsStrings::LENGTH, len, true, context)?;
Ok(())
}

Expand Down Expand Up @@ -366,7 +366,7 @@ impl Array {
// 6. Perform ! OrdinaryDefineOwnProperty(A, "length", PropertyDescriptor { [[Value]]: 𝔽(length), [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: false }).
ordinary_define_own_property(
&array,
&utf16!("length").into(),
&StaticJsStrings::LENGTH.into(),
PropertyDescriptor::builder()
.value(length)
.writable(true)
Expand Down Expand Up @@ -593,7 +593,7 @@ impl Array {
}

// 13. Perform ? Set(A, "length", 𝔽(len), true).
a.set(utf16!("length"), len, true, context)?;
a.set(StaticJsStrings::LENGTH, len, true, context)?;

// 14. Return A.
return Ok(a.into());
Expand Down Expand Up @@ -623,7 +623,7 @@ impl Array {
// iii. Let next be ? IteratorStep(iteratorRecord).
if iterator_record.step(context)? {
// 1. Perform ? Set(A, "length", 𝔽(k), true).
a.set(utf16!("length"), k, true, context)?;
a.set(StaticJsStrings::LENGTH, k, true, context)?;
// 2. Return A.
return Ok(a.into());
}
Expand Down Expand Up @@ -3461,7 +3461,7 @@ fn array_exotic_define_own_property(
// 1. Assert: IsPropertyKey(P) is true.
match key {
// 2. If P is "length", then
PropertyKey::String(ref s) if s == utf16!("length") => {
PropertyKey::String(ref s) if s == &StaticJsStrings::LENGTH => {
// a. Return ? ArraySetLength(A, Desc).

array_set_length(obj, desc, context)
Expand All @@ -3471,8 +3471,9 @@ fn array_exotic_define_own_property(
let index = index.get();

// a. Let oldLenDesc be OrdinaryGetOwnProperty(A, "length").
let old_len_desc = ordinary_get_own_property(obj, &utf16!("length").into(), context)?
.expect("the property descriptor must exist");
let old_len_desc =
ordinary_get_own_property(obj, &StaticJsStrings::LENGTH.into(), context)?
.expect("the property descriptor must exist");

// b. Assert: ! IsDataDescriptor(oldLenDesc) is true.
debug_assert!(old_len_desc.is_data_descriptor());
Expand Down Expand Up @@ -3507,7 +3508,7 @@ fn array_exotic_define_own_property(
// ii. Set succeeded to OrdinaryDefineOwnProperty(A, "length", oldLenDesc).
let succeeded = ordinary_define_own_property(
obj,
&utf16!("length").into(),
&StaticJsStrings::LENGTH.into(),
old_len_desc.into(),
context,
)?;
Expand Down Expand Up @@ -3542,7 +3543,7 @@ fn array_set_length(
// 1. If Desc.[[Value]] is absent, then
let Some(new_len_val) = desc.value() else {
// a. Return OrdinaryDefineOwnProperty(A, "length", Desc).
return ordinary_define_own_property(obj, &utf16!("length").into(), desc, context);
return ordinary_define_own_property(obj, &StaticJsStrings::LENGTH.into(), desc, context);
};

// 3. Let newLen be ? ToUint32(Desc.[[Value]]).
Expand All @@ -3568,7 +3569,7 @@ fn array_set_length(
.maybe_configurable(desc.configurable());

// 7. Let oldLenDesc be OrdinaryGetOwnProperty(A, "length").
let old_len_desc = ordinary_get_own_property(obj, &utf16!("length").into(), context)?
let old_len_desc = ordinary_get_own_property(obj, &StaticJsStrings::LENGTH.into(), context)?
.expect("the property descriptor must exist");

// 8. Assert: ! IsDataDescriptor(oldLenDesc) is true.
Expand All @@ -3585,7 +3586,7 @@ fn array_set_length(
// a. Return OrdinaryDefineOwnProperty(A, "length", newLenDesc).
return ordinary_define_own_property(
obj,
&utf16!("length").into(),
&StaticJsStrings::LENGTH.into(),
new_len_desc.build(),
context,
);
Expand Down Expand Up @@ -3615,7 +3616,7 @@ fn array_set_length(
// 16. If succeeded is false, return false.
if !ordinary_define_own_property(
obj,
&utf16!("length").into(),
&StaticJsStrings::LENGTH.into(),
new_len_desc.clone().build(),
context,
)
Expand Down Expand Up @@ -3652,7 +3653,7 @@ fn array_set_length(
// iii. Perform ! OrdinaryDefineOwnProperty(A, "length", newLenDesc).
ordinary_define_own_property(
obj,
&utf16!("length").into(),
&StaticJsStrings::LENGTH.into(),
new_len_desc.build(),
context,
)
Expand All @@ -3669,7 +3670,7 @@ fn array_set_length(
// PropertyDescriptor { [[Writable]]: false }).
let succeeded = ordinary_define_own_property(
obj,
&utf16!("length").into(),
&StaticJsStrings::LENGTH.into(),
PropertyDescriptor::builder().writable(false).build(),
context,
)
Expand Down
3 changes: 2 additions & 1 deletion core/engine/src/builtins/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::{
},
property::{Attribute, PropertyDescriptor, PropertyKey},
realm::Realm,
string::common::StaticJsStrings,
JsObject, JsString, JsValue, NativeFunction,
};

Expand Down Expand Up @@ -106,7 +107,7 @@ impl<S: ApplyToObject + IsConstructor> ApplyToObject for Callable<S> {
function.realm = Some(self.realm);
}
object.insert(
utf16!("length"),
StaticJsStrings::LENGTH,
PropertyDescriptor::builder()
.value(self.length)
.writable(false)
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/builtins/error/type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl IntrinsicObject for ThrowTypeError {
fn init(realm: &Realm) {
let obj = BuiltInBuilder::with_intrinsic::<Self>(realm)
.prototype(realm.intrinsics().constructors().function().prototype())
.static_property(utf16!("length"), 0, Attribute::empty())
.static_property(StaticJsStrings::LENGTH, 0, Attribute::empty())
.static_property(utf16!("name"), js_string!(), Attribute::empty())
.build();

Expand Down
6 changes: 3 additions & 3 deletions core/engine/src/builtins/function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,9 +702,9 @@ impl BuiltInFunctionObject {

// 5. Let targetHasLength be ? HasOwnProperty(Target, "length").
// 6. If targetHasLength is true, then
if target.has_own_property(utf16!("length"), context)? {
if target.has_own_property(StaticJsStrings::LENGTH, context)? {
// a. Let targetLen be ? Get(Target, "length").
let target_len = target.get(utf16!("length"), context)?;
let target_len = target.get(StaticJsStrings::LENGTH, context)?;
// b. If Type(targetLen) is Number, then
if target_len.is_number() {
// 1. Let targetLenAsInt be ! ToIntegerOrInfinity(targetLen).
Expand All @@ -729,7 +729,7 @@ impl BuiltInFunctionObject {

// 7. Perform ! SetFunctionLength(F, L).
f.define_property_or_throw(
utf16!("length"),
StaticJsStrings::LENGTH,
PropertyDescriptor::builder()
.value(l)
.writable(false)
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/builtins/string/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ impl String {
// 8. Perform ! DefinePropertyOrThrow(S, "length", PropertyDescriptor { [[Value]]: 𝔽(length),
// [[Writable]]: false, [[Enumerable]]: false, [[Configurable]]: false }).
s.define_property_or_throw(
utf16!("length"),
StaticJsStrings::LENGTH,
PropertyDescriptor::builder()
.value(len)
.writable(false)
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/builtins/typed_array/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ impl IntrinsicObject for BuiltinTypedArray {
Attribute::CONFIGURABLE | Attribute::NON_ENUMERABLE,
)
.accessor(
utf16!("length"),
StaticJsStrings::LENGTH,
Some(get_length),
None,
Attribute::CONFIGURABLE | Attribute::NON_ENUMERABLE,
Expand Down
4 changes: 2 additions & 2 deletions core/engine/src/object/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::{
native_function::{NativeFunction, NativeFunctionObject},
property::{Attribute, PropertyDescriptor, PropertyKey},
realm::Realm,
string::utf16,
string::{common::StaticJsStrings, utf16},
Context, JsString, JsSymbol, JsValue,
};

Expand Down Expand Up @@ -1016,7 +1016,7 @@ impl<'ctx> ConstructorBuilder<'ctx> {
},
};

constructor.insert(utf16!("length"), length);
constructor.insert(StaticJsStrings::LENGTH, length);
constructor.insert(utf16!("name"), name);

if let Some(proto) = self.custom_prototype.take() {
Expand Down
5 changes: 3 additions & 2 deletions core/engine/src/object/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
object::{JsObject, PrivateElement, PrivateName, CONSTRUCTOR, PROTOTYPE},
property::{PropertyDescriptor, PropertyDescriptorBuilder, PropertyKey, PropertyNameKind},
realm::Realm,
string::utf16,
string::common::StaticJsStrings,
value::Type,
Context, JsResult, JsSymbol, JsValue,
};
Expand Down Expand Up @@ -578,7 +578,8 @@ impl JsObject {
}

// 2. Return ℝ(? ToLength(? Get(obj, "length"))).
self.get(utf16!("length"), context)?.to_length(context)
self.get(StaticJsStrings::LENGTH, context)?
.to_length(context)
}

/// `7.3.22 SpeciesConstructor ( O, defaultConstructor )`
Expand Down
24 changes: 13 additions & 11 deletions core/engine/src/property/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,8 @@ impl From<u16> for PropertyKey {

impl From<u32> for PropertyKey {
fn from(value: u32) -> Self {
NonMaxU32::new(value).map_or(Self::String(js_string!(value.to_string())), Self::Index)
NonMaxU32::new(value)
.map_or_else(|| Self::String(js_string!(value.to_string())), Self::Index)
}
}

Expand All @@ -751,7 +752,7 @@ impl From<usize> for PropertyKey {
u32::try_from(value)
.ok()
.and_then(NonMaxU32::new)
.map_or(Self::String(js_string!(value.to_string())), Self::Index)
.map_or_else(|| Self::String(js_string!(value.to_string())), Self::Index)
}
}

Expand All @@ -760,7 +761,7 @@ impl From<i64> for PropertyKey {
u32::try_from(value)
.ok()
.and_then(NonMaxU32::new)
.map_or(Self::String(js_string!(value.to_string())), Self::Index)
.map_or_else(|| Self::String(js_string!(value.to_string())), Self::Index)
}
}

Expand All @@ -769,7 +770,7 @@ impl From<u64> for PropertyKey {
u32::try_from(value)
.ok()
.and_then(NonMaxU32::new)
.map_or(Self::String(js_string!(value.to_string())), Self::Index)
.map_or_else(|| Self::String(js_string!(value.to_string())), Self::Index)
}
}

Expand All @@ -778,25 +779,26 @@ impl From<isize> for PropertyKey {
u32::try_from(value)
.ok()
.and_then(NonMaxU32::new)
.map_or(Self::String(js_string!(value.to_string())), Self::Index)
.map_or_else(|| Self::String(js_string!(value.to_string())), Self::Index)
}
}

impl From<i32> for PropertyKey {
fn from(value: i32) -> Self {
u32::try_from(value)
.ok()
.and_then(NonMaxU32::new)
.map_or(Self::String(js_string!(value.to_string())), Self::Index)
if !value.is_negative() {
// 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()))
}
}

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(
Self::String(ryu_js::Buffer::new().format(value).into()),
u32::from_f64(value).and_then(NonMaxU32::new).map_or_else(
|| Self::String(ryu_js::Buffer::new().format(value).into()),
Self::Index,
)
}
Expand Down
1 change: 1 addition & 0 deletions core/engine/src/string/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ impl StaticJsStrings {
// Some consts are only used on certain features, which triggers the unused lint.
well_known_statics! {
(EMPTY_STRING, ""),
(LENGTH, "length"),
// Symbols
(SYMBOL_ASYNC_ITERATOR, "Symbol.asyncIterator"),
(SYMBOL_HAS_INSTANCE, "Symbol.hasInstance"),
Expand Down
4 changes: 2 additions & 2 deletions core/engine/src/vm/opcode/push/array.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{
builtins::Array,
string::utf16,
string::common::StaticJsStrings,
vm::{opcode::Operation, CompletionType},
Context, JsResult, JsValue,
};
Expand Down Expand Up @@ -74,7 +74,7 @@ impl Operation for PushElisionToArray {
.length_of_array_like(context)
.expect("arrays should always have a 'length' property");

o.set(utf16!("length"), len + 1, true, context)?;
o.set(StaticJsStrings::LENGTH, len + 1, true, context)?;
context.vm.push(array);
Ok(CompletionType::Normal)
}
Expand Down
Loading