Skip to content

Commit

Permalink
Use Rc instead of Gc for CompileTimeEnvironments (#3025)
Browse files Browse the repository at this point in the history
* Use `Rc` instead of `Gc` for `CompileTimeEnvironment`s

* Add comment
  • Loading branch information
HalidOdat authored Jun 20, 2023
1 parent b8dd0b0 commit 610cf2c
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 35 deletions.
5 changes: 3 additions & 2 deletions boa_engine/src/bytecompiler/env.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use std::{cell::RefCell, rc::Rc};

use super::ByteCompiler;
use crate::environments::{BindingLocator, BindingLocatorError, CompileTimeEnvironment};
use boa_ast::expression::Identifier;
use boa_gc::{Gc, GcRefCell};

impl ByteCompiler<'_, '_> {
/// Push either a new declarative or function environment on the compile time environment stack.
pub(crate) fn push_compile_environment(&mut self, function_scope: bool) {
self.current_environment = Gc::new(GcRefCell::new(CompileTimeEnvironment::new(
self.current_environment = Rc::new(RefCell::new(CompileTimeEnvironment::new(
self.current_environment.clone(),
function_scope,
)));
Expand Down
6 changes: 4 additions & 2 deletions boa_engine/src/bytecompiler/function.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::{cell::RefCell, rc::Rc};

use crate::{
builtins::function::ThisMode,
bytecompiler::ByteCompiler,
Expand All @@ -6,7 +8,7 @@ use crate::{
Context,
};
use boa_ast::function::{FormalParameterList, FunctionBody};
use boa_gc::{Gc, GcRefCell};
use boa_gc::Gc;
use boa_interner::Sym;

/// `FunctionCompiler` is used to compile AST functions to bytecode.
Expand Down Expand Up @@ -88,7 +90,7 @@ impl FunctionCompiler {
mut self,
parameters: &FormalParameterList,
body: &FunctionBody,
outer_env: Gc<GcRefCell<CompileTimeEnvironment>>,
outer_env: Rc<RefCell<CompileTimeEnvironment>>,
context: &mut Context<'_>,
) -> Gc<CodeBlock> {
self.strict = self.strict || body.strict();
Expand Down
13 changes: 8 additions & 5 deletions boa_engine/src/bytecompiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ mod module;
mod statement;
mod utils;

use std::cell::Cell;
use std::{
cell::{Cell, RefCell},
rc::Rc,
};

use crate::{
builtins::function::ThisMode,
Expand All @@ -35,7 +38,7 @@ use boa_ast::{
pattern::Pattern,
Declaration, Expression, Statement, StatementList, StatementListItem,
};
use boa_gc::{Gc, GcRefCell};
use boa_gc::Gc;
use boa_interner::{Interner, Sym};
use rustc_hash::FxHashMap;

Expand Down Expand Up @@ -241,10 +244,10 @@ pub struct ByteCompiler<'ctx, 'host> {
pub(crate) functions: Vec<Gc<CodeBlock>>,

/// Compile time environments in this function.
pub(crate) compile_environments: Vec<Gc<GcRefCell<CompileTimeEnvironment>>>,
pub(crate) compile_environments: Vec<Rc<RefCell<CompileTimeEnvironment>>>,

/// The environment that is currently active.
pub(crate) current_environment: Gc<GcRefCell<CompileTimeEnvironment>>,
pub(crate) current_environment: Rc<RefCell<CompileTimeEnvironment>>,

pub(crate) code_block_flags: CodeBlockFlags,

Expand Down Expand Up @@ -273,7 +276,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
name: Sym,
strict: bool,
json_parse: bool,
current_environment: Gc<GcRefCell<CompileTimeEnvironment>>,
current_environment: Rc<RefCell<CompileTimeEnvironment>>,
// TODO: remove when we separate scripts from the context
context: &'ctx mut Context<'host>,
) -> ByteCompiler<'ctx, 'host> {
Expand Down
18 changes: 12 additions & 6 deletions boa_engine/src/environments/compile.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use std::{cell::RefCell, rc::Rc};

use crate::environments::runtime::BindingLocator;
use boa_ast::expression::Identifier;
use boa_gc::{Finalize, Gc, GcRefCell, Trace};
use boa_gc::{empty_trace, Finalize, Trace};

use rustc_hash::FxHashMap;

Expand All @@ -20,15 +22,19 @@ struct CompileTimeBinding {
/// A compile time environment maps bound identifiers to their binding positions.
///
/// A compile time environment also indicates, if it is a function environment.
#[derive(Debug, Finalize, Trace)]
#[derive(Debug, Finalize)]
pub(crate) struct CompileTimeEnvironment {
outer: Option<Gc<GcRefCell<Self>>>,
outer: Option<Rc<RefCell<Self>>>,
environment_index: u32,
#[unsafe_ignore_trace]
bindings: FxHashMap<Identifier, CompileTimeBinding>,
function_scope: bool,
}

// Safety: Nothing in this struct needs tracing, so this is safe.
unsafe impl Trace for CompileTimeEnvironment {
empty_trace!();
}

impl CompileTimeEnvironment {
/// Creates a new global compile time environment.
pub(crate) fn new_global() -> Self {
Expand All @@ -41,7 +47,7 @@ impl CompileTimeEnvironment {
}

/// Creates a new compile time environment.
pub(crate) fn new(parent: Gc<GcRefCell<Self>>, function_scope: bool) -> Self {
pub(crate) fn new(parent: Rc<RefCell<Self>>, function_scope: bool) -> Self {
let index = parent.borrow().environment_index + 1;
Self {
outer: Some(parent),
Expand Down Expand Up @@ -291,7 +297,7 @@ impl CompileTimeEnvironment {
}

/// Gets the outer environment of this environment.
pub(crate) fn outer(&self) -> Option<Gc<GcRefCell<Self>>> {
pub(crate) fn outer(&self) -> Option<Rc<RefCell<Self>>> {
self.outer.clone()
}

Expand Down
18 changes: 12 additions & 6 deletions boa_engine/src/environments/runtime/declarative/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@ mod global;
mod lexical;
mod module;

use std::cell::Cell;
use std::{
cell::{Cell, RefCell},
rc::Rc,
};

use boa_gc::{Finalize, Gc, GcRefCell, Trace};
use boa_gc::{Finalize, GcRefCell, Trace};
pub(crate) use function::{FunctionEnvironment, FunctionSlots, ThisBindingStatus};
pub(crate) use global::GlobalEnvironment;
pub(crate) use lexical::LexicalEnvironment;
Expand Down Expand Up @@ -36,28 +39,31 @@ use crate::{environments::CompileTimeEnvironment, JsObject, JsResult, JsValue};
#[derive(Debug, Trace, Finalize)]
pub(crate) struct DeclarativeEnvironment {
kind: DeclarativeEnvironmentKind,
compile: Gc<GcRefCell<CompileTimeEnvironment>>,

// Safety: Nothing in CompileTimeEnvironment needs tracing.
#[unsafe_ignore_trace]
compile: Rc<RefCell<CompileTimeEnvironment>>,
}

impl DeclarativeEnvironment {
/// Creates a new global `DeclarativeEnvironment`.
pub(crate) fn global(global_this: JsObject) -> Self {
Self {
kind: DeclarativeEnvironmentKind::Global(GlobalEnvironment::new(global_this)),
compile: Gc::new(GcRefCell::new(CompileTimeEnvironment::new_global())),
compile: Rc::new(RefCell::new(CompileTimeEnvironment::new_global())),
}
}

/// Creates a new `DeclarativeEnvironment` from its kind and compile environment.
pub(crate) fn new(
kind: DeclarativeEnvironmentKind,
compile: Gc<GcRefCell<CompileTimeEnvironment>>,
compile: Rc<RefCell<CompileTimeEnvironment>>,
) -> Self {
Self { kind, compile }
}

/// Gets the compile time environment of this environment.
pub(crate) fn compile_env(&self) -> Gc<GcRefCell<CompileTimeEnvironment>> {
pub(crate) fn compile_env(&self) -> Rc<RefCell<CompileTimeEnvironment>> {
self.compile.clone()
}

Expand Down
17 changes: 8 additions & 9 deletions boa_engine/src/environments/runtime/mod.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use std::{cell::RefCell, rc::Rc};

use crate::{
environments::CompileTimeEnvironment,
object::{JsObject, PrivateName},
Context, JsResult, JsString, JsSymbol, JsValue,
};
use boa_ast::expression::Identifier;
use boa_gc::{empty_trace, Finalize, Gc, GcRefCell, Trace};
use boa_gc::{empty_trace, Finalize, Gc, Trace};
use rustc_hash::FxHashSet;

mod declarative;
Expand Down Expand Up @@ -220,7 +222,7 @@ impl EnvironmentStack {
#[track_caller]
pub(crate) fn push_lexical(
&mut self,
compile_environment: Gc<GcRefCell<CompileTimeEnvironment>>,
compile_environment: Rc<RefCell<CompileTimeEnvironment>>,
) -> u32 {
let num_bindings = compile_environment.borrow().num_bindings();

Expand Down Expand Up @@ -265,7 +267,7 @@ impl EnvironmentStack {
#[track_caller]
pub(crate) fn push_function(
&mut self,
compile_environment: Gc<GcRefCell<CompileTimeEnvironment>>,
compile_environment: Rc<RefCell<CompileTimeEnvironment>>,
function_slots: FunctionSlots,
) {
let num_bindings = compile_environment.borrow().num_bindings();
Expand Down Expand Up @@ -309,7 +311,7 @@ impl EnvironmentStack {
#[track_caller]
pub(crate) fn push_function_inherit(
&mut self,
compile_environment: Gc<GcRefCell<CompileTimeEnvironment>>,
compile_environment: Rc<RefCell<CompileTimeEnvironment>>,
) {
let num_bindings = compile_environment.borrow().num_bindings();

Expand Down Expand Up @@ -361,10 +363,7 @@ impl EnvironmentStack {
///
/// Panics if no environment exists on the stack.
#[track_caller]
pub(crate) fn push_module(
&mut self,
compile_environment: Gc<GcRefCell<CompileTimeEnvironment>>,
) {
pub(crate) fn push_module(&mut self, compile_environment: Rc<RefCell<CompileTimeEnvironment>>) {
let num_bindings = compile_environment.borrow().num_bindings();
self.stack.push(Environment::Declarative(Gc::new(
DeclarativeEnvironment::new(
Expand Down Expand Up @@ -401,7 +400,7 @@ impl EnvironmentStack {
/// # Panics
///
/// Panics if no environment exists on the stack.
pub(crate) fn current_compile_environment(&self) -> Gc<GcRefCell<CompileTimeEnvironment>> {
pub(crate) fn current_compile_environment(&self) -> Rc<RefCell<CompileTimeEnvironment>> {
self.stack
.iter()
.filter_map(Environment::as_declarative)
Expand Down
4 changes: 2 additions & 2 deletions boa_engine/src/module/source.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::{
cell::Cell,
cell::{Cell, RefCell},
collections::HashSet,
hash::{BuildHasherDefault, Hash},
rc::Rc,
Expand Down Expand Up @@ -1403,7 +1403,7 @@ impl SourceTextModule {
// 6. Set module.[[Environment]] to env.
let global_env = realm.environment().clone();
let global_compile_env = global_env.compile_env();
let module_compile_env = Gc::new(GcRefCell::new(CompileTimeEnvironment::new(
let module_compile_env = Rc::new(RefCell::new(CompileTimeEnvironment::new(
global_compile_env,
true,
)));
Expand Down
16 changes: 13 additions & 3 deletions boa_engine/src/vm/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,15 @@ use crate::{
};
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_interner::Sym;
use boa_profiler::Profiler;
use std::{cell::Cell, collections::VecDeque, mem::size_of};
use std::{
cell::{Cell, RefCell},
collections::VecDeque,
mem::size_of,
rc::Rc,
};
use thin_vec::ThinVec;

#[cfg(any(feature = "trace", feature = "flowgraph"))]
Expand Down Expand Up @@ -129,7 +134,12 @@ pub struct CodeBlock {
pub(crate) functions: Box<[Gc<Self>]>,

/// Compile time environments in this function.
pub(crate) compile_environments: Box<[Gc<GcRefCell<CompileTimeEnvironment>>]>,
///
// Safety: Nothing in CompileTimeEnvironment needs tracing, so this is safe.
//
// TODO(#3034): Maybe changing this to Gc after garbage collection would be better than Rc.
#[unsafe_ignore_trace]
pub(crate) compile_environments: Box<[Rc<RefCell<CompileTimeEnvironment>>]>,
}

/// ---- `CodeBlock` public API ----
Expand Down

0 comments on commit 610cf2c

Please sign in to comment.