From 509c10d14db0f2e2ce2b89f307d9dd360b855eb5 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 9 Jul 2024 11:37:06 +0200 Subject: [PATCH] LibJS: Make GetById and GetByValue avoid get_identifier() in common case We now defer looking up the various identifiers by IdentifierTableIndex until the last moment. This allows us to avoid the retrieval in common cases like when a property access is cached. Knocks a ~12% item off the profile on https://ventrella.com/Clusters/ --- .../Libraries/LibJS/Bytecode/Executable.h | 2 + .../Libraries/LibJS/Bytecode/Generator.cpp | 2 + Userland/Libraries/LibJS/Bytecode/Generator.h | 2 + .../Libraries/LibJS/Bytecode/Interpreter.cpp | 73 +++++++++++++------ 4 files changed, 57 insertions(+), 22 deletions(-) diff --git a/Userland/Libraries/LibJS/Bytecode/Executable.h b/Userland/Libraries/LibJS/Bytecode/Executable.h index 610b8961c3d..9bdf2bcb17d 100644 --- a/Userland/Libraries/LibJS/Bytecode/Executable.h +++ b/Userland/Libraries/LibJS/Bytecode/Executable.h @@ -86,6 +86,8 @@ public: Vector local_variable_names; size_t local_index_base { 0 }; + Optional length_identifier; + ByteString const& get_string(StringTableIndex index) const { return string_table->get(index); } DeprecatedFlyString const& get_identifier(IdentifierTableIndex index) const { return identifier_table->get(index); } diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.cpp b/Userland/Libraries/LibJS/Bytecode/Generator.cpp index fa54cb6adfc..a7b1d651a35 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Generator.cpp @@ -453,6 +453,7 @@ CodeGenerationErrorOr> Generator::compile(VM& vm, ASTNo executable->source_map = move(source_map); executable->local_variable_names = move(local_variable_names); executable->local_index_base = number_of_registers + number_of_constants; + executable->length_identifier = generator.m_length_identifier; generator.m_finished = true; @@ -1075,6 +1076,7 @@ CodeGenerationErrorOr> Generator::emit_named_evaluation_ void Generator::emit_get_by_id(ScopedOperand dst, ScopedOperand base, IdentifierTableIndex property_identifier, Optional base_identifier) { if (m_identifier_table->get(property_identifier) == "length"sv) { + m_length_identifier = property_identifier; emit(dst, base, move(base_identifier), m_next_property_lookup_cache++); return; } diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.h b/Userland/Libraries/LibJS/Bytecode/Generator.h index 139ae51791d..efdeb94def7 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.h +++ b/Userland/Libraries/LibJS/Bytecode/Generator.h @@ -402,6 +402,8 @@ private: bool m_must_propagate_completion { true }; GCPtr m_function; + + Optional m_length_identifier; }; } diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp index 4cebd5120f3..956e9144c80 100644 --- a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp @@ -876,6 +876,24 @@ inline void fast_typed_array_set_element(TypedArrayBase& typed_array, u32 index, *slot = value; } +Completion throw_null_or_undefined_property_get(VM& vm, Value base_value, Optional base_identifier, IdentifierTableIndex property_identifier, Executable const& executable) +{ + VERIFY(base_value.is_nullish()); + + if (base_identifier.has_value()) + return vm.throw_completion(ErrorType::ToObjectNullOrUndefinedWithPropertyAndName, executable.get_identifier(property_identifier), base_value, executable.get_identifier(base_identifier.value())); + return vm.throw_completion(ErrorType::ToObjectNullOrUndefinedWithProperty, executable.get_identifier(property_identifier), base_value); +} + +Completion throw_null_or_undefined_property_get(VM& vm, Value base_value, Optional base_identifier, Value property, Executable const& executable) +{ + VERIFY(base_value.is_nullish()); + + if (base_identifier.has_value()) + return vm.throw_completion(ErrorType::ToObjectNullOrUndefinedWithPropertyAndName, property, base_value, executable.get_identifier(base_identifier.value())); + return vm.throw_completion(ErrorType::ToObjectNullOrUndefinedWithProperty, property, base_value); +} + template ALWAYS_INLINE Completion throw_null_or_undefined_property_access(VM& vm, Value base_value, BaseType const& base_identifier, PropertyType const& property_identifier) { @@ -898,8 +916,7 @@ ALWAYS_INLINE Completion throw_null_or_undefined_property_access(VM& vm, Value b return vm.throw_completion(ErrorType::ToObjectNullOrUndefined); } -template -ALWAYS_INLINE ThrowCompletionOr> base_object_for_get(VM& vm, Value base_value, BaseType const& base_identifier, PropertyType const& property_identifier) +ALWAYS_INLINE GCPtr base_object_for_get_impl(VM& vm, Value base_value) { if (base_value.is_object()) [[likely]] return base_value.as_object(); @@ -917,8 +934,25 @@ ALWAYS_INLINE ThrowCompletionOr> base_object_for_get(VM& vm if (base_value.is_symbol()) return realm.intrinsics().symbol_prototype(); + return nullptr; +} + +ALWAYS_INLINE ThrowCompletionOr> base_object_for_get(VM& vm, Value base_value, Optional base_identifier, IdentifierTableIndex property_identifier, Executable const& executable) +{ + if (auto base_object = base_object_for_get_impl(vm, base_value)) + return NonnullGCPtr { *base_object }; + // NOTE: At this point this is guaranteed to throw (null or undefined). - return throw_null_or_undefined_property_access(vm, base_value, base_identifier, property_identifier); + return throw_null_or_undefined_property_get(vm, base_value, base_identifier, property_identifier, executable); +} + +ALWAYS_INLINE ThrowCompletionOr> base_object_for_get(VM& vm, Value base_value, Optional base_identifier, Value property, Executable const& executable) +{ + if (auto base_object = base_object_for_get_impl(vm, base_value)) + return NonnullGCPtr { *base_object }; + + // NOTE: At this point this is guaranteed to throw (null or undefined). + return throw_null_or_undefined_property_get(vm, base_value, base_identifier, property, executable); } enum class GetByIdMode { @@ -927,7 +961,7 @@ enum class GetByIdMode { }; template -inline ThrowCompletionOr get_by_id(VM& vm, Optional const& base_identifier, DeprecatedFlyString const& property, Value base_value, Value this_value, PropertyLookupCache& cache) +inline ThrowCompletionOr get_by_id(VM& vm, Optional base_identifier, IdentifierTableIndex property, Value base_value, Value this_value, PropertyLookupCache& cache, Executable const& executable) { if constexpr (mode == GetByIdMode::Length) { if (base_value.is_string()) { @@ -935,7 +969,7 @@ inline ThrowCompletionOr get_by_id(VM& vm, Optional get_by_id(VM& vm, Optionalinternal_get(property, this_value, &cacheable_metadata)); + auto value = TRY(base_obj->internal_get(executable.get_identifier(property), this_value, &cacheable_metadata)); if (cacheable_metadata.type == CacheablePropertyMetadata::Type::OwnProperty) { cache = {}; @@ -982,7 +1016,7 @@ inline ThrowCompletionOr get_by_id(VM& vm, Optional get_by_value(VM& vm, Optional const& base_identifier, Value base_value, Value property_key_value) +inline ThrowCompletionOr get_by_value(VM& vm, Optional base_identifier, Value base_value, Value property_key_value, Executable const& executable) { // OPTIMIZATION: Fast path for simple Int32 indexes in array-like objects. if (base_value.is_object() && property_key_value.is_int32() && property_key_value.as_i32() >= 0) { @@ -1044,7 +1078,7 @@ inline ThrowCompletionOr get_by_value(VM& vm, Optional SetVariableBinding::execute_impl(Bytecode::Interpreter& ThrowCompletionOr GetById::execute_impl(Bytecode::Interpreter& interpreter) const { - auto base_identifier = interpreter.current_executable().get_identifier(m_base_identifier); - auto const& property_identifier = interpreter.current_executable().get_identifier(m_property); - auto base_value = interpreter.get(base()); auto& cache = interpreter.current_executable().property_lookup_caches[m_cache_index]; - interpreter.set(dst(), TRY(get_by_id(interpreter.vm(), base_identifier, property_identifier, base_value, base_value, cache))); + interpreter.set(dst(), TRY(get_by_id(interpreter.vm(), m_base_identifier, m_property, base_value, base_value, cache, interpreter.current_executable()))); return {}; } @@ -2324,18 +2355,17 @@ ThrowCompletionOr GetByIdWithThis::execute_impl(Bytecode::Interpreter& int auto base_value = interpreter.get(m_base); auto this_value = interpreter.get(m_this_value); auto& cache = interpreter.current_executable().property_lookup_caches[m_cache_index]; - interpreter.set(dst(), TRY(get_by_id(interpreter.vm(), {}, interpreter.current_executable().get_identifier(m_property), base_value, this_value, cache))); + interpreter.set(dst(), TRY(get_by_id(interpreter.vm(), {}, m_property, base_value, this_value, cache, interpreter.current_executable()))); return {}; } ThrowCompletionOr GetLength::execute_impl(Bytecode::Interpreter& interpreter) const { - auto base_identifier = interpreter.current_executable().get_identifier(m_base_identifier); - auto base_value = interpreter.get(base()); - auto& cache = interpreter.current_executable().property_lookup_caches[m_cache_index]; + auto& executable = interpreter.current_executable(); + auto& cache = executable.property_lookup_caches[m_cache_index]; - interpreter.set(dst(), TRY(get_by_id(interpreter.vm(), base_identifier, interpreter.vm().names.length.as_string(), base_value, base_value, cache))); + interpreter.set(dst(), TRY(get_by_id(interpreter.vm(), m_base_identifier, *executable.length_identifier, base_value, base_value, cache, executable))); return {}; } @@ -2343,8 +2373,9 @@ ThrowCompletionOr GetLengthWithThis::execute_impl(Bytecode::Interpreter& i { auto base_value = interpreter.get(m_base); auto this_value = interpreter.get(m_this_value); - auto& cache = interpreter.current_executable().property_lookup_caches[m_cache_index]; - interpreter.set(dst(), TRY(get_by_id(interpreter.vm(), {}, interpreter.vm().names.length.as_string(), base_value, this_value, cache))); + auto& executable = interpreter.current_executable(); + auto& cache = executable.property_lookup_caches[m_cache_index]; + interpreter.set(dst(), TRY(get_by_id(interpreter.vm(), {}, *executable.length_identifier, base_value, this_value, cache, executable))); return {}; } @@ -2699,9 +2730,7 @@ void Await::execute_impl(Bytecode::Interpreter& interpreter) const ThrowCompletionOr GetByValue::execute_impl(Bytecode::Interpreter& interpreter) const { - auto base_identifier = interpreter.current_executable().get_identifier(m_base_identifier); - - interpreter.set(dst(), TRY(get_by_value(interpreter.vm(), base_identifier, interpreter.get(m_base), interpreter.get(m_property)))); + interpreter.set(dst(), TRY(get_by_value(interpreter.vm(), m_base_identifier, interpreter.get(m_base), interpreter.get(m_property), interpreter.current_executable()))); return {}; }