From 533170fbfa597c696be4d380b28691d91ab216e6 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Tue, 8 Mar 2022 11:56:42 -0500 Subject: [PATCH] LibJS: Combine DeclarativeEnvironment's bindings and names into one list This reduces the size of the DeclarativeEnvironment from 72 bytes to 48 bytes. This savings helps in the context of nested for-loops which use 'let' to bind the initial variable declarations. In this case, the spec dicates we create a new environment for each loop iteration by way of the CreatePerIterationEnvironment AO. In particular, test262's generated RegExp tests contains many loops of the form: for (let i = 0; i < a_number_on_the_order_of_10; ++i) for (let j = 0; j < a_number_on_the_order_of_10_thousand; ++j) This results in creating hundreds of thousands of environments. --- .../LibJS/Runtime/DeclarativeEnvironment.cpp | 27 +++++++++---------- .../LibJS/Runtime/DeclarativeEnvironment.h | 24 +++++++++++++++-- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp index 4099785a4c..758983ce7d 100644 --- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp +++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp @@ -37,7 +37,7 @@ void DeclarativeEnvironment::visit_edges(Visitor& visitor) // 9.1.1.1.1 HasBinding ( N ), https://tc39.es/ecma262/#sec-declarative-environment-records-hasbinding-n ThrowCompletionOr DeclarativeEnvironment::has_binding(FlyString const& name, Optional* out_index) const { - auto index = m_names.find_first_index(name); + auto index = find_binding_index(name); if (!index.has_value()) return false; if (!is_permanently_screwed_by_eval() && out_index) @@ -50,16 +50,16 @@ ThrowCompletionOr DeclarativeEnvironment::create_mutable_binding(GlobalObj { // 2. Create a mutable binding in envRec for N and record that it is uninitialized. If D is true, record that the newly created binding may be deleted by a subsequent DeleteBinding call. m_bindings.append(Binding { + .name = name, .value = {}, .strict = false, .mutable_ = true, .can_be_deleted = can_be_deleted, .initialized = false, }); - m_names.append(name); // 1. Assert: envRec does not already have a binding for N. - // NOTE: We skip this to avoid O(n) traversal of m_names. + // NOTE: We skip this to avoid O(n) traversal of m_bindings. // 3. Return NormalCompletion(empty). return {}; @@ -70,16 +70,16 @@ ThrowCompletionOr DeclarativeEnvironment::create_immutable_binding(GlobalO { // 2. Create an immutable binding in envRec for N and record that it is uninitialized. If S is true, record that the newly created binding is a strict binding. m_bindings.append(Binding { + .name = name, .value = {}, .strict = strict, .mutable_ = false, .can_be_deleted = false, .initialized = false, }); - m_names.append(name); // 1. Assert: envRec does not already have a binding for N. - // NOTE: We skip this to avoid O(n) traversal of m_names. + // NOTE: We skip this to avoid O(n) traversal of m_bindings. // 3. Return NormalCompletion(empty). return {}; @@ -88,7 +88,7 @@ ThrowCompletionOr DeclarativeEnvironment::create_immutable_binding(GlobalO // 9.1.1.1.4 InitializeBinding ( N, V ), https://tc39.es/ecma262/#sec-declarative-environment-records-initializebinding-n-v ThrowCompletionOr DeclarativeEnvironment::initialize_binding(GlobalObject&, FlyString const& name, Value value) { - auto index = m_names.find_first_index(name); + auto index = find_binding_index(name); VERIFY(index.has_value()); auto& binding = m_bindings[*index]; @@ -109,7 +109,7 @@ ThrowCompletionOr DeclarativeEnvironment::initialize_binding(GlobalObject& ThrowCompletionOr DeclarativeEnvironment::set_mutable_binding(GlobalObject& global_object, FlyString const& name, Value value, bool strict) { // 1. If envRec does not have a binding for N, then - auto index = m_names.find_first_index(name); + auto index = find_binding_index(name); if (!index.has_value()) { // a. If S is true, throw a ReferenceError exception. if (strict) @@ -139,7 +139,7 @@ ThrowCompletionOr DeclarativeEnvironment::set_mutable_binding_direct(Globa strict = true; if (!binding.initialized) - return vm().throw_completion(global_object, ErrorType::BindingNotInitialized, m_names[index]); + return vm().throw_completion(global_object, ErrorType::BindingNotInitialized, binding.name); if (binding.mutable_) { binding.value = value; @@ -155,7 +155,7 @@ ThrowCompletionOr DeclarativeEnvironment::set_mutable_binding_direct(Globa ThrowCompletionOr DeclarativeEnvironment::get_binding_value(GlobalObject& global_object, FlyString const& name, bool strict) { // 1. Assert: envRec has a binding for N. - auto index = m_names.find_first_index(name); + auto index = find_binding_index(name); VERIFY(index.has_value()); // 2-3. (extracted into a non-standard function below) @@ -168,7 +168,7 @@ ThrowCompletionOr DeclarativeEnvironment::get_binding_value_direct(Global // 2. If the binding for N in envRec is an uninitialized binding, throw a ReferenceError exception. if (!binding.initialized) - return vm().throw_completion(global_object, ErrorType::BindingNotInitialized, m_names[index]); + return vm().throw_completion(global_object, ErrorType::BindingNotInitialized, binding.name); // 3. Return the value currently bound to N in envRec. return binding.value; @@ -178,7 +178,7 @@ ThrowCompletionOr DeclarativeEnvironment::get_binding_value_direct(Global ThrowCompletionOr DeclarativeEnvironment::delete_binding(GlobalObject&, FlyString const& name) { // 1. Assert: envRec has a binding for the name that is the value of N. - auto index = m_names.find_first_index(name); + auto index = find_binding_index(name); VERIFY(index.has_value()); auto& binding = m_bindings[*index]; @@ -188,9 +188,8 @@ ThrowCompletionOr DeclarativeEnvironment::delete_binding(GlobalObject&, Fl return false; // 3. Remove the binding for N from envRec. - // NOTE: We keep the entries in m_bindings and m_names to avoid disturbing indices. + // NOTE: We keep the entries in m_bindings to avoid disturbing indices. binding = {}; - m_names[*index] = {}; // 4. Return true. return true; @@ -198,7 +197,7 @@ ThrowCompletionOr DeclarativeEnvironment::delete_binding(GlobalObject&, Fl ThrowCompletionOr DeclarativeEnvironment::initialize_or_set_mutable_binding(GlobalObject& global_object, FlyString const& name, Value value) { - auto index = m_names.find_first_index(name); + auto index = find_binding_index(name); VERIFY(index.has_value()); auto& binding = m_bindings[*index]; if (!binding.initialized) diff --git a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h index 036620bb42..67d36b9533 100644 --- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h +++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h @@ -34,7 +34,16 @@ public: ThrowCompletionOr initialize_or_set_mutable_binding(GlobalObject& global_object, FlyString const& name, Value value); // This is not a method defined in the spec! Do not use this in any LibJS (or other spec related) code. - [[nodiscard]] Vector const& bindings() const { return m_names; } + [[nodiscard]] Vector bindings() const + { + Vector names; + names.ensure_capacity(m_bindings.size()); + + for (auto const& binding : m_bindings) + names.unchecked_append(binding.name); + + return names; + } ThrowCompletionOr get_binding_value_direct(GlobalObject&, size_t index, bool strict); ThrowCompletionOr set_mutable_binding_direct(GlobalObject&, size_t index, Value, bool strict); @@ -45,7 +54,19 @@ protected: private: virtual bool is_declarative_environment() const override { return true; } + Optional find_binding_index(FlyString const& name) const + { + auto it = m_bindings.find_if([&](auto const& binding) { + return binding.name == name; + }); + + if (it == m_bindings.end()) + return {}; + return it.index(); + } + struct Binding { + FlyString name; Value value; bool strict { false }; bool mutable_ { false }; @@ -53,7 +74,6 @@ private: bool initialized { false }; }; - Vector m_names; Vector m_bindings; };