Skip to content

Commit

Permalink
Implement non-recursive GC tracing
Browse files Browse the repository at this point in the history
  • Loading branch information
HalidOdat committed Dec 6, 2023
1 parent 47351ef commit c313927
Show file tree
Hide file tree
Showing 32 changed files with 590 additions and 327 deletions.
2 changes: 1 addition & 1 deletion core/engine/src/builtins/function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ pub enum ClassFieldDefinition {
}

unsafe impl Trace for ClassFieldDefinition {
custom_trace! {this, {
custom_trace! {this, mark, {
match this {
Self::Public(_key, func) => {
mark(func);
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/builtins/generator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub(crate) enum GeneratorState {

// Need to manually implement, since `Trace` adds a `Drop` impl which disallows destructuring.
unsafe impl Trace for GeneratorState {
custom_trace!(this, {
custom_trace!(this, mark, {
match &this {
Self::SuspendedStart { context } | Self::SuspendedYield { context } => mark(context),
Self::Executing | Self::Completed => {}
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/builtins/intl/collator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub(crate) struct Collator {

// SAFETY: only `bound_compare` is a traceable object.
unsafe impl Trace for Collator {
custom_trace!(this, mark(&this.bound_compare));
custom_trace!(this, mark, mark(&this.bound_compare));
}

impl Collator {
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/builtins/map/ordered_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub struct OrderedMap<V> {
}

unsafe impl<V: Trace> Trace for OrderedMap<V> {
custom_trace!(this, {
custom_trace!(this, mark, {
for (k, v) in &this.map {
if let MapKey::Key(key) = k {
mark(key);
Expand Down
6 changes: 3 additions & 3 deletions core/engine/src/builtins/promise/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub enum PromiseState {
}

unsafe impl Trace for PromiseState {
custom_trace!(this, {
custom_trace!(this, mark, {
match this {
Self::Fulfilled(v) | Self::Rejected(v) => mark(v),
Self::Pending => {}
Expand Down Expand Up @@ -121,7 +121,7 @@ pub struct ResolvingFunctions {

// Manually implementing `Trace` to allow destructuring.
unsafe impl Trace for ResolvingFunctions {
custom_trace!(this, {
custom_trace!(this, mark, {
mark(&this.resolve);
mark(&this.reject);
});
Expand Down Expand Up @@ -176,7 +176,7 @@ pub(crate) struct PromiseCapability {

// SAFETY: manually implementing `Trace` to allow destructuring.
unsafe impl Trace for PromiseCapability {
custom_trace!(this, {
custom_trace!(this, mark, {
mark(&this.promise);
mark(&this.functions);
});
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/builtins/set/ordered_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub struct OrderedSet {
}

unsafe impl Trace for OrderedSet {
custom_trace!(this, {
custom_trace!(this, mark, {
for v in &this.inner {
if let MapKey::Key(v) = v {
mark(v);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ pub(crate) enum ThisBindingStatus {
}

unsafe impl Trace for ThisBindingStatus {
custom_trace!(this, {
custom_trace!(this, mark, {
match this {
Self::Initialized(obj) => mark(obj),
Self::Lexical | Self::Uninitialized => {}
Expand Down
6 changes: 4 additions & 2 deletions core/engine/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ pub struct JsError {

// SAFETY: just mirroring the default derive to allow destructuring.
unsafe impl Trace for JsError {
custom_trace!(this, mark(&this.inner));
custom_trace!(this, mark, mark(&this.inner));
}

/// Internal representation of a [`JsError`].
Expand All @@ -76,6 +76,7 @@ enum Repr {
unsafe impl Trace for Repr {
custom_trace!(
this,
mark,
match &this {
Self::Native(err) => mark(err),
Self::Opaque(val) => mark(val),
Expand Down Expand Up @@ -549,7 +550,7 @@ impl fmt::Display for JsNativeError {

// SAFETY: just mirroring the default derive to allow destructuring.
unsafe impl Trace for JsNativeError {
custom_trace!(this, {
custom_trace!(this, mark, {
mark(&this.kind);
mark(&this.cause);
mark(&this.realm);
Expand Down Expand Up @@ -1108,6 +1109,7 @@ pub enum JsNativeErrorKind {
unsafe impl Trace for JsNativeErrorKind {
custom_trace!(
this,
mark,
match &this {
Self::Aggregate(errors) => mark(errors),
Self::Error
Expand Down
4 changes: 2 additions & 2 deletions core/engine/src/module/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ enum Status {
// useful to have for state machines like `Status`. This is solved by manually implementing
// `Trace`.
unsafe impl Trace for Status {
custom_trace!(this, {
custom_trace!(this, mark, {
match this {
Self::Unlinked | Self::Linking { info: _ } => {}
Self::PreLinked { context, info: _ } | Self::Linked { context, info: _ } => {
Expand Down Expand Up @@ -239,7 +239,7 @@ impl std::fmt::Debug for SourceTextContext {
}

unsafe impl Trace for SourceTextContext {
custom_trace!(this, {
custom_trace!(this, mark, {
mark(&this.codeblock);
mark(&this.environments);
mark(&this.realm);
Expand Down
4 changes: 2 additions & 2 deletions core/engine/src/native_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub struct NativeFunctionObject {

// SAFETY: this traces all fields that need to be traced by the GC.
unsafe impl Trace for NativeFunctionObject {
custom_trace!(this, {
custom_trace!(this, mark, {
mark(&this.f);
mark(&this.realm);
});
Expand Down Expand Up @@ -125,7 +125,7 @@ enum Inner {
// Manual implementation because deriving `Trace` triggers the `single_use_lifetimes` lint.
// SAFETY: Only closures can contain `Trace` captures, so this implementation is safe.
unsafe impl Trace for NativeFunction {
custom_trace!(this, {
custom_trace!(this, mark, {
if let Inner::Closure(c) = &this.inner {
mark(c);
}
Expand Down
14 changes: 3 additions & 11 deletions core/engine/src/object/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,9 @@ impl dyn NativeObject {
}

/// The internal representation of a JavaScript object.
#[derive(Debug, Finalize)]
#[derive(Debug, Finalize, Trace)]
// SAFETY: This does not implement drop, so this is safe.
#[boa_gc(unsafe_no_drop)]
pub struct Object<T: ?Sized> {
/// The collection of properties contained in the object
pub(crate) properties: PropertyMap,
Expand All @@ -201,16 +203,6 @@ impl<T: Default> Default for Object<T> {
}
}

unsafe impl<T: Trace + ?Sized> Trace for Object<T> {
boa_gc::custom_trace!(this, {
mark(&this.data);
mark(&this.properties);
for (_, element) in &this.private_elements {
mark(element);
}
});
}

/// A Private Name.
#[derive(Clone, Debug, PartialEq, Eq, Trace, Finalize)]
pub struct PrivateName {
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/object/property_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl<K: Trace> Default for OrderedHashMap<K> {
}

unsafe impl<K: Trace> Trace for OrderedHashMap<K> {
custom_trace!(this, {
custom_trace!(this, mark, {
for (k, v) in &this.0 {
mark(k);
mark(v);
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/value/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub enum JsValue {
}

unsafe impl Trace for JsValue {
custom_trace! {this, {
custom_trace! {this, mark, {
if let Self::Object(o) = this {
mark(o);
}
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/vm/completion_record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub(crate) enum CompletionRecord {
// SAFETY: this matches all possible variants and traces
// their inner contents, which makes this safe.
unsafe impl Trace for CompletionRecord {
custom_trace!(this, {
custom_trace!(this, mark, {
match this {
Self::Normal(v) => mark(v),
Self::Return(r) => mark(r),
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/vm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ pub(crate) enum ActiveRunnable {
}

unsafe impl Trace for ActiveRunnable {
custom_trace!(this, {
custom_trace!(this, mark, {
match this {
Self::Script(script) => mark(script),
Self::Module(module) => mark(module),
Expand Down
14 changes: 14 additions & 0 deletions core/engine/src/vm/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,3 +417,17 @@ fn cross_context_funtion_call() {

assert_eq!(result, Ok(JsValue::new(100)));
}

// See: https://github.com/boa-dev/boa/issues/1848
#[test]
fn long_object_chain_gc_trace_stack_overflow() {
run_test_actions([
TestAction::run(indoc! {r#"
let old = {};
for (let i = 0; i < 100000; i++) {
old = { old };
}
"#}),
TestAction::inspect_context(|_| boa_gc::force_collect()),
]);
}
9 changes: 6 additions & 3 deletions core/gc/src/cell.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
//! A garbage collected cell implementation
use crate::trace::{Finalize, Trace};
use crate::{
trace::{Finalize, Trace},
Tracer,
};
use std::{
cell::{Cell, UnsafeCell},
cmp::Ordering,
Expand Down Expand Up @@ -218,11 +221,11 @@ impl<T: Trace + ?Sized> Finalize for GcRefCell<T> {}
// Implementing a Trace while the cell is being written to or incorrectly implementing Trace
// on GcCell's value may cause Undefined Behavior
unsafe impl<T: Trace + ?Sized> Trace for GcRefCell<T> {
unsafe fn trace(&self) {
unsafe fn trace(&self, tracer: &mut Tracer) {
match self.flags.get().borrowed() {
BorrowState::Writing => (),
// SAFETY: Please see GcCell's Trace impl Safety note.
_ => unsafe { (*self.cell.get()).trace() },
_ => unsafe { (*self.cell.get()).trace(tracer) },
}
}

Expand Down
Loading

0 comments on commit c313927

Please sign in to comment.