RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-25 03:27:44 +00:00
Code Issues Activity Actions

LibJS: Get rid of unnecessary work from canonical_numeric_index_string

Browse source 
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.
This commit is contained in:
Anonymous 2022-02-12 12:51:06 -08:00 • committed by Andreas Kling
parent 44a2ebea00
commit 3a184f7841
8 changed files with 57 additions and 98 deletions
Download patch file Download diff file Expand all files Collapse all files

12
Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator.cpp
View file

@ -2501,7 +2501,7 @@ JS::ThrowCompletionOr<Optional<JS::PropertyDescriptor>> @class_name@::legacy_pla
if (interface.supports_indexed_properties()) {
// ...and P is an array index, then:
get_own_property_generator.append(R"~~~(
if (IDL::is_an_array_index(global_object, property_name)) {
if (IDL::is_an_array_index(property_name)) {
// 1. Let index be the result of calling ToUint32(P).
u32 index = property_name.as_number();
@ -2776,7 +2776,7 @@ JS::ThrowCompletionOr<bool> @class_name@::internal_set(JS::PropertyKey const& pr
if (interface.indexed_property_setter.has_value()) {
// ...and P is an array index, then:
scoped_generator.append(R"~~~(
if (IDL::is_an_array_index(global_object, property_name)) {
if (IDL::is_an_array_index(property_name)) {
// 1. Invoke the indexed property setter on O with P and V.
TRY(invoke_indexed_property_setter(global_object, impl(), property_name, value));
@ -2821,14 +2821,14 @@ JS::ThrowCompletionOr<bool> @class_name@::internal_set(JS::PropertyKey const& pr
JS::ThrowCompletionOr<bool> @class_name@::internal_define_own_property(JS::PropertyKey const& property_name, JS::PropertyDescriptor const& property_descriptor)
{
[[maybe_unused]] auto& vm = this->vm();
auto& global_object = this->global_object();
[[maybe_unused]] auto& global_object = this->global_object();
)~~~");
// 1. If O supports indexed properties...
if (interface.supports_indexed_properties()) {
// ...and P is an array index, then:
scoped_generator.append(R"~~~(
if (IDL::is_an_array_index(global_object, property_name)) {
if (IDL::is_an_array_index(property_name)) {
// 1. If the result of calling IsDataDescriptor(Desc) is false, then return false.
if (!property_descriptor.is_data_descriptor())
return false;
@ -2940,14 +2940,14 @@ JS::ThrowCompletionOr<bool> @class_name@::internal_define_own_property(JS::Prope
scoped_generator.append(R"~~~(
JS::ThrowCompletionOr<bool> @class_name@::internal_delete(JS::PropertyKey const& property_name)
{
auto& global_object = this->global_object();
[[maybe_unused]] auto& global_object = this->global_object();
)~~~");
// 1. If O supports indexed properties...
if (interface.supports_indexed_properties()) {
// ...and P is an array index, then:
scoped_generator.append(R"~~~(
if (IDL::is_an_array_index(global_object, property_name)) {
if (IDL::is_an_array_index(property_name)) {
// 1. Let index be the result of calling ToUint32(P).
u32 index = property_name.as_number();

24
Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp
View file

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

3
Userland/Libraries/LibJS/Runtime/AbstractOperations.h
View file

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

27
Userland/Libraries/LibJS/Runtime/StringObject.cpp
View file

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

51
Userland/Libraries/LibJS/Runtime/TypedArray.h
View file

@ -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;
}

1
Userland/Libraries/LibJS/Runtime/Value.h
View file

@ -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; }

35
Userland/Libraries/LibWeb/Bindings/IDLAbstractOperations.cpp
View file

@ -17,39 +17,26 @@
namespace Web::Bindings::IDL {
// https://webidl.spec.whatwg.org/#is-an-array-index
bool is_an_array_index(JS::GlobalObject& global_object, JS::PropertyKey const& property_name)
bool is_an_array_index(JS::PropertyKey const& property_name)
{
// 1. If Type(P) is not String, then return false.
if (!property_name.is_number())
return false;
// 2. Let index be ! CanonicalNumericIndexString(P).
auto index = JS::canonical_numeric_index_string(global_object, property_name);
auto index = JS::canonical_numeric_index_string(property_name);
// 3. If index is undefined, then return false.
if (index.is_undefined())
if (!index.has_value()) {
// All of these are handled by canonical_numeric_index_string
// 3. If index is undefined, then return false.
// 4. If IsInteger(index) is false, then return false.
// 5. If index is −0, then return false.
// 6. If index < 0, then return false.
// 7. If index ≥ 2 ** 32 − 1, then return false.
// Note: 2 ** 32 − 1 is the maximum array length allowed by ECMAScript.
return false;
}
// 4. If IsInteger(index) is false, then return false.
// NOTE: IsInteger is the old name of IsIntegralNumber.
if (!index.is_integral_number())
return false;
// 5. If index is −0, then return false.
if (index.is_negative_zero())
return false;
// FIXME: I'm not sure if this is correct.
auto index_as_double = index.as_double();
// 6. If index < 0, then return false.
if (index_as_double < 0)
return false;
// 7. If index ≥ 2 ** 32 − 1, then return false.
// Note: 2 ** 32 − 1 is the maximum array length allowed by ECMAScript.
if (index_as_double >= NumericLimits<u32>::max())
return false;
// 8. Return true.
return true;

2
Userland/Libraries/LibWeb/Bindings/IDLAbstractOperations.h
View file

@ -16,7 +16,7 @@
namespace Web::Bindings::IDL {
bool is_an_array_index(JS::GlobalObject&, JS::PropertyKey const&);
bool is_an_array_index(JS::PropertyKey const&);
Optional<ByteBuffer> get_buffer_source_copy(JS::Object const& buffer_source);
// https://webidl.spec.whatwg.org/#call-user-object-operation-return