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

Implement inline caching tests and cleanup #3513

Merged
merged 1 commit into from
Feb 6, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 0 additions & 7 deletions core/engine/src/object/internal_methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,8 +577,6 @@ pub(crate) fn ordinary_has_property(
// 2. Let hasOwn be ? O.[[GetOwnProperty]](P).
// 3. If hasOwn is not undefined, return true.
if obj.__get_own_property__(key, context)?.is_some() {
context.slot().attributes |= SlotAttributes::FOUND;

Ok(true)
} else {
// 4. Let parent be ? O.[[GetPrototypeOf]]().
Expand Down Expand Up @@ -627,8 +625,6 @@ pub(crate) fn ordinary_get(
}
}
Some(ref desc) => {
context.slot().attributes |= SlotAttributes::FOUND;

match desc.kind() {
// 4. If IsDataDescriptor(desc) is true, return desc.[[Value]].
DescriptorKind::Data {
Expand Down Expand Up @@ -747,8 +743,6 @@ pub(crate) fn ordinary_set(
return receiver.create_data_property_with_slot(key, value, context);
}

context.slot().attributes |= SlotAttributes::FOUND;

// 4. Assert: IsAccessorDescriptor(ownDesc) is true.
debug_assert!(own_desc.is_accessor_descriptor());

Expand Down Expand Up @@ -902,7 +896,6 @@ pub(crate) fn validate_and_apply_property_descriptor(
},
slot,
);
slot.attributes |= SlotAttributes::FOUND;
}

// e. Return true.
Expand Down
5 changes: 3 additions & 2 deletions core/engine/src/object/property_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,9 @@ impl PropertyMap {
out_slot.index = slot.index;

// Remove all descriptor attributes, but keep inline caching bits.
out_slot.attributes =
(out_slot.attributes & SlotAttributes::INLINE_CACHE_BITS) | slot.attributes;
out_slot.attributes = (out_slot.attributes & SlotAttributes::INLINE_CACHE_BITS)
| slot.attributes
| SlotAttributes::FOUND;
return Some(self.get_storage(slot));
}

Expand Down
15 changes: 14 additions & 1 deletion core/engine/src/object/shape/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ impl From<SharedShape> for Shape {
}

/// Represents a weak reaference to an object's [`Shape`].
#[derive(Debug, Trace, Finalize, Clone)]
#[derive(Debug, Trace, Finalize, Clone, PartialEq)]
pub(crate) enum WeakShape {
Unique(WeakUniqueShape),
Shared(WeakSharedShape),
Expand All @@ -251,6 +251,19 @@ impl WeakShape {
WeakShape::None => 0,
}
}

/// Return location in memory of the [`Shape`].
///
/// Returns `0` if the shape has been freed.
#[inline]
#[must_use]
pub(crate) fn upgrade(&self) -> Option<Shape> {
match self {
WeakShape::Shared(shape) => Some(shape.upgrade()?.into()),
WeakShape::Unique(shape) => Some(shape.upgrade()?.into()),
WeakShape::None => None,
}
}
}

impl From<&Shape> for WeakShape {
Expand Down
12 changes: 11 additions & 1 deletion core/engine/src/object/shape/shared_shape/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ impl SharedShape {
}

/// Represents a weak reference to [`SharedShape`].
#[derive(Debug, Trace, Finalize, Clone)]
#[derive(Debug, Trace, Finalize, Clone, PartialEq)]
pub(crate) struct WeakSharedShape {
inner: WeakGc<Inner>,
}
Expand All @@ -499,6 +499,16 @@ impl WeakSharedShape {
ptr as usize
})
}

/// Upgrade returns a [`SharedShape`] pointer for the internal value if the pointer is still live,
/// or [`None`] if the value was already garbage collected.
#[inline]
#[must_use]
pub(crate) fn upgrade(&self) -> Option<SharedShape> {
Some(SharedShape {
inner: self.inner.upgrade()?,
})
}
}

impl From<&SharedShape> for WeakSharedShape {
Expand Down
10 changes: 10 additions & 0 deletions core/engine/src/object/shape/slot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ impl SlotAttributes {
pub(crate) const fn is_cachable(self) -> bool {
!self.contains(Self::NOT_CACHABLE) && self.contains(Self::FOUND)
}

#[cfg(test)]
pub(crate) const fn in_prototype(self) -> bool {
self.contains(Self::PROTOTYPE)
}
}

/// Represents an [`u32`] index and it's slot attributes of an element in a object storage.
Expand All @@ -69,6 +74,11 @@ impl Slot {
self.attributes.is_cachable()
}

#[cfg(test)]
pub(crate) const fn in_prototype(self) -> bool {
self.attributes.in_prototype()
}

/// Get the width of the slot.
pub(crate) fn width(self) -> u32 {
self.attributes.width()
Expand Down
12 changes: 11 additions & 1 deletion core/engine/src/object/shape/unique_shape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ impl UniqueShape {
}

/// Represents a weak reference to [`UniqueShape`].
#[derive(Debug, Clone, Trace, Finalize)]
#[derive(Debug, Clone, Trace, Finalize, PartialEq)]
pub(crate) struct WeakUniqueShape {
inner: WeakGc<Inner>,
}
Expand All @@ -258,6 +258,16 @@ impl WeakUniqueShape {
ptr as usize
})
}

