From 969be11931ef7501f7d789ff4e4520741b44c059 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Fri, 26 May 2023 19:42:47 +0200 Subject: [PATCH 1/7] Remove redundant `num_bindings` field from `CodeBlock` --- boa_engine/src/bytecompiler/class.rs | 16 +++++--------- boa_engine/src/bytecompiler/declarations.rs | 1 - boa_engine/src/bytecompiler/function.rs | 2 +- boa_engine/src/bytecompiler/mod.rs | 13 ------------ boa_engine/src/environments/runtime/mod.rs | 3 ++- boa_engine/src/vm/code_block.rs | 23 +++++++++------------ 6 files changed, 18 insertions(+), 40 deletions(-) diff --git a/boa_engine/src/bytecompiler/class.rs b/boa_engine/src/bytecompiler/class.rs index f4b9ea0cbcc..c25e3ca3772 100644 --- a/boa_engine/src/bytecompiler/class.rs +++ b/boa_engine/src/bytecompiler/class.rs @@ -56,15 +56,13 @@ impl ByteCompiler<'_, '_> { compiler.patch_jump_with_target(env_labels.1, env_info.index); compiler.pop_compile_environment(); } else { - compiler.num_bindings = env_info.num_bindings; compiler.is_class_constructor = true; } } else { if class.super_ref().is_some() { compiler.emit_opcode(Opcode::SuperCallDerived); } - let env_info = compiler.pop_compile_environment(); - compiler.num_bindings = env_info.num_bindings; + compiler.pop_compile_environment(); compiler.is_class_constructor = true; } @@ -266,9 +264,8 @@ impl ByteCompiler<'_, '_> { } else { field_compiler.emit_opcode(Opcode::PushUndefined); } - let env_info = field_compiler.pop_compile_environment(); field_compiler.pop_compile_environment(); - field_compiler.num_bindings = env_info.num_bindings; + field_compiler.pop_compile_environment(); field_compiler.emit_opcode(Opcode::Return); let mut code = field_compiler.finish(); @@ -298,9 +295,8 @@ impl ByteCompiler<'_, '_> { } else { field_compiler.emit_opcode(Opcode::PushUndefined); } - let env_info = field_compiler.pop_compile_environment(); field_compiler.pop_compile_environment(); - field_compiler.num_bindings = env_info.num_bindings; + field_compiler.pop_compile_environment(); field_compiler.emit_opcode(Opcode::Return); let mut code = field_compiler.finish(); @@ -340,9 +336,8 @@ impl ByteCompiler<'_, '_> { } else { field_compiler.emit_opcode(Opcode::PushUndefined); } - let env_info = field_compiler.pop_compile_environment(); field_compiler.pop_compile_environment(); - field_compiler.num_bindings = env_info.num_bindings; + field_compiler.pop_compile_environment(); field_compiler.emit_opcode(Opcode::Return); let mut code = field_compiler.finish(); @@ -392,9 +387,8 @@ impl ByteCompiler<'_, '_> { ); compiler.compile_statement_list(body.statements(), false, false); - let env_info = compiler.pop_compile_environment(); compiler.pop_compile_environment(); - compiler.num_bindings = env_info.num_bindings; + compiler.pop_compile_environment(); let code = Gc::new(compiler.finish()); let index = self.functions.len() as u32; diff --git a/boa_engine/src/bytecompiler/declarations.rs b/boa_engine/src/bytecompiler/declarations.rs index 6c9ec81feef..e6aa572cec5 100644 --- a/boa_engine/src/bytecompiler/declarations.rs +++ b/boa_engine/src/bytecompiler/declarations.rs @@ -987,7 +987,6 @@ impl ByteCompiler<'_, '_> { // b. Let varEnv be NewDeclarativeEnvironment(env). // c. Set the VariableEnvironment of calleeContext to varEnv. self.push_compile_environment(true); - self.function_environment_push_location = self.next_opcode_location(); env_labels = Some(self.emit_opcode_with_two_operands(Opcode::PushFunctionEnvironment)); // d. Let instantiatedVarNames be a new empty List. diff --git a/boa_engine/src/bytecompiler/function.rs b/boa_engine/src/bytecompiler/function.rs index f1a40bc7db2..edd585130e4 100644 --- a/boa_engine/src/bytecompiler/function.rs +++ b/boa_engine/src/bytecompiler/function.rs @@ -138,7 +138,7 @@ impl FunctionCompiler { Some(compiler.pop_compile_environment().num_bindings); } - compiler.num_bindings = compiler.pop_compile_environment().num_bindings; + compiler.pop_compile_environment(); if self.binding_identifier.is_some() { compiler.pop_compile_environment(); diff --git a/boa_engine/src/bytecompiler/mod.rs b/boa_engine/src/bytecompiler/mod.rs index d046d060bab..ca2bef03c09 100644 --- a/boa_engine/src/bytecompiler/mod.rs +++ b/boa_engine/src/bytecompiler/mod.rs @@ -244,9 +244,6 @@ pub struct ByteCompiler<'ctx, 'host> { /// Locators for all bindings in the codeblock. pub(crate) bindings: Vec, - /// Number of binding for the function environment. - pub(crate) num_bindings: u32, - /// Functions inside this function pub(crate) functions: Vec>, @@ -262,12 +259,6 @@ pub struct ByteCompiler<'ctx, 'host> { /// The `[[ClassFieldInitializerName]]` internal slot. pub(crate) class_field_initializer_name: Option, - /// Marks the location in the code where the function environment in pushed. - /// This is only relevant for functions with expressions in the parameters. - /// We execute the parameter expressions in the function code and push the function environment afterward. - /// When the execution of the parameter expressions throws an error, we do not need to pop the function environment. - pub(crate) function_environment_push_location: u32, - /// The environment that is currently active. pub(crate) current_environment: Gc>, @@ -313,7 +304,6 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { names: Vec::default(), private_names: Vec::default(), bindings: Vec::default(), - num_bindings: 0, functions: Vec::default(), has_binding_identifier: false, this_mode: ThisMode::Global, @@ -322,7 +312,6 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { compile_environments: Vec::default(), is_class_constructor: false, class_field_initializer_name: None, - function_environment_push_location: 0, parameters_env_bindings: None, literals_map: FxHashMap::default(), @@ -1369,13 +1358,11 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { names: self.names.into_boxed_slice(), private_names: self.private_names.into_boxed_slice(), bindings: self.bindings.into_boxed_slice(), - num_bindings: self.num_bindings, functions: self.functions.into_boxed_slice(), arguments_binding: self.arguments_binding, compile_environments: self.compile_environments.into_boxed_slice(), is_class_constructor: self.is_class_constructor, class_field_initializer_name: self.class_field_initializer_name, - function_environment_push_location: self.function_environment_push_location, parameters_env_bindings: self.parameters_env_bindings, #[cfg(feature = "trace")] trace: std::cell::Cell::new(false), diff --git a/boa_engine/src/environments/runtime/mod.rs b/boa_engine/src/environments/runtime/mod.rs index f90d85d19f9..7ffc67ac641 100644 --- a/boa_engine/src/environments/runtime/mod.rs +++ b/boa_engine/src/environments/runtime/mod.rs @@ -266,10 +266,11 @@ impl EnvironmentStack { #[track_caller] pub(crate) fn push_function( &mut self, - num_bindings: u32, compile_environment: Gc>, function_slots: FunctionSlots, ) { + let num_bindings = compile_environment.borrow().num_bindings(); + let (poisoned, with) = { let with = self .stack diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 4cd1175aa79..9734a54f89e 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -52,6 +52,16 @@ unsafe impl Readable for i64 {} unsafe impl Readable for f32 {} unsafe impl Readable for f64 {} +// bitflags! { +// #[derive(Clone, Copy, Debug, Finalize)] +// struct Flags: u32 { +// const A = 0b00000001; +// const B = 0b00000010; +// const C = 0b00000100; +// const ABC = Self::A.bits() | Self::B.bits() | Self::C.bits(); +// } +// } + /// The internal representation of a JavaScript function. /// /// A `CodeBlock` is generated for each function compiled by the @@ -96,9 +106,6 @@ pub struct CodeBlock { #[unsafe_ignore_trace] pub(crate) bindings: Box<[BindingLocator]>, - /// Number of binding for the function environment. - pub(crate) num_bindings: u32, - /// Functions inside this function pub(crate) functions: Box<[Gc]>, @@ -116,12 +123,6 @@ pub struct CodeBlock { #[unsafe_ignore_trace] pub(crate) class_field_initializer_name: Option, - /// Marks the location in the code where the function environment in pushed. - /// This is only relevant for functions with expressions in the parameters. - /// We execute the parameter expressions in the function code and push the function environment afterward. - /// When the execution of the parameter expressions throws an error, we do not need to pop the function environment. - pub(crate) function_environment_push_location: u32, - /// The number of bindings in the parameters environment. pub(crate) parameters_env_bindings: Option, @@ -142,7 +143,6 @@ impl CodeBlock { names: Box::default(), private_names: Box::default(), bindings: Box::default(), - num_bindings: 0, functions: Box::default(), name, has_binding_identifier: false, @@ -154,7 +154,6 @@ impl CodeBlock { compile_environments: Box::default(), is_class_constructor: false, class_field_initializer_name: None, - function_environment_push_location: 0, parameters_env_bindings: None, #[cfg(feature = "trace")] trace: std::cell::Cell::new(false), @@ -1030,7 +1029,6 @@ impl JsObject { } context.vm.environments.push_function( - code.num_bindings, code.compile_environments[last_env].clone(), FunctionSlots::new(this, self.clone(), None), ); @@ -1284,7 +1282,6 @@ impl JsObject { } context.vm.environments.push_function( - code.num_bindings, code.compile_environments[last_env].clone(), FunctionSlots::new( this.clone().map_or(ThisBindingStatus::Uninitialized, |o| { From 2370a4fd51c75478a04090bdb088f33cc2ac550b Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Fri, 26 May 2023 22:14:46 +0200 Subject: [PATCH 2/7] Remove redundant num_bindings parameter from push_function_inherits --- boa_engine/src/environments/runtime/mod.rs | 5 +++-- boa_engine/src/vm/code_block.rs | 14 +++++++------- boa_engine/src/vm/opcode/push/environment.rs | 11 ++++------- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/boa_engine/src/environments/runtime/mod.rs b/boa_engine/src/environments/runtime/mod.rs index 7ffc67ac641..06ada6363d7 100644 --- a/boa_engine/src/environments/runtime/mod.rs +++ b/boa_engine/src/environments/runtime/mod.rs @@ -222,9 +222,10 @@ impl EnvironmentStack { #[track_caller] pub(crate) fn push_lexical( &mut self, - num_bindings: u32, compile_environment: Gc>, ) -> u32 { + let num_bindings = compile_environment.borrow().num_bindings(); + let (poisoned, with) = { let with = self .stack @@ -310,9 +311,9 @@ impl EnvironmentStack { #[track_caller] pub(crate) fn push_function_inherit( &mut self, - num_bindings: u32, compile_environment: Gc>, ) { + let num_bindings = compile_environment.borrow().num_bindings(); debug_assert!( self.stack.len() as u32 == compile_environment.borrow().environment_index(), "tried to push an invalid compile environment" diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 9734a54f89e..aa759a6bfb7 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -1008,7 +1008,7 @@ impl JsObject { let index = context .vm .environments - .push_lexical(1, code.compile_environments[last_env].clone()); + .push_lexical(code.compile_environments[last_env].clone()); context .vm .environments @@ -1020,7 +1020,7 @@ impl JsObject { let index = context .vm .environments - .push_lexical(1, code.compile_environments[last_env].clone()); + .push_lexical(code.compile_environments[last_env].clone()); context .vm .environments @@ -1033,12 +1033,12 @@ impl JsObject { FunctionSlots::new(this, self.clone(), None), ); - if let Some(bindings) = code.parameters_env_bindings { + if let Some(_bindings) = code.parameters_env_bindings { last_env -= 1; context .vm .environments - .push_lexical(bindings, code.compile_environments[last_env].clone()); + .push_lexical(code.compile_environments[last_env].clone()); } if let Some(binding) = code.arguments_binding { @@ -1273,7 +1273,7 @@ impl JsObject { let index = context .vm .environments - .push_lexical(1, code.compile_environments[last_env].clone()); + .push_lexical(code.compile_environments[last_env].clone()); context .vm .environments @@ -1292,11 +1292,11 @@ impl JsObject { ), ); - if let Some(bindings) = code.parameters_env_bindings { + if let Some(_bindings) = code.parameters_env_bindings { context .vm .environments - .push_lexical(bindings, code.compile_environments[0].clone()); + .push_lexical(code.compile_environments[0].clone()); } if let Some(binding) = code.arguments_binding { diff --git a/boa_engine/src/vm/opcode/push/environment.rs b/boa_engine/src/vm/opcode/push/environment.rs index 67a9cca76ba..5ce2fa57d80 100644 --- a/boa_engine/src/vm/opcode/push/environment.rs +++ b/boa_engine/src/vm/opcode/push/environment.rs @@ -17,15 +17,12 @@ impl Operation for PushDeclarativeEnvironment { const INSTRUCTION: &'static str = "INST - PushDeclarativeEnvironment"; fn execute(context: &mut Context<'_>) -> JsResult { - let num_bindings = context.vm.read::(); + let _num_bindings = context.vm.read::(); let compile_environments_index = context.vm.read::(); let compile_environment = context.vm.frame().code_block.compile_environments [compile_environments_index as usize] .clone(); - context - .vm - .environments - .push_lexical(num_bindings, compile_environment); + context.vm.environments.push_lexical(compile_environment); context.vm.frame_mut().inc_frame_env_stack(); Ok(CompletionType::Normal) } @@ -43,7 +40,7 @@ impl Operation for PushFunctionEnvironment { const INSTRUCTION: &'static str = "INST - PushFunctionEnvironment"; fn execute(context: &mut Context<'_>) -> JsResult { - let num_bindings = context.vm.read::(); + let _num_bindings = context.vm.read::(); let compile_environments_index = context.vm.read::(); let compile_environment = context.vm.frame().code_block.compile_environments [compile_environments_index as usize] @@ -51,7 +48,7 @@ impl Operation for PushFunctionEnvironment { context .vm .environments - .push_function_inherit(num_bindings, compile_environment); + .push_function_inherit(compile_environment); Ok(CompletionType::Normal) } } From be5454d7876fe45a0ff295ffcab4fe0bddda9017 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Sat, 27 May 2023 06:06:47 +0200 Subject: [PATCH 3/7] Remove redundant num_bindings operand from environment opcodes --- boa_engine/src/builtins/eval/mod.rs | 7 ++-- boa_engine/src/bytecompiler/class.rs | 14 ++++---- boa_engine/src/bytecompiler/declarations.rs | 9 +++-- boa_engine/src/bytecompiler/env.rs | 4 +-- boa_engine/src/bytecompiler/function.rs | 11 +++---- boa_engine/src/bytecompiler/mod.rs | 8 ++--- .../src/bytecompiler/statement/block.rs | 5 ++- boa_engine/src/bytecompiler/statement/loop.rs | 33 ++++++++----------- .../src/bytecompiler/statement/switch.rs | 5 ++- boa_engine/src/bytecompiler/statement/try.rs | 5 ++- boa_engine/src/environments/runtime/mod.rs | 1 + boa_engine/src/vm/code_block.rs | 30 +++++++---------- boa_engine/src/vm/flowgraph/mod.rs | 1 - boa_engine/src/vm/opcode/mod.rs | 4 +-- boa_engine/src/vm/opcode/push/environment.rs | 2 -- 15 files changed, 59 insertions(+), 80 deletions(-) diff --git a/boa_engine/src/builtins/eval/mod.rs b/boa_engine/src/builtins/eval/mod.rs index 0a4f0b38159..2bf9a508fed 100644 --- a/boa_engine/src/builtins/eval/mod.rs +++ b/boa_engine/src/builtins/eval/mod.rs @@ -232,14 +232,15 @@ impl Eval { ); compiler.push_compile_environment(strict); - let push_env = compiler.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment); + + let push_env = compiler.emit_opcode_with_operand(Opcode::PushDeclarativeEnvironment); compiler.eval_declaration_instantiation(&body, strict)?; compiler.compile_statement_list(body.statements(), true, false); let env_info = compiler.pop_compile_environment(); - compiler.patch_jump_with_target(push_env.0, env_info.num_bindings); - compiler.patch_jump_with_target(push_env.1, env_info.index); + compiler.patch_jump_with_target(push_env, env_info.index); + compiler.emit_opcode(Opcode::PopEnvironment); let code_block = Gc::new(compiler.finish()); diff --git a/boa_engine/src/bytecompiler/class.rs b/boa_engine/src/bytecompiler/class.rs index c25e3ca3772..8ac5d47089a 100644 --- a/boa_engine/src/bytecompiler/class.rs +++ b/boa_engine/src/bytecompiler/class.rs @@ -39,7 +39,7 @@ impl ByteCompiler<'_, '_> { compiler.length = expr.parameters().length(); compiler.params = expr.parameters().clone(); - let (env_labels, _) = compiler.function_declaration_instantiation( + let (env_label, _) = compiler.function_declaration_instantiation( expr.body(), expr.parameters(), false, @@ -51,9 +51,8 @@ impl ByteCompiler<'_, '_> { let env_info = compiler.pop_compile_environment(); - if let Some(env_labels) = env_labels { - compiler.patch_jump_with_target(env_labels.0, env_info.num_bindings); - compiler.patch_jump_with_target(env_labels.1, env_info.index); + if let Some(env_label) = env_label { + compiler.patch_jump_with_target(env_label, env_info.index); compiler.pop_compile_environment(); } else { compiler.is_class_constructor = true; @@ -79,11 +78,11 @@ impl ByteCompiler<'_, '_> { self.emit(Opcode::GetFunction, &[index]); self.emit_u8(0); - let class_env: Option<(super::Label, super::Label)> = match class.name() { + let class_env: Option = match class.name() { Some(name) if class.has_binding_identifier() => { self.push_compile_environment(false); self.create_immutable_binding(name, true); - Some(self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment)) + Some(self.emit_opcode_with_operand(Opcode::PushDeclarativeEnvironment)) } _ => None, }; @@ -542,8 +541,7 @@ impl ByteCompiler<'_, '_> { if let Some(class_env) = class_env { let env_info = self.pop_compile_environment(); - self.patch_jump_with_target(class_env.0, env_info.num_bindings); - self.patch_jump_with_target(class_env.1, env_info.index); + self.patch_jump_with_target(class_env, env_info.index); self.emit_opcode(Opcode::PopEnvironment); } diff --git a/boa_engine/src/bytecompiler/declarations.rs b/boa_engine/src/bytecompiler/declarations.rs index e6aa572cec5..b049fa6996f 100644 --- a/boa_engine/src/bytecompiler/declarations.rs +++ b/boa_engine/src/bytecompiler/declarations.rs @@ -777,8 +777,8 @@ impl ByteCompiler<'_, '_> { arrow: bool, strict: bool, generator: bool, - ) -> (Option<(Label, Label)>, bool) { - let mut env_labels = None; + ) -> (Option