github
/
boa
mirror of https://github.com/boa-dev/boa.git
1
0
Fork 0
Code Issues Releases Wiki Activity
Browse Source

Refactor mapped `Arguments` object (#1849) 

This refactors the representation of the `[[ParameterMap]]` internal slot on the `Arguments` exotic object to be faster at runtime.

Previously `[[ParameterMap]]` was a `JsObject` like the spec describes. This can be pretty slow a runtime, because the argument getters and setters must be represented as function objects on the `[[ParameterMap]]` object. In addition to the time spend on creation and calling of those functions, every getter/setter needs a cloned gc reference to the function environment to access the bindings. This adds to the gc overhead.

The spec states that the `[[ParameterMap]]` internal slot doesn't have to be a `JsObject`. See NOTE 3 here: https://tc39.es/ecma262/#sec-arguments-exotic-objects
Leveraging this freedom, we can use a more optimized representation, that avoids any `JsObject` usage and only needs one clone of the function environment.
pull/1855/head
raskad 3 years ago
parent
46f96d4bea
commit
1d6e14763b
3 changed files with 191 additions and 185 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 173
      boa/src/builtins/function/arguments.rs
  2. 187
      boa/src/object/internal_methods/arguments.rs
  3. 16
      boa/src/object/mod.rs

173
boa/src/builtins/function/arguments.rs
Unescape View File

@ -2,8 +2,8 @@ use crate::{
builtins::Array, builtins::Array,
environments::DeclarativeEnvironment, environments::DeclarativeEnvironment,
gc::{Finalize, Trace}, gc::{Finalize, Trace},
object::{FunctionBuilder, JsObject, ObjectData}, object::{JsObject, ObjectData},
property::{PropertyDescriptor, PropertyKey}, property::PropertyDescriptor,
symbol::{self, WellKnownSymbols}, symbol::{self, WellKnownSymbols},
syntax::ast::node::FormalParameter, syntax::ast::node::FormalParameter,
Context, JsValue, Context, JsValue,
@ -11,19 +11,57 @@ use crate::{
use gc::Gc; use gc::Gc;
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
/// `ParameterMap` represents the `[[ParameterMap]]` internal slot on a Arguments exotic object.
///
/// This struct stores all the data to access mapped function parameters in their environment.
#[derive(Debug, Clone, Trace, Finalize)] #[derive(Debug, Clone, Trace, Finalize)]
pub struct MappedArguments(JsObject); pub struct ParameterMap {
binding_indices: Vec<Option<usize>>,
environment: Gc<DeclarativeEnvironment>,
}
impl MappedArguments { impl ParameterMap {
pub(crate) fn parameter_map(&self) -> JsObject { /// Deletes the binding with the given index from the parameter map.
self.0.clone() pub(crate) fn delete(&mut self, index: usize) {
if let Some(binding) = self.binding_indices.get_mut(index) {
*binding = None;
}
}
/// Get the value of the binding at the given index from the function environment.
///
/// Note: This function is the abstract getter closure described in 10.4.4.7.1 `MakeArgGetter ( name, env )`
///
/// More information:
/// - [ECMAScript reference][spec]
///
/// [spec]: https://tc39.es/ecma262/#sec-makearggetter
pub(crate) fn get(&self, index: usize) -> Option<JsValue> {
if let Some(Some(binding_index)) = self.binding_indices.get(index) {
return Some(self.environment.get(*binding_index));
}
None
}
/// Set the value of the binding at the given index in the function environment.
///
/// Note: This function is the abstract setter closure described in 10.4.4.7.2 `MakeArgSetter ( name, env )`
///
/// More information:
/// - [ECMAScript reference][spec]
///
/// [spec]: https://tc39.es/ecma262/#sec-makeargsetter
pub(crate) fn set(&self, index: usize, value: &JsValue) {
if let Some(Some(binding_index)) = self.binding_indices.get(index) {
self.environment.set(*binding_index, value.clone());
}
} }
} }
#[derive(Debug, Clone, Trace, Finalize)] #[derive(Debug, Clone, Trace, Finalize)]
pub enum Arguments { pub enum Arguments {
Unmapped, Unmapped,
Mapped(MappedArguments), Mapped(ParameterMap),
} }
impl Arguments { impl Arguments {
@ -128,38 +166,8 @@ impl Arguments {
// 8. Set obj.[[Delete]] as specified in 10.4.4.5. // 8. Set obj.[[Delete]] as specified in 10.4.4.5.
// 9. Set obj.[[Prototype]] to %Object.prototype%. // 9. Set obj.[[Prototype]] to %Object.prototype%.
// 10. Let map be ! OrdinaryObjectCreate(null). // Section 17-19 are done first, for easier object creation in 11.
let map = JsObject::empty(); //
// 11. Set obj.[[ParameterMap]] to map.
let obj = JsObject::from_proto_and_data(
context.standard_objects().object_object().prototype(),
ObjectData::arguments(Self::Mapped(MappedArguments(map.clone()))),
);
// 14. Let index be 0.
// 15. Repeat, while index < len,
for (index, val) in arguments_list.iter().cloned().enumerate() {
// a. Let val be argumentsList[index].
// b. Perform ! CreateDataPropertyOrThrow(obj, ! ToString(𝔽(index)), val).
obj.create_data_property_or_throw(index, val, context)
.expect("Defining new own properties for a new ordinary object cannot fail");
// c. Set index to index + 1.
}
// 16. Perform ! DefinePropertyOrThrow(obj, "length", PropertyDescriptor { [[Value]]: 𝔽(len),
// [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true }).
obj.define_property_or_throw(
"length",
PropertyDescriptor::builder()
.value(len)
.writable(true)
.enumerable(false)
.configurable(true),
context,
)
.expect("Defining new own properties for a new ordinary object cannot fail");
// The section 17-19 differs from the spec, due to the way the runtime environments work. // The section 17-19 differs from the spec, due to the way the runtime environments work.
// //
// This section creates getters and setters for all mapped arguments. // This section creates getters and setters for all mapped arguments.
@ -205,59 +213,54 @@ impl Arguments {
property_index += 1; property_index += 1;
} }
} }
let mut map = ParameterMap {
binding_indices: vec![None; property_index],
environment: env.clone(),
};
for (binding_index, property_index) in bindings.values() { for (binding_index, property_index) in bindings.values() {
// 19.b.ii.1. Let g be MakeArgGetter(name, env). map.binding_indices[*property_index] = Some(*binding_index);
// https://tc39.es/ecma262/#sec-makearggetter }
let g = {
// 2. Let getter be ! CreateBuiltinFunction(getterClosure, 0, "", « »).
// 3. NOTE: getter is never directly accessible to ECMAScript code.
// 4. Return getter.
FunctionBuilder::closure_with_captures(
context,
// 1. Let getterClosure be a new Abstract Closure with no parameters that captures
// name and env and performs the following steps when called:
|_, _, captures, _| Ok(captures.0.get(captures.1)),
(env.clone(), *binding_index),
)
.length(0)
.build()
};
// 19.b.ii.2. Let p be MakeArgSetter(name, env).
// https://tc39.es/ecma262/#sec-makeargsetter
let p = {
// 2. Let setter be ! CreateBuiltinFunction(setterClosure, 1, "", « »).
// 3. NOTE: setter is never directly accessible to ECMAScript code.
// 4. Return setter.
FunctionBuilder::closure_with_captures(
context,
// 1. Let setterClosure be a new Abstract Closure with parameters (value) that captures
// name and env and performs the following steps when called:
|_, args, captures, _| {
let value = args.get(0).cloned().unwrap_or_default();
captures.0.set(captures.1, value);
Ok(JsValue::undefined())
},
(env.clone(), *binding_index),
)
.length(1)
.build()
};
// 19.b.ii.3. Perform map.[[DefineOwnProperty]](! ToString(𝔽(index)), PropertyDescriptor { // 11. Set obj.[[ParameterMap]] to map.
// [[Set]]: p, [[Get]]: g, [[Enumerable]]: false, [[Configurable]]: true }). let obj = JsObject::from_proto_and_data(
map.__define_own_property__( context.standard_objects().object_object().prototype(),
PropertyKey::from(*property_index), ObjectData::arguments(Self::Mapped(map)),
);
// 14. Let index be 0.
// 15. Repeat, while index < len,
for (index, val) in arguments_list.iter().cloned().enumerate() {
// a. Let val be argumentsList[index].
// b. Perform ! CreateDataPropertyOrThrow(obj, ! ToString(𝔽(index)), val).
// Note: Insert is used here because `CreateDataPropertyOrThrow` would cause a panic while executing
// exotic argument object set methods before the variables in the environment are initialized.
obj.insert(
index,
PropertyDescriptor::builder() PropertyDescriptor::builder()
.set(p) .value(val)
.get(g) .writable(true)
.enumerable(false) .enumerable(true)
.configurable(true) .configurable(true)
.build(), .build(),
context, );
) // c. Set index to index + 1.
.expect("Defining new own properties for a new ordinary object cannot fail");
} }
// 16. Perform ! DefinePropertyOrThrow(obj, "length", PropertyDescriptor { [[Value]]: 𝔽(len),
// [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true }).
obj.define_property_or_throw(
"length",
PropertyDescriptor::builder()
.value(len)
.writable(true)
.enumerable(false)
.configurable(true),
context,
)
.expect("Defining new own properties for a new ordinary object cannot fail");
// 20. Perform ! DefinePropertyOrThrow(obj, @@iterator, PropertyDescriptor { // 20. Perform ! DefinePropertyOrThrow(obj, @@iterator, PropertyDescriptor {
// [[Value]]: %Array.prototype.values%, [[Writable]]: true, [[Enumerable]]: false, // [[Value]]: %Array.prototype.values%, [[Writable]]: true, [[Enumerable]]: false,
// [[Configurable]]: true }). // [[Configurable]]: true }).

187
boa/src/object/internal_methods/arguments.rs
Unescape View File

@ -37,31 +37,29 @@ pub(crate) fn arguments_exotic_get_own_property(
}; };
// 3. Let map be args.[[ParameterMap]]. // 3. Let map be args.[[ParameterMap]].
let map = obj // 4. Let isMapped be ! HasOwnProperty(map, P).
.borrow() // 5. If isMapped is true, then
.as_mapped_arguments() if let PropertyKey::Index(index) = key {
.expect("arguments exotic method must only be callable from arguments objects") if let Some(value) = obj
.parameter_map(); .borrow()
.as_mapped_arguments()
Ok(Some( .expect("arguments exotic method must only be callable from arguments objects")
// 4. Let isMapped be ! HasOwnProperty(map, P). .get(*index as usize)
// 5. If isMapped is true, then
if map
.has_own_property(key.clone(), context)
.expect("HasOwnProperty must not fail per the spec")
{ {
// a. Set desc.[[Value]] to Get(map, P). // a. Set desc.[[Value]] to Get(map, P).
PropertyDescriptor::builder() return Ok(Some(
.value(map.get(key.clone(), context)?) PropertyDescriptor::builder()
.maybe_writable(desc.writable()) .value(value)
.maybe_enumerable(desc.enumerable()) .maybe_writable(desc.writable())
.maybe_configurable(desc.configurable()) .maybe_enumerable(desc.enumerable())
.build() .maybe_configurable(desc.configurable())
} else { .build(),
// 6. Return desc. ));
desc }
}, }
))
// 6. Return desc.
Ok(Some(desc))
} }
/// `[[DefineOwnProperty]]` for arguments exotic objects. /// `[[DefineOwnProperty]]` for arguments exotic objects.
@ -77,15 +75,17 @@ pub(crate) fn arguments_exotic_define_own_property(
desc: PropertyDescriptor, desc: PropertyDescriptor,
context: &mut Context, context: &mut Context,
) -> JsResult<bool> { ) -> JsResult<bool> {
// 1. Let map be args.[[ParameterMap]].
let map = obj
.borrow()
.as_mapped_arguments()
.expect("arguments exotic method must only be callable from arguments objects")
.parameter_map();
// 2. Let isMapped be HasOwnProperty(map, P). // 2. Let isMapped be HasOwnProperty(map, P).
let is_mapped = map.has_own_property(key.clone(), context)?; let mapped = if let PropertyKey::Index(index) = key {
// 1. Let map be args.[[ParameterMap]].
obj.borrow()
.as_mapped_arguments()
.expect("arguments exotic method must only be callable from arguments objects")
.get(index as usize)
.map(|value| (index as usize, value))
} else {
None
};
let new_arg_desc = match desc.kind() { let new_arg_desc = match desc.kind() {
// 4. If isMapped is true and IsDataDescriptor(Desc) is true, then // 4. If isMapped is true and IsDataDescriptor(Desc) is true, then
@ -94,16 +94,20 @@ pub(crate) fn arguments_exotic_define_own_property(
DescriptorKind::Data { DescriptorKind::Data {
writable: Some(false), writable: Some(false),
value: None, value: None,
} if is_mapped => } =>
// i. Set newArgDesc to a copy of Desc. // i. Set newArgDesc to a copy of Desc.
// ii. Set newArgDesc.[[Value]] to Get(map, P). // ii. Set newArgDesc.[[Value]] to Get(map, P).
{ {
PropertyDescriptor::builder() if let Some((_, value)) = &mapped {
.value(map.get(key.clone(), context)?) PropertyDescriptor::builder()
.writable(false) .value(value)
.maybe_enumerable(desc.enumerable()) .writable(false)
.maybe_configurable(desc.configurable()) .maybe_enumerable(desc.enumerable())
.build() .maybe_configurable(desc.configurable())
.build()
} else {
desc.clone()
}
} }
// 3. Let newArgDesc be Desc. // 3. Let newArgDesc be Desc.
@ -112,32 +116,36 @@ pub(crate) fn arguments_exotic_define_own_property(
// 5. Let allowed be ? OrdinaryDefineOwnProperty(args, P, newArgDesc). // 5. Let allowed be ? OrdinaryDefineOwnProperty(args, P, newArgDesc).
// 6. If allowed is false, return false. // 6. If allowed is false, return false.
if !super::ordinary_define_own_property(obj, key.clone(), new_arg_desc, context)? { if !super::ordinary_define_own_property(obj, key, new_arg_desc, context)? {
return Ok(false); return Ok(false);
} }
// 7. If isMapped is true, then // 7. If isMapped is true, then
if is_mapped { if let Some((index, _)) = mapped {
// 1. Let map be args.[[ParameterMap]].
let mut obj_mut = obj.borrow_mut();
let map = obj_mut
.as_mapped_arguments_mut()
.expect("arguments exotic method must only be callable from arguments objects");
// a. If IsAccessorDescriptor(Desc) is true, then // a. If IsAccessorDescriptor(Desc) is true, then
if desc.is_accessor_descriptor() { if desc.is_accessor_descriptor() {
// i. Call map.[[Delete]](P). // i. Call map.[[Delete]](P).
map.__delete__(&key, context)?; map.delete(index);
} }
// b. Else, // b. Else,
else { else {
// i. If Desc.[[Value]] is present, then // i. If Desc.[[Value]] is present, then
if let Some(value) = desc.value() { if let Some(value) = desc.value() {
// 1. Let setStatus be Set(map, P, Desc.[[Value]], false). // 1. Let setStatus be Set(map, P, Desc.[[Value]], false).
let set_status = map.set(key.clone(), value, false, context);
// 2. Assert: setStatus is true because formal parameters mapped by argument objects are always writable. // 2. Assert: setStatus is true because formal parameters mapped by argument objects are always writable.
assert_eq!(set_status, Ok(true)); map.set(index, value);
} }
// ii. If Desc.[[Writable]] is present and its value is false, then // ii. If Desc.[[Writable]] is present and its value is false, then
if let Some(false) = desc.writable() { if let Some(false) = desc.writable() {
// 1. Call map.[[Delete]](P). // 1. Call map.[[Delete]](P).
map.__delete__(&key, context)?; map.delete(index);
} }
} }
} }
@ -158,28 +166,24 @@ pub(crate) fn arguments_exotic_get(
receiver: JsValue, receiver: JsValue,
context: &mut Context, context: &mut Context,
) -> JsResult<JsValue> { ) -> JsResult<JsValue> {
// 1. Let map be args.[[ParameterMap]]. if let PropertyKey::Index(index) = key {
let map = obj // 1. Let map be args.[[ParameterMap]].
.borrow() // 2. Let isMapped be ! HasOwnProperty(map, P).
.as_mapped_arguments() if let Some(value) = obj
.expect("arguments exotic method must only be callable from arguments objects") .borrow()
.parameter_map(); .as_mapped_arguments()
.expect("arguments exotic method must only be callable from arguments objects")
// 2. Let isMapped be ! HasOwnProperty(map, P). .get(*index as usize)
// 4. Else, {
if map // a. Assert: map contains a formal parameter mapping for P.
.has_own_property(key.clone(), context) // b. Return Get(map, P).
.expect("HasOwnProperty must not fail per the spec") return Ok(value);
{ }
// a. Assert: map contains a formal parameter mapping for P. }
// b. Return Get(map, P).
map.get(key.clone(), context)
// 3. If isMapped is false, then // 3. If isMapped is false, then
} else { // a. Return ? OrdinaryGet(args, P, Receiver).
// a. Return ? OrdinaryGet(args, P, Receiver). super::ordinary_get(obj, key, receiver, context)
super::ordinary_get(obj, key, receiver, context)
}
} }
/// `[[Set]]` for arguments exotic objects. /// `[[Set]]` for arguments exotic objects.
@ -198,25 +202,17 @@ pub(crate) fn arguments_exotic_set(
// 1. If SameValue(args, Receiver) is false, then // 1. If SameValue(args, Receiver) is false, then
// a. Let isMapped be false. // a. Let isMapped be false.
// 2. Else, // 2. Else,
if JsValue::same_value(&obj.clone().into(), &receiver) { if let PropertyKey::Index(index) = key {
// a. Let map be args.[[ParameterMap]]. if JsValue::same_value(&obj.clone().into(), &receiver) {
let map = obj // a. Let map be args.[[ParameterMap]].
.borrow() // b. Let isMapped be ! HasOwnProperty(map, P).
.as_mapped_arguments() // 3. If isMapped is true, then
.expect("arguments exotic method must only be callable from arguments objects")
.parameter_map();
// b. Let isMapped be ! HasOwnProperty(map, P).
// 3. If isMapped is true, then
if map
.has_own_property(key.clone(), context)
.expect("HasOwnProperty must not fail per the spec")
{
// a. Let setStatus be Set(map, P, V, false). // a. Let setStatus be Set(map, P, V, false).
let set_status = map.set(key.clone(), value.clone(), false, context);
// b. Assert: setStatus is true because formal parameters mapped by argument objects are always writable. // b. Assert: setStatus is true because formal parameters mapped by argument objects are always writable.
assert_eq!(set_status, Ok(true)); obj.borrow_mut()
.as_mapped_arguments_mut()
.expect("arguments exotic method must only be callable from arguments objects")
.set(index as usize, &value);
} }
} }
@ -235,25 +231,20 @@ pub(crate) fn arguments_exotic_delete(
key: &PropertyKey, key: &PropertyKey,
context: &mut Context, context: &mut Context,
) -> JsResult<bool> { ) -> JsResult<bool> {
// 1. Let map be args.[[ParameterMap]].
let map = obj
.borrow()
.as_mapped_arguments()
.expect("arguments exotic method must only be callable from arguments objects")
.parameter_map();
// 2. Let isMapped be ! HasOwnProperty(map, P).
let is_mapped = map
.has_own_property(key.clone(), context)
.expect("HasOwnProperty must not fail per the spec");
// 3. Let result be ? OrdinaryDelete(args, P). // 3. Let result be ? OrdinaryDelete(args, P).
let result = super::ordinary_delete(obj, key, context)?; let result = super::ordinary_delete(obj, key, context)?;
// 4. If result is true and isMapped is true, then if result {
if is_mapped && result { if let PropertyKey::Index(index) = key {
// a. Call map.[[Delete]](P). // 1. Let map be args.[[ParameterMap]].
map.__delete__(key, context)?; // 2. Let isMapped be ! HasOwnProperty(map, P).
// 4. If result is true and isMapped is true, then
// a. Call map.[[Delete]](P).
obj.borrow_mut()
.as_mapped_arguments_mut()
.expect("arguments exotic method must only be callable from arguments objects")
.delete(*index as usize);
}
} }
// 5. Return result. // 5. Return result.

16
boa/src/object/mod.rs
Unescape View File

@ -4,7 +4,7 @@ use crate::{
builtins::{ builtins::{
array::array_iterator::ArrayIterator, array::array_iterator::ArrayIterator,
array_buffer::ArrayBuffer, array_buffer::ArrayBuffer,
function::arguments::{Arguments, MappedArguments}, function::arguments::{Arguments, ParameterMap},
function::{BoundFunction, Captures, Function, NativeFunctionSignature}, function::{BoundFunction, Captures, Function, NativeFunctionSignature},
map::map_iterator::MapIterator, map::map_iterator::MapIterator,
map::ordered_map::OrderedMap, map::ordered_map::OrderedMap,
@ -958,7 +958,7 @@ impl Object {
/// Gets the mapped arguments data if this is a mapped arguments object. /// Gets the mapped arguments data if this is a mapped arguments object.
#[inline] #[inline]
pub fn as_mapped_arguments(&self) -> Option<&MappedArguments> { pub fn as_mapped_arguments(&self) -> Option<&ParameterMap> {
match self.data { match self.data {
ObjectData { ObjectData {
kind: ObjectKind::Arguments(Arguments::Mapped(ref args)), kind: ObjectKind::Arguments(Arguments::Mapped(ref args)),
@ -968,6 +968,18 @@ impl Object {
} }
} }
/// Gets the mutable mapped arguments data if this is a mapped arguments object.
#[inline]
pub fn as_mapped_arguments_mut(&mut self) -> Option<&mut ParameterMap> {
match self.data {
ObjectData {
kind: ObjectKind::Arguments(Arguments::Mapped(ref mut args)),
..
} => Some(args),
_ => None,
}
}
/// Gets the typed array data (integer indexed object) if this is a typed array. /// Gets the typed array data (integer indexed object) if this is a typed array.
#[inline] #[inline]
pub fn as_typed_array(&self) -> Option<&IntegerIndexed> { pub fn as_typed_array(&self) -> Option<&IntegerIndexed> {

Write Preview
Loading…
Cancel
Save