Skip to content

Commit

Permalink
Move property desciptor flags to shape
Browse files Browse the repository at this point in the history
  • Loading branch information
HalidOdat committed Mar 24, 2023
1 parent 335906c commit 649150e
Show file tree
Hide file tree
Showing 4 changed files with 233 additions and 60 deletions.
151 changes: 125 additions & 26 deletions boa_engine/src/object/property_map.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::{
shape::{Shape, Slot, TransitionKey},
shape::{PropertyDescriptorAttribute, Shape, Slot, TransitionKey},
PropertyDescriptor, PropertyKey,
};
use crate::{property::PropertyDescriptorBuilder, JsString, JsSymbol, JsValue};
Expand Down Expand Up @@ -229,11 +229,10 @@ pub struct PropertyMap {
indexed_properties: IndexedProperties,

pub(crate) shape: Shape,
storage: Vec<PropertyDescriptor>,

// TODO: remove these, since they will be replaced by the shape
string_properties: OrderedHashMap<JsString>,
symbol_properties: OrderedHashMap<JsSymbol>,
// FIXME: currently every property takes 2 spaces in the array
// to account for the accessor (get, set), add different sized
// storage based on attribute flags.
storage: Vec<JsValue>,
}

impl PropertyMap {
Expand All @@ -243,8 +242,6 @@ impl PropertyMap {
pub fn new(shape: Shape) -> Self {
Self {
indexed_properties: IndexedProperties::default(),
string_properties: OrderedHashMap::default(),
symbol_properties: OrderedHashMap::default(),
shape,
storage: Vec::default(),
}
Expand All @@ -257,19 +254,38 @@ impl PropertyMap {
return self.indexed_properties.get(*index);
}
if let Some(slot) = self.shape.lookup(key) {
return Some(self.storage[slot.index as usize].clone());
return Some(self.get_storage(slot));
}

None
}

/// Get the property with the given key from the [`PropertyMap`].
#[must_use]
pub fn get_storage(&self, slot: Slot) -> PropertyDescriptor {
self.storage[slot.index as usize].clone()
pub fn get_storage(&self, Slot { index, attributes }: Slot) -> PropertyDescriptor {
let index = index as usize;
let mut builder = PropertyDescriptor::builder()
.configurable(attributes.contains(PropertyDescriptorAttribute::CONFIGURABLE))
.enumerable(attributes.contains(PropertyDescriptorAttribute::ENUMERABLE));
if attributes.contains(PropertyDescriptorAttribute::HAS_GET)
|| attributes.contains(PropertyDescriptorAttribute::HAS_SET)
{
if attributes.contains(PropertyDescriptorAttribute::HAS_GET) {
builder = builder.get(self.storage[index].clone());
}
if attributes.contains(PropertyDescriptorAttribute::HAS_SET) {
builder = builder.set(self.storage[index + 1].clone());
}
} else {
builder = builder.writable(attributes.contains(PropertyDescriptorAttribute::WRITABLE));
builder = builder.value(self.storage[index].clone());
}
builder.build()
}

/// Insert the given property descriptor with the given key [`PropertyMap`].
// FIXME: Temporary lint allow, remove after prototyping
#[allow(clippy::missing_panics_doc)]
pub fn insert(
&mut self,
key: &PropertyKey,
Expand All @@ -278,19 +294,103 @@ impl PropertyMap {
if let PropertyKey::Index(index) = key {
return self.indexed_properties.insert(*index, property);
}

// TODO: maybe extract into a PropertyDescriptor method?
let mut attributes = PropertyDescriptorAttribute::empty();
attributes.set(
PropertyDescriptorAttribute::CONFIGURABLE,
property.expect_configurable(),
);
attributes.set(
PropertyDescriptorAttribute::ENUMERABLE,
property.expect_enumerable(),
);
if property.is_data_descriptor() {
attributes.set(
PropertyDescriptorAttribute::WRITABLE,
property.expect_writable(),
);
} else if property.is_accessor_descriptor() {
attributes.set(
PropertyDescriptorAttribute::HAS_GET,
property.get().is_some(),
);
attributes.set(
PropertyDescriptorAttribute::HAS_SET,
property.set().is_some(),
);
} else {
unreachable!()
}

// println!("Attributes: {key} IN {attributes:?}");

if let Some(slot) = self.shape.lookup(key) {
return Some(std::mem::replace(
&mut self.storage[slot.index as usize],
property,
));
let index = slot.index as usize;

// TODO: figure out if the return value of self.insert is ever used,
// if not remove it, seems unneded.
let old = self.get_storage(slot);
if slot.attributes != attributes {
let key = TransitionKey {
property_key: key.clone(),
attributes,
};
self.shape = self.shape.change_attributes_transition(key);
}

// TODO: maybe implement is_accessor/data_descriptor on attributes?
if attributes.contains(PropertyDescriptorAttribute::HAS_GET)
|| attributes.contains(PropertyDescriptorAttribute::HAS_SET)
{
if attributes.contains(PropertyDescriptorAttribute::HAS_GET) {
self.storage[index] = property
.get()
.cloned()
.map(JsValue::new)
.unwrap_or_default();
}
if attributes.contains(PropertyDescriptorAttribute::HAS_SET) {
self.storage[index + 1] = property
.set()
.cloned()
.map(JsValue::new)
.unwrap_or_default();
}
} else {
// TODO: maybe take the value instead of cloning, we own it.
self.storage[index] = property.expect_value().clone();
}
return Some(old);
}

let transition_key = TransitionKey {
property_key: key.clone(),
// attributes: Attribute::all(),
attributes,
};
self.shape = self.shape.insert_property_transition(transition_key);
self.storage.push(property);
if attributes.contains(PropertyDescriptorAttribute::HAS_GET)
|| attributes.contains(PropertyDescriptorAttribute::HAS_SET)
{
self.storage.push(
property
.get()
.cloned()
.map(JsValue::new)
.unwrap_or_default(),
);
self.storage.push(
property
.set()
.cloned()
.map(JsValue::new)
.unwrap_or_default(),
);
} else {
self.storage
.push(property.value().cloned().unwrap_or_default());
self.storage.push(JsValue::undefined());
}

None
}
Expand All @@ -301,15 +401,14 @@ impl PropertyMap {
return self.indexed_properties.remove(*index);
}
if let Some(slot) = self.shape.lookup(key) {
let result = self.storage.remove(slot.index as usize);

let transition_key = TransitionKey {
property_key: key.clone(),
// attributes: Attribute::all(),
};
self.shape = self.shape.remove_property_transition(&transition_key);

return Some(result);
// TOOD: figure out if the removed value is ever used otherwise make it return nothing.
let old = self.get_storage(slot);
// shift all elements
self.storage.remove(slot.index as usize + 1);
self.storage.remove(slot.index as usize);

self.shape = self.shape.remove_property_transition(key);
return Some(old);
}

None
Expand Down
31 changes: 24 additions & 7 deletions boa_engine/src/object/shape/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use std::{
rc::Rc,
};

use bitflags::bitflags;
use boa_gc::{empty_trace, Finalize, Gc, GcRefCell, Trace, WeakGc};
use rustc_hash::FxHashMap;

Expand All @@ -31,10 +32,21 @@ use self::{shared_shape::SharedShape, unique_shape::UniqueShape};

use super::JsPrototype;

bitflags! {
#[derive(Default, Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct PropertyDescriptorAttribute: u8 {
const WRITABLE = 0b0000_0001;
const ENUMERABLE = 0b0000_0010;
const CONFIGURABLE = 0b0000_0100;
const HAS_GET = 0b0000_1000;
const HAS_SET = 0b0001_0000;
}
}

#[derive(Debug, Finalize, Clone, PartialEq, Eq, Hash)]
pub(crate) struct TransitionKey {
pub(crate) property_key: PropertyKey,
// attributes: Attribute,
pub(crate) attributes: PropertyDescriptorAttribute,
}

// SAFETY: non of the member of this struct are garbage collected,
Expand All @@ -47,10 +59,8 @@ unsafe impl Trace for TransitionKey {
enum TransitionType {
/// Inserts a new property.
Insert,
// Change existing property attributes
// TODO:
// Configure,

// Change existing property attributes.
Configure,
// TODO:
// Prototype,
}
Expand All @@ -68,7 +78,7 @@ enum Inner {
#[derive(Debug, Clone, Copy)]
pub struct Slot {
pub index: u32,
// pub attribute: Attribute,
pub attributes: PropertyDescriptorAttribute,
}

#[derive(Debug, Trace, Finalize, Clone)]
Expand Down Expand Up @@ -108,7 +118,14 @@ impl Shape {
}
}

pub(crate) fn remove_property_transition(&self, key: &TransitionKey) -> Self {
pub(crate) fn change_attributes_transition(&self, key: TransitionKey) -> Self {
match &self.inner {
Inner::Shared(shape) => Self::shared(shape.change_attributes_transition(key)),
Inner::Unique(shape) => Self::unique(shape.change_attributes_transition(key)),
}
}

pub(crate) fn remove_property_transition(&self, key: &PropertyKey) -> Self {
match &self.inner {
Inner::Shared(shape) => Self::shared(shape.remove_property_transition(key)),
Inner::Unique(shape) => Self::unique(shape.remove_property_transition(key)),
Expand Down
Loading

0 comments on commit 649150e

Please sign in to comment.