/// Upgrade returns a [`UniqueShape`] pointer for the internal value if the pointer is still live,
/// or [`None`] if the value was already garbage collected.
#[inline]
#[must_use]
pub(crate) fn upgrade(&self) -> Option<UniqueShape> {
Some(UniqueShape {
inner: self.inner.upgrade()?,
})
}
}

impl From<&UniqueShape> for WeakUniqueShape {
Expand Down
46 changes: 3 additions & 43 deletions core/engine/src/vm/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,17 @@ use crate::{
OrdinaryObject,
},
environments::{BindingLocator, CompileTimeEnvironment},
object::{
shape::{slot::Slot, Shape, WeakShape},
JsObject,
},
object::JsObject,
Context, JsBigInt, JsString, JsValue,
};
use bitflags::bitflags;
use boa_ast::function::FormalParameterList;
use boa_gc::{empty_trace, Finalize, Gc, GcRefCell, Trace};
use boa_gc::{empty_trace, Finalize, Gc, Trace};
use boa_profiler::Profiler;
use std::{cell::Cell, fmt::Display, mem::size_of, rc::Rc};
use thin_vec::ThinVec;

use super::{Instruction, InstructionIterator};
use super::{InlineCache, Instruction, InstructionIterator};

/// This represents whether a value can be read from [`CodeBlock`] code.
///
Expand Down Expand Up @@ -123,43 +120,6 @@ pub(crate) enum Constant {
CompileTimeEnvironment(#[unsafe_ignore_trace] Rc<CompileTimeEnvironment>),
}

/// An inline cache entry for a property access.
#[derive(Clone, Debug, Trace, Finalize)]
pub(crate) struct InlineCache {
/// The property that is accessed.
pub(crate) name: JsString,

/// A pointer is kept to the shape to avoid the shape from being deallocated.
pub(crate) shape: GcRefCell<WeakShape>,

/// The [`Slot`] of the property.
#[unsafe_ignore_trace]
pub(crate) slot: Cell<Slot>,
}

impl InlineCache {
pub(crate) const fn new(name: JsString) -> Self {
Self {
name,
shape: GcRefCell::new(WeakShape::None),
slot: Cell::new(Slot::new()),
}
}

pub(crate) fn set(&self, shape: &Shape, slot: Slot) {
*self.shape.borrow_mut() = shape.into();
self.slot.set(slot);
}

pub(crate) fn slot(&self) -> Slot {
self.slot.get()
}

pub(crate) fn matches(&self, shape: &Shape) -> bool {
self.shape.borrow().to_addr_usize() == shape.to_addr_usize()
}
}

/// The internal representation of a JavaScript function.
///
/// A `CodeBlock` is generated for each function compiled by the
Expand Down
61 changes: 61 additions & 0 deletions core/engine/src/vm/inline_cache/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
use std::cell::Cell;

use boa_gc::GcRefCell;
use boa_macros::{Finalize, Trace};

use crate::{
object::shape::{slot::Slot, Shape, WeakShape},
JsString,
};

#[cfg(test)]
mod tests;

/// An inline cache entry for a property access.
#[derive(Clone, Debug, Trace, Finalize)]
pub(crate) struct InlineCache {
/// The property that is accessed.
pub(crate) name: JsString,

/// A pointer is kept to the shape to avoid the shape from being deallocated.
pub(crate) shape: GcRefCell<WeakShape>,

/// The [`Slot`] of the property.
#[unsafe_ignore_trace]
pub(crate) slot: Cell<Slot>,
}

impl InlineCache {
pub(crate) const fn new(name: JsString) -> Self {
Self {
name,
shape: GcRefCell::new(WeakShape::None),
slot: Cell::new(Slot::new()),
}
}

pub(crate) fn set(&self, shape: &Shape, slot: Slot) {
*self.shape.borrow_mut() = shape.into();
self.slot.set(slot);
}

pub(crate) fn slot(&self) -> Slot {
self.slot.get()
}

/// Returns true, if the [`InlineCache`]'s shape matches with the given shape.
///
/// Otherwise we reset the internal weak reference to [`WeakShape::None`],
/// so it can be deallocated by the GC.
pub(crate) fn match_or_reset(&self, shape: &Shape) -> Option<(Shape, Slot)> {
let mut old = self.shape.borrow_mut();

let old_upgraded = old.upgrade();
if old_upgraded.as_ref().map_or(0, Shape::to_addr_usize) == shape.to_addr_usize() {
return old_upgraded.map(|shape| (shape, self.slot()));
}

*old = WeakShape::None;
None
}
}