Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make update operations reuse the last found binding locator #2876

Merged
merged 9 commits into from
May 3, 2023
1 change: 0 additions & 1 deletion boa_engine/src/builtins/json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ impl Json {
context.realm().environment().compile_env(),
context,
);
compiler.create_script_decls(&statement_list, false);
compiler.compile_statement_list(&statement_list, true, false);
Gc::new(compiler.finish())
};
Expand Down
3 changes: 1 addition & 2 deletions boa_engine/src/bytecompiler/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ impl ByteCompiler<'_, '_> {
} else {
None
};
compiler.create_script_decls(expr.body(), false);
compiler.compile_statement_list(expr.body(), false, false);

let env_info = compiler.pop_compile_environment();
Expand Down Expand Up @@ -397,7 +396,7 @@ impl ByteCompiler<'_, '_> {
compiler.push_compile_environment(false);
compiler.create_immutable_binding(class_name.into(), true);
compiler.push_compile_environment(true);
compiler.create_script_decls(statement_list, false);
compiler.create_declarations(statement_list, false);
compiler.compile_statement_list(statement_list, false, false);
let env_info = compiler.pop_compile_environment();
compiler.pop_compile_environment();
Expand Down
19 changes: 14 additions & 5 deletions boa_engine/src/bytecompiler/expression/assign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,13 @@ impl ByteCompiler<'_, '_> {
Access::Variable { name } => {
let binding = self.get_binding_value(name);
let index = self.get_or_insert_binding(binding);
self.emit(Opcode::GetName, &[index]);
let lex = self.current_environment.borrow().is_lex_binding(name);

if lex {
self.emit(Opcode::GetName, &[index]);
} else {
self.emit(Opcode::GetNameAndLocator, &[index]);
}

if short_circuit {
early_exit = Some(self.emit_opcode_with_operand(opcode));
Expand All @@ -68,10 +74,13 @@ impl ByteCompiler<'_, '_> {
if use_expr {
self.emit_opcode(Opcode::Dup);
}

let binding = self.set_mutable_binding(name);
let index = self.get_or_insert_binding(binding);
self.emit(Opcode::SetName, &[index]);
if lex {
let binding = self.set_mutable_binding(name);
let index = self.get_or_insert_binding(binding);
self.emit(Opcode::SetName, &[index]);
} else {
self.emit_opcode(Opcode::SetNameByLocator);
}
}
Access::Property { access } => match access {
PropertyAccess::Simple(access) => match access.field() {
Expand Down
18 changes: 14 additions & 4 deletions boa_engine/src/bytecompiler/expression/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ impl ByteCompiler<'_, '_> {
Access::Variable { name } => {
let binding = self.get_binding_value(name);
let index = self.get_or_insert_binding(binding);
self.emit(Opcode::GetName, &[index]);
let lex = self.current_environment.borrow().is_lex_binding(name);

if lex {
self.emit(Opcode::GetName, &[index]);
} else {
self.emit(Opcode::GetNameAndLocator, &[index]);
}

self.emit_opcode(opcode);
if post {
Expand All @@ -33,9 +39,13 @@ impl ByteCompiler<'_, '_> {
self.emit_opcode(Opcode::Dup);
}

let binding = self.set_mutable_binding(name);
let index = self.get_or_insert_binding(binding);
self.emit(Opcode::SetName, &[index]);
if lex {
let binding = self.set_mutable_binding(name);
let index = self.get_or_insert_binding(binding);
self.emit(Opcode::SetName, &[index]);
} else {
self.emit_opcode(Opcode::SetNameByLocator);
}
}
Access::Property { access } => match access {
PropertyAccess::Simple(access) => match access.field() {
Expand Down
1 change: 0 additions & 1 deletion boa_engine/src/bytecompiler/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ impl FunctionCompiler {
compiler.emit_opcode(Opcode::Yield);
}

compiler.create_script_decls(body, false);
compiler.compile_statement_list(body, false, false);

if let Some(env_label) = env_label {
Expand Down
24 changes: 19 additions & 5 deletions boa_engine/src/bytecompiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,13 +631,26 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
{
match access {
Access::Variable { name } => {
let binding = self.get_binding_value(name);
let index = self.get_or_insert_binding(binding);
let lex = self.current_environment.borrow().is_lex_binding(name);

if !lex {
self.emit(Opcode::GetLocator, &[index]);
}

expr_fn(self, 0);
if use_expr {
self.emit(Opcode::Dup, &[]);
}
let binding = self.set_mutable_binding(name);
let index = self.get_or_insert_binding(binding);
self.emit(Opcode::SetName, &[index]);

if lex {
let binding = self.set_mutable_binding(name);
let index = self.get_or_insert_binding(binding);
self.emit(Opcode::SetName, &[index]);
} else {
self.emit_opcode(Opcode::SetNameByLocator);
}
}
Access::Property { access } => match access {
PropertyAccess::Simple(access) => match access.field() {
Expand Down Expand Up @@ -736,6 +749,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
use_expr: bool,
configurable_globals: bool,
) {
self.create_declarations(list, configurable_globals);
if use_expr {
let expr_index = list
.statements()
Expand Down Expand Up @@ -770,7 +784,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
self.push_compile_environment(strict);
let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment);

self.create_script_decls(list, true);
self.create_declarations(list, true);

if use_expr {
let expr_index = list
Expand Down Expand Up @@ -1349,7 +1363,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
}

/// Creates the declarations for a script.
pub(crate) fn create_script_decls(
pub(crate) fn create_declarations(
&mut self,
stmt_list: &StatementList,
configurable_globals: bool,
Expand Down
1 change: 0 additions & 1 deletion boa_engine/src/bytecompiler/statement/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ impl ByteCompiler<'_, '_> {
self.push_compile_environment(false);
let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment);

self.create_script_decls(block.statement_list(), configurable_globals);
self.compile_statement_list(block.statement_list(), use_expr, configurable_globals);

let env_info = self.pop_compile_environment();
Expand Down
17 changes: 12 additions & 5 deletions boa_engine/src/bytecompiler/statement/switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,22 @@ use boa_ast::statement::Switch;
impl ByteCompiler<'_, '_> {
/// Compile a [`Switch`] `boa_ast` node
pub(crate) fn compile_switch(&mut self, switch: &Switch, configurable_globals: bool) {
self.compile_expr(switch.val(), true);

self.push_compile_environment(false);
let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment);
for case in switch.cases() {
self.create_script_decls(case.body(), configurable_globals);
self.create_declarations(case.body(), configurable_globals);
}
if let Some(body) = switch.default() {
self.create_declarations(body, configurable_globals);
}
let (start_label, end_label) = self.emit_opcode_with_two_operands(Opcode::LoopStart);

let start_address = self.next_opcode_location();
self.push_switch_control_info(None, start_address);
self.patch_jump_with_target(start_label, start_address);

self.compile_expr(switch.val(), true);
let mut labels = Vec::with_capacity(switch.cases().len());
for case in switch.cases() {
self.compile_expr(case.condition(), true);
Expand All @@ -26,13 +30,16 @@ impl ByteCompiler<'_, '_> {

for (label, case) in labels.into_iter().zip(switch.cases()) {
self.patch_jump(label);
self.compile_statement_list(case.body(), false, configurable_globals);
for item in case.body().statements() {
self.compile_stmt_list_item(item, false, configurable_globals);
}
}

self.patch_jump(exit);
if let Some(body) = switch.default() {
self.create_script_decls(body, configurable_globals);
self.compile_statement_list(body, false, configurable_globals);
for item in body.statements() {
self.compile_stmt_list_item(item, false, configurable_globals);
}
}

self.pop_switch_control_info();
Expand Down
3 changes: 0 additions & 3 deletions boa_engine/src/bytecompiler/statement/try.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ impl ByteCompiler<'_, '_> {
self.push_compile_environment(false);
let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment);

self.create_script_decls(t.block().statement_list(), configurable_globals);
self.compile_statement_list(t.block().statement_list(), use_expr, configurable_globals);

let env_info = self.pop_compile_environment();
Expand Down Expand Up @@ -89,7 +88,6 @@ impl ByteCompiler<'_, '_> {
self.emit_opcode(Opcode::Pop);
}

self.create_script_decls(catch.block().statement_list(), configurable_globals);
self.compile_statement_list(
catch.block().statement_list(),
use_expr,
Expand Down Expand Up @@ -119,7 +117,6 @@ impl ByteCompiler<'_, '_> {
self.push_compile_environment(false);
let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment);

self.create_script_decls(finally.block().statement_list(), configurable_globals);
self.compile_statement_list(
finally.block().statement_list(),
false,
Expand Down
1 change: 0 additions & 1 deletion boa_engine/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,6 @@ impl<'host> Context<'host> {
self.realm.environment().compile_env(),
self,
);
compiler.create_script_decls(statement_list, false);
compiler.compile_statement_list(statement_list, true, false);
Ok(Gc::new(compiler.finish()))
}
Expand Down
11 changes: 11 additions & 0 deletions boa_engine/src/environments/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,17 @@ impl CompileTimeEnvironment {
.map_or(false, |binding| binding.lex)
}

/// Checks if `name` is a lexical binding.
pub(crate) fn is_lex_binding(&self, name: Identifier) -> bool {
if let Some(binding) = self.bindings.get(&name) {
binding.lex
} else if let Some(outer) = &self.outer {
outer.borrow().is_lex_binding(name)
} else {
false
}
}

/// Returns the number of bindings in this environment.
pub(crate) fn num_bindings(&self) -> usize {
self.bindings.len()
Expand Down
20 changes: 15 additions & 5 deletions boa_engine/src/environments/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{
JsResult, JsString, JsSymbol, JsValue,
};
use boa_ast::expression::Identifier;
use boa_gc::{Finalize, Gc, GcRefCell, Trace};
use boa_gc::{empty_trace, Finalize, Gc, GcRefCell, Trace};
use rustc_hash::FxHashSet;
use std::cell::Cell;

Expand Down Expand Up @@ -663,7 +663,7 @@ impl DeclarativeEnvironmentStack {
/// A binding locator contains all information about a binding that is needed to resolve it at runtime.
///
/// Binding locators get created at bytecode compile time and are accessible at runtime via the [`crate::vm::CodeBlock`].
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, Finalize)]
pub(crate) struct BindingLocator {
name: Identifier,
environment_index: usize,
Expand All @@ -673,6 +673,10 @@ pub(crate) struct BindingLocator {
silent: bool,
}

unsafe impl Trace for BindingLocator {
empty_trace!();
}

impl BindingLocator {
/// Creates a new declarative binding locator that has knows indices.
pub(crate) const fn declarative(
Expand All @@ -691,7 +695,7 @@ impl BindingLocator {
}

/// Creates a binding locator that indicates that the binding is on the global object.
pub(in crate::environments) const fn global(name: Identifier) -> Self {
pub(super) const fn global(name: Identifier) -> Self {
Self {
name,
environment_index: 0,
Expand Down Expand Up @@ -844,7 +848,13 @@ impl Context<'_> {
Environment::Declarative(env) => {
Ok(env.bindings.borrow()[locator.binding_index].is_some())
}
Environment::Object(_) => Ok(true),
Environment::Object(obj) => {
let key: JsString = self
.interner()
.resolve_expect(locator.name.sym())
.into_common(false);
obj.clone().has_property(key, self)
}
}
}
}
Expand Down Expand Up @@ -953,7 +963,7 @@ impl Context<'_> {
}

/// Return the environment at the given index. Panics if the index is out of range.
fn environment_expect(&self, index: usize) -> &Environment {
pub(crate) fn environment_expect(&self, index: usize) -> &Environment {
self.vm
.environments
.stack
Expand Down
9 changes: 8 additions & 1 deletion boa_engine/src/vm/call_frame/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
mod abrupt_record;
mod env_stack;

use crate::{builtins::promise::PromiseCapability, object::JsObject, vm::CodeBlock};
use crate::{
builtins::promise::PromiseCapability, environments::BindingLocator, object::JsObject,
vm::CodeBlock,
};
use boa_gc::{Finalize, Gc, Trace};
use thin_vec::ThinVec;

Expand Down Expand Up @@ -39,6 +42,9 @@ pub struct CallFrame {

// Iterators and their `[[Done]]` flags that must be closed when an abrupt completion is thrown.
pub(crate) iterators: ThinVec<(JsObject, bool)>,

// The stack of bindings being updated.
pub(crate) binding_stack: Vec<BindingLocator>,
}

/// ---- `CallFrame` public API ----
Expand Down Expand Up @@ -69,6 +75,7 @@ impl CallFrame {
promise_capability: None,
async_generator: None,
iterators: ThinVec::new(),
binding_stack: Vec::new(),
}
}

Expand Down
3 changes: 3 additions & 0 deletions boa_engine/src/vm/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,8 @@ impl CodeBlock {
| Opcode::DefInitLet
| Opcode::DefInitConst
| Opcode::GetName
| Opcode::GetLocator
| Opcode::GetNameAndLocator
| Opcode::GetNameOrUndefined
| Opcode::SetName
| Opcode::DeleteName => {
Expand Down Expand Up @@ -495,6 +497,7 @@ impl CodeBlock {
| Opcode::SetPrototype
| Opcode::PushObjectEnvironment
| Opcode::IsObject
| Opcode::SetNameByLocator
| Opcode::Nop => String::new(),
}
}
Expand Down
3 changes: 3 additions & 0 deletions boa_engine/src/vm/flowgraph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,8 @@ impl CodeBlock {
| Opcode::DefInitLet
| Opcode::DefInitConst
| Opcode::GetName
| Opcode::GetLocator
| Opcode::GetNameAndLocator
| Opcode::GetNameOrUndefined
| Opcode::SetName
| Opcode::DeleteName => {
Expand Down Expand Up @@ -566,6 +568,7 @@ impl CodeBlock {
| Opcode::SuperCallPrepare
| Opcode::SetPrototype
| Opcode::IsObject
| Opcode::SetNameByLocator
| Opcode::Nop
| Opcode::PushObjectEnvironment => {
graph.add_node(previous_pc, NodeShape::None, label.into(), Color::None);
Expand Down
1 change: 1 addition & 0 deletions boa_engine/src/vm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ pub(crate) use {

#[cfg(test)]
mod tests;

/// Virtual Machine.
#[derive(Debug)]
pub struct Vm {
Expand Down