LibJS: Get rid of unnecessary work from canonical_numeric_index_string

The spec version of canonical_numeric_index_string is absurdly complex,
and ends up converting from a string to a number, and then back again
which is both slow and also requires a few allocations and a string
compare.

Instead lets use the logic we already have as that is much more
efficient.

This improves performance of all non-numeric property names.

5 files changed, 39 insertions, 67 deletions

diff --git a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp
index 037027776c..ee0c708c23 100644
--- a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp
+++ b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp
@@ -1027,7 +1027,7 @@ Object* create_mapped_arguments_object(GlobalObject& global_object, FunctionObje
 }
 
 // 7.1.21 CanonicalNumericIndexString ( argument ), https://tc39.es/ecma262/#sec-canonicalnumericindexstring
-Value canonical_numeric_index_string(GlobalObject& global_object, PropertyKey const& property_key)
+Optional<u32> canonical_numeric_index_string(PropertyKey const& property_key)
 {
     // NOTE: If the property name is a number type (An implementation-defined optimized
     // property key type), it can be treated as a string property that has already been
@@ -1035,25 +1035,11 @@ Value canonical_numeric_index_string(GlobalObject& global_object, PropertyKey co
 
     VERIFY(property_key.is_string() || property_key.is_number());
 
+    // If property_key is a string containing a canonical numeric index
+    // the act of calling is_number() will return true
     if (property_key.is_number())
-        return Value(property_key.as_number());
-
-    // 1. Assert: Type(argument) is String.
-    auto argument = Value(js_string(global_object.vm(), property_key.as_string()));
-
-    // 2. If argument is "-0", return -0𝔽.
-    if (argument.as_string().string() == "-0")
-        return Value(-0.0);
-
-    // 3. Let n be ! ToNumber(argument).
-    auto n = MUST(argument.to_number(global_object));
-
-    // 4. If SameValue(! ToString(n), argument) is false, return undefined.
-    if (!same_value(MUST(n.to_primitive_string(global_object)), argument))
-        return js_undefined();
-
-    // 5. Return n.
-    return n;
+        return property_key.as_number();
+    return {};
 }
 
 // 22.1.3.17.1 GetSubstitution ( matched, str, position, captures, namedCaptures, replacement ), https://tc39.es/ecma262/#sec-getsubstitution
diff --git a/Userland/Libraries/LibJS/Runtime/AbstractOperations.h b/Userland/Libraries/LibJS/Runtime/AbstractOperations.h
index 9e0035baac..b3650bbd2d 100644
--- a/Userland/Libraries/LibJS/Runtime/AbstractOperations.h
+++ b/Userland/Libraries/LibJS/Runtime/AbstractOperations.h
@@ -39,8 +39,9 @@ bool validate_and_apply_property_descriptor(Object*, PropertyKey const&, bool ex
 ThrowCompletionOr<Object*> get_prototype_from_constructor(GlobalObject&, FunctionObject const& constructor, Object* (GlobalObject::*intrinsic_default_prototype)());
 Object* create_unmapped_arguments_object(GlobalObject&, Span<Value> arguments);
 Object* create_mapped_arguments_object(GlobalObject&, FunctionObject&, Vector<FunctionNode::Parameter> const&, Span<Value> arguments, Environment&);
-Value canonical_numeric_index_string(GlobalObject&, PropertyKey const&);
+Optional<u32> canonical_numeric_index_string(PropertyKey const&);
 ThrowCompletionOr<String> get_substitution(GlobalObject&, Utf16View const& matched, Utf16View const& str, size_t position, Span<Value> captures, Value named_captures, Value replacement);
+double string_to_number(const PrimitiveString&);
 
 enum class CallerMode {
     Strict,
diff --git a/Userland/Libraries/LibJS/Runtime/StringObject.cpp b/Userland/Libraries/LibJS/Runtime/StringObject.cpp
index f92a65eb43..5c6f452314 100644
--- a/Userland/Libraries/LibJS/Runtime/StringObject.cpp
+++ b/Userland/Libraries/LibJS/Runtime/StringObject.cpp
@@ -44,7 +44,7 @@ void StringObject::visit_edges(Cell::Visitor& visitor)
 }
 
 // 10.4.3.5 StringGetOwnProperty ( S, P ), https://tc39.es/ecma262/#sec-stringgetownproperty
-static Optional<PropertyDescriptor> string_get_own_property(GlobalObject& global_object, StringObject const& string, PropertyKey const& property_key)
+static Optional<PropertyDescriptor> string_get_own_property(StringObject const& string, PropertyKey const& property_key)
 {
     // 1. Assert: S is an Object that has a [[StringData]] internal slot.
     // 2. Assert: IsPropertyKey(P) is true.
@@ -57,17 +57,14 @@ static Optional<PropertyDescriptor> string_get_own_property(GlobalObject& global
         return {};
 
     // 4. Let index be ! CanonicalNumericIndexString(P).
-    auto index = canonical_numeric_index_string(global_object, property_key);
-    // 5. If index is undefined, return undefined.
-    if (index.is_undefined())
-        return {};
-    // 6. If IsIntegralNumber(index) is false, return undefined.
-    if (!index.is_integral_number())
-        return {};
-    // 7. If index is -0𝔽, return undefined.
-    if (index.is_negative_zero())
-        return {};
+    auto index = canonical_numeric_index_string(property_key);
 
+    if (!index.has_value()) {
+        // 5. If index is undefined, return undefined.
+        // 6. If IsIntegralNumber(index) is false, return undefined.
+        // 7. If index is -0𝔽, return undefined.
+        return {};
+    }
     // 8. Let str be S.[[StringData]].
     // 9. Assert: Type(str) is String.
     auto str = string.primitive_string().utf16_string_view();
@@ -76,11 +73,11 @@ static Optional<PropertyDescriptor> string_get_own_property(GlobalObject& global
     auto length = str.length_in_code_units();
 
     // 11. If ℝ(index) < 0 or len ≤ ℝ(index), return undefined.
-    if (index.as_double() < 0 || length <= index.as_double())
+    if (length <= static_cast<u64>(*index))
         return {};
 
     // 12. Let resultStr be the String value of length 1, containing one code unit from str, specifically the code unit at index ℝ(index).
-    auto result_str = js_string(string.vm(), str.substring_view(index.as_double(), 1));
+    auto result_str = js_string(string.vm(), str.substring_view(*index, 1));
 
     // 13. Return the PropertyDescriptor { [[Value]]: resultStr, [[Writable]]: false, [[Enumerable]]: true, [[Configurable]]: false }.
     return PropertyDescriptor {
@@ -104,7 +101,7 @@ ThrowCompletionOr<Optional<PropertyDescriptor>> StringObject::internal_get_own_p
         return descriptor;
 
     // 4. Return ! StringGetOwnProperty(S, P).
-    return string_get_own_property(global_object(), *this, property_key);
+    return string_get_own_property(*this, property_key);
 }
 
 // 10.4.3.2 [[DefineOwnProperty]] ( P, Desc ), https://tc39.es/ecma262/#sec-string-exotic-objects-defineownproperty-p-desc
@@ -114,7 +111,7 @@ ThrowCompletionOr<bool> StringObject::internal_define_own_property(PropertyKey c
     VERIFY(property_key.is_valid());
 
     // 2. Let stringDesc be ! StringGetOwnProperty(S, P).
-    auto string_descriptor = string_get_own_property(global_object(), *this, property_key);
+    auto string_descriptor = string_get_own_property(*this, property_key);
 
     // 3. If stringDesc is not undefined, then
     if (string_descriptor.has_value()) {
diff --git a/Userland/Libraries/LibJS/Runtime/TypedArray.h b/Userland/Libraries/LibJS/Runtime/TypedArray.h
index 640e516457..6ee3efeb37 100644
--- a/Userland/Libraries/LibJS/Runtime/TypedArray.h
+++ b/Userland/Libraries/LibJS/Runtime/TypedArray.h
@@ -74,7 +74,7 @@ private:
 };
 
 // 10.4.5.9 IsValidIntegerIndex ( O, index ), https://tc39.es/ecma262/#sec-isvalidintegerindex
-inline bool is_valid_integer_index(TypedArrayBase const& typed_array, Value property_index)
+inline bool is_valid_integer_index(TypedArrayBase const& typed_array, u32 property_index)
 {
     if (typed_array.viewed_array_buffer()->is_detached())
         return false;
@@ -82,15 +82,8 @@ inline bool is_valid_integer_index(TypedArrayBase const& typed_array, Value prop
     // TODO: This can be optimized by skipping the following 3 out of 4 checks if property_index
     //  came from a number-type PropertyKey instead of a canonicalized string-type PropertyKey
 
-    // If ! IsIntegralNumber(index) is false, return false.
-    if (!property_index.is_integral_number())
-        return false;
-    // If index is -0𝔽, return false.
-    if (property_index.is_negative_zero())
-        return false;
-
     // If ℝ(index) < 0 or ℝ(index) ≥ O.[[ArrayLength]], return false.
-    if (property_index.as_double() < 0 || property_index.as_double() >= typed_array.array_length())
+    if (property_index >= typed_array.array_length())
         return false;
 
     return true;
@@ -98,7 +91,7 @@ inline bool is_valid_integer_index(TypedArrayBase const& typed_array, Value prop
 
 // 10.4.5.10 IntegerIndexedElementGet ( O, index ), https://tc39.es/ecma262/#sec-integerindexedelementget
 template<typename T>
-inline Value integer_indexed_element_get(TypedArrayBase const& typed_array, Value property_index)
+inline Value integer_indexed_element_get(TypedArrayBase const& typed_array, i64 property_index)
 {
     // 1. Assert: O is an Integer-Indexed exotic object.
 
@@ -112,7 +105,7 @@ inline Value integer_indexed_element_get(TypedArrayBase const& typed_array, Valu
     // 4. Let arrayTypeName be the String value of O.[[TypedArrayName]].
     // 5. Let elementSize be the Element Size value specified in Table 64 for arrayTypeName.
     // 6. Let indexedPosition be (ℝ(index) × elementSize) + offset.
-    Checked<size_t> indexed_position = (i64)property_index.as_double();
+    Checked<size_t> indexed_position = property_index;
     indexed_position *= typed_array.element_size();
     indexed_position += offset;
     // FIXME: Not exactly sure what we should do when overflow occurs.
@@ -130,7 +123,7 @@ inline Value integer_indexed_element_get(TypedArrayBase const& typed_array, Valu
 // 10.4.5.11 IntegerIndexedElementSet ( O, index, value ), https://tc39.es/ecma262/#sec-integerindexedelementset
 // NOTE: In error cases, the function will return as if it succeeded.
 template<typename T>
-inline ThrowCompletionOr<void> integer_indexed_element_set(TypedArrayBase& typed_array, Value property_index, Value value)
+inline ThrowCompletionOr<void> integer_indexed_element_set(TypedArrayBase& typed_array, i64 property_index, Value value)
 {
     VERIFY(!value.is_empty());
     auto& global_object = typed_array.global_object();
@@ -157,7 +150,7 @@ inline ThrowCompletionOr<void> integer_indexed_element_set(TypedArrayBase& typed
     // b. Let arrayTypeName be the String value of O.[[TypedArrayName]].
     // c. Let elementSize be the Element Size value specified in Table 64 for arrayTypeName.
     // d. Let indexedPosition be (ℝ(index) × elementSize) + offset.
-    Checked<size_t> indexed_position = (i64)property_index.as_double();
+    Checked<size_t> indexed_position = property_index;
     indexed_position *= typed_array.element_size();
     indexed_position += offset;
     // FIXME: Not exactly sure what we should do when overflow occurs.
@@ -198,11 +191,10 @@ public:
         // NOTE: This includes an implementation-defined optimization, see note above!
         if (property_key.is_string() || property_key.is_number()) {
             // a. Let numericIndex be ! CanonicalNumericIndexString(P).
-            auto numeric_index = canonical_numeric_index_string(global_object(), property_key);
             // b. If numericIndex is not undefined, then
-            if (!numeric_index.is_undefined()) {
+            if (auto numeric_index = canonical_numeric_index_string(property_key); numeric_index.has_value()) {
                 // i. Let value be ! IntegerIndexedElementGet(O, numericIndex).
-                auto value = integer_indexed_element_get<T>(*this, numeric_index);
+                auto value = integer_indexed_element_get<T>(*this, *numeric_index);
 
                 // ii. If value is undefined, return undefined.
                 if (value.is_undefined())
@@ -238,10 +230,9 @@ public:
         // NOTE: This includes an implementation-defined optimization, see note above!
         if (property_key.is_string() || property_key.is_number()) {
             // a. Let numericIndex be ! CanonicalNumericIndexString(P).
-            auto numeric_index = canonical_numeric_index_string(global_object(), property_key);
             // b. If numericIndex is not undefined, return ! IsValidIntegerIndex(O, numericIndex).
-            if (!numeric_index.is_undefined())
-                return is_valid_integer_index(*this, numeric_index);
+            if (auto numeric_index = canonical_numeric_index_string(property_key); numeric_index.has_value())
+                return is_valid_integer_index(*this, *numeric_index);
         }
 
         // 4. Return ? OrdinaryHasProperty(O, P).
@@ -264,11 +255,10 @@ public:
         // NOTE: This includes an implementation-defined optimization, see note above!
         if (property_key.is_string() || property_key.is_number()) {
             // a. Let numericIndex be ! CanonicalNumericIndexString(P).
-            auto numeric_index = canonical_numeric_index_string(global_object(), property_key);
             // b. If numericIndex is not undefined, then
-            if (!numeric_index.is_undefined()) {
+            if (auto numeric_index = canonical_numeric_index_string(property_key); numeric_index.has_value()) {
                 // i. If ! IsValidIntegerIndex(O, numericIndex) is false, return false.
-                if (!is_valid_integer_index(*this, numeric_index))
+                if (!is_valid_integer_index(*this, *numeric_index))
                     return false;
 
                 // ii. If Desc has a [[Configurable]] field and if Desc.[[Configurable]] is false, return false.
@@ -289,7 +279,7 @@ public:
 
                 // vi. If Desc has a [[Value]] field, perform ? IntegerIndexedElementSet(O, numericIndex, Desc.[[Value]]).
                 if (property_descriptor.value.has_value())
-                    TRY(integer_indexed_element_set<T>(*this, numeric_index, *property_descriptor.value));
+                    TRY(integer_indexed_element_set<T>(*this, *numeric_index, *property_descriptor.value));
 
                 // vii. Return true.
                 return true;
@@ -315,11 +305,10 @@ public:
         // NOTE: This includes an implementation-defined optimization, see note above!
         if (property_key.is_string() || property_key.is_number()) {
             // a. Let numericIndex be ! CanonicalNumericIndexString(P).
-            auto numeric_index = canonical_numeric_index_string(global_object(), property_key);
             // b. If numericIndex is not undefined, then
-            if (!numeric_index.is_undefined()) {
+            if (auto numeric_index = canonical_numeric_index_string(property_key); numeric_index.has_value()) {
                 // i. Return ! IntegerIndexedElementGet(O, numericIndex).
-                return integer_indexed_element_get<T>(*this, numeric_index);
+                return integer_indexed_element_get<T>(*this, *numeric_index);
             }
         }
 
@@ -343,11 +332,10 @@ public:
         // NOTE: This includes an implementation-defined optimization, see note above!
         if (property_key.is_string() || property_key.is_number()) {
             // a. Let numericIndex be ! CanonicalNumericIndexString(P).
-            auto numeric_index = canonical_numeric_index_string(global_object(), property_key);
             // b. If numericIndex is not undefined, then
-            if (!numeric_index.is_undefined()) {
+            if (auto numeric_index = canonical_numeric_index_string(property_key); numeric_index.has_value()) {
                 // i. Perform ? IntegerIndexedElementSet(O, numericIndex, V).
-                TRY(integer_indexed_element_set<T>(*this, numeric_index, value));
+                TRY(integer_indexed_element_set<T>(*this, *numeric_index, value));
 
                 // ii. Return true.
                 return true;
@@ -373,11 +361,10 @@ public:
         // NOTE: This includes an implementation-defined optimization, see note above!
         if (property_key.is_string() || property_key.is_number()) {
             // a. Let numericIndex be ! CanonicalNumericIndexString(P).
-            auto numeric_index = canonical_numeric_index_string(global_object(), property_key);
             // b. If numericIndex is not undefined, then
-            if (!numeric_index.is_undefined()) {
+            if (auto numeric_index = canonical_numeric_index_string(property_key); numeric_index.has_value()) {
                 // i. If ! IsValidIntegerIndex(O, numericIndex) is false, return true; else return false.
-                if (!is_valid_integer_index(*this, numeric_index))
+                if (!is_valid_integer_index(*this, *numeric_index))
                     return true;
                 return false;
             }
diff --git a/Userland/Libraries/LibJS/Runtime/Value.h b/Userland/Libraries/LibJS/Runtime/Value.h
index 7b96309cf9..6a001cf9b0 100644
--- a/Userland/Libraries/LibJS/Runtime/Value.h
+++ b/Userland/Libraries/LibJS/Runtime/Value.h
@@ -55,6 +55,7 @@ public:
     bool is_undefined() const { return m_type == Type::Undefined; }
     bool is_null() const { return m_type == Type::Null; }
     bool is_number() const { return m_type == Type::Int32 || m_type == Type::Double; }
+    bool is_int32() const { return m_type == Type::Int32; }
     bool is_string() const { return m_type == Type::String; }
     bool is_object() const { return m_type == Type::Object; }
     bool is_boolean() const { return m_type == Type::Boolean; }