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

Cache this value #3771

Merged
merged 1 commit into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion core/engine/src/builtins/function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1054,11 +1054,17 @@ fn function_construct(
Some(this)
};

let frame = CallFrame::new(code.clone(), script_or_module, environments, realm)
let mut frame = CallFrame::new(code.clone(), script_or_module, environments, realm)
.with_argument_count(argument_count as u32)
.with_env_fp(env_fp)
.with_flags(CallFrameFlags::CONSTRUCT);

// We push the `this` below so we can mark this function as having the this value
// cached if it's initialized.
frame
.flags
.set(CallFrameFlags::THIS_VALUE_CACHED, this.is_some());

let len = context.vm.stack.len();

context.vm.push_frame(frame);
Expand Down
9 changes: 9 additions & 0 deletions core/engine/src/vm/call_frame/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ bitflags::bitflags! {

/// Does this [`CallFrame`] need to push registers on [`Vm::push_frame()`].
const REGISTERS_ALREADY_PUSHED = 0b0000_0100;

/// If the `this` value has been cached.
const THIS_VALUE_CACHED = 0b0000_1000;
}
}

Expand Down Expand Up @@ -323,6 +326,12 @@ impl CallFrame {
self.flags
.contains(CallFrameFlags::REGISTERS_ALREADY_PUSHED)
}
/// Does this [`CallFrame`] have a cached `this` value.
///
/// The cached value is placed in the [`CallFrame::THIS_POSITION`] position.
pub(crate) fn has_this_value_cached(&self) -> bool {
self.flags.contains(CallFrameFlags::THIS_VALUE_CACHED)
}
}

/// ---- `CallFrame` stack methods ----
Expand Down
21 changes: 13 additions & 8 deletions core/engine/src/vm/opcode/control_flow/return.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,21 @@ impl Operation for CheckReturn {
);
return Ok(CompletionType::Throw);
} else {
let realm = context.vm.frame().realm.clone();
let this = context.vm.environments.get_this_binding();
let frame = context.vm.frame();
if frame.has_this_value_cached() {
this
} else {
let realm = frame.realm.clone();
let this = context.vm.environments.get_this_binding();

match this {
Err(err) => {
let err = err.inject_realm(realm);
context.vm.pending_exception = Some(err);
return Ok(CompletionType::Throw);
match this {
Err(err) => {
let err = err.inject_realm(realm);
context.vm.pending_exception = Some(err);
return Ok(CompletionType::Throw);
}
Ok(this) => this,
}
Ok(this) => this,
}
};

Expand Down
12 changes: 11 additions & 1 deletion core/engine/src/vm/opcode/environment/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::{
builtins::function::OrdinaryFunction,
error::JsNativeError,
object::internal_methods::InternalMethodContext,
vm::{opcode::Operation, CompletionType},
vm::{opcode::Operation, CallFrameFlags, CompletionType},
Context, JsResult, JsValue,
};

Expand All @@ -19,7 +19,17 @@ impl Operation for This {
const COST: u8 = 1;

fn execute(context: &mut Context) -> JsResult<CompletionType> {
let frame = context.vm.frame_mut();
let this_index = frame.fp();
if frame.has_this_value_cached() {
let this = context.vm.stack[this_index as usize].clone();
context.vm.push(this);
return Ok(CompletionType::Normal);
}

let this = context.vm.environments.get_this_binding()?;
context.vm.frame_mut().flags |= CallFrameFlags::THIS_VALUE_CACHED;
context.vm.stack[this_index as usize] = this.clone();
context.vm.push(this);
Ok(CompletionType::Normal)
}
Expand Down
Loading