Optimize number to PropertyKey conversion (#3769)

In previous implementation when converting a number to PropertyKey, we
always converted it to string and only when the conversion to
PropertyKey::Index was not possible we returned that string.

core/engine/src/builtins/array/mod.rs

@@ -120,7 +120,7 @@ impl IntrinsicObject for Array {
                 Attribute::CONFIGURABLE,
             )
             .property(
-                utf16!("length"),
+                StaticJsStrings::LENGTH,
                 0,
                 Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
             )
@@ -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())
@@ -307,7 +307,7 @@ impl Array {
             }
         }
 
-        o.set(utf16!("length"), len, true, context)?;
+        o.set(StaticJsStrings::LENGTH, len, true, context)?;
         Ok(())
     }
 
@@ -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)
@@ -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());
@@ -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());
             }
@@ -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)
@@ -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());
@@ -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,
                     )?;
@@ -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]]).
@@ -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.
@@ -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,
         );
@@ -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,
     )
@@ -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,
             )
@@ -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,
         )

core/engine/src/builtins/builder.rs

@@ -9,6 +9,7 @@ use crate::{
     },
     property::{Attribute, PropertyDescriptor, PropertyKey},
     realm::Realm,
+    string::common::StaticJsStrings,
     JsObject, JsString, JsValue, NativeFunction,
 };
 
@@ -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)

core/engine/src/builtins/error/type.rs

@@ -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();
 

core/engine/src/builtins/function/mod.rs

@@ -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).
@@ -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)

core/engine/src/builtins/string/mod.rs

@@ -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)

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

@@ -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,

core/engine/src/object/mod.rs

@@ -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,
 };
 
@@ -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() {

core/engine/src/object/operations.rs

@@ -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,
 };
@@ -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 )`

core/engine/src/property/mod.rs

@@ -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)
     }
 }
 
@@ -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)
     }
 }
 
@@ -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)
     }
 }
 
@@ -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)
     }
 }
 
@@ -778,16 +779,17 @@ 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()))
     }
 }
 
@@ -795,8 +797,8 @@ 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,
         )
     }

core/engine/src/string/common.rs

@@ -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"),

core/engine/src/vm/opcode/push/array.rs

@@ -1,6 +1,6 @@
 use crate::{
     builtins::Array,
-    string::utf16,
+    string::common::StaticJsStrings,
     vm::{opcode::Operation, CompletionType},
     Context, JsResult, JsValue,
 };
@@ -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)
     }