Skip to content

Commit

Permalink
Allow passing owned HostHooks and JobQueues to Context (#2811)
Browse files Browse the repository at this point in the history
This allows `thread_local` contexts to have owned `HostHooks` and `JobQueues`.

It changes the following:

- Creates a new `MaybeShared` struct that can hold either a reference or an `Rc`.
- Changes the `job_queue` and `host_hooks` parameters of `Context` to use `MaybeShared`.

This PR also allows us to make `SimpleJobQueue` the default promise runner, which I think it's pretty cool :)

cc @lastmjs
  • Loading branch information
jedel1043 committed Apr 12, 2023
1 parent 8276982 commit 0d6ba53
Show file tree
Hide file tree
Showing 14 changed files with 147 additions and 119 deletions.
4 changes: 2 additions & 2 deletions boa_cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,9 @@ fn evaluate_files(args: &Opt, context: &mut Context<'_>) -> Result<(), io::Error
fn main() -> Result<(), io::Error> {
let args = Opt::parse();

let queue = Jobs::default();
let queue: &dyn JobQueue = &Jobs::default();
let mut context = ContextBuilder::new()
.job_queue(&queue)
.job_queue(queue)
.build()
.expect("cannot fail with default global object");

Expand Down
4 changes: 2 additions & 2 deletions boa_engine/src/builtins/intl/locale/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use icu_locid::{
locale, Locale,
};
use icu_plurals::provider::CardinalV1Marker;
use icu_provider::{DataLocale, DataProvider, DataRequest, DataRequestMetadata};
use icu_provider::{BufferProvider, DataLocale, DataProvider, DataRequest, DataRequestMetadata};

use crate::{
builtins::intl::{
Expand Down Expand Up @@ -73,7 +73,7 @@ impl Service for TestService {

#[test]
fn locale_resolution() {
let provider = boa_icu_provider::buffer();
let provider: &dyn BufferProvider = boa_icu_provider::buffer();
let icu = Icu::new(BoaProvider::Buffer(provider)).unwrap();
let mut default = default_locale(icu.locale_canonicalizer());
default
Expand Down
4 changes: 2 additions & 2 deletions boa_engine/src/builtins/intl/locale/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ pub(in crate::builtins::intl) fn validate_extension<M: KeyedDataMarker>(
mod tests {
use icu_locid::{langid, locale, Locale};
use icu_plurals::provider::CardinalV1Marker;
use icu_provider::AsDeserializingBufferProvider;
use icu_provider::{AsDeserializingBufferProvider, BufferProvider};

use crate::{
builtins::intl::locale::utils::{
Expand Down Expand Up @@ -579,7 +579,7 @@ mod tests {

#[test]
fn lookup_match() {
let provider = boa_icu_provider::buffer();
let provider: &dyn BufferProvider = boa_icu_provider::buffer();
let icu = Icu::new(BoaProvider::Buffer(provider)).unwrap();

// requested: []
Expand Down
21 changes: 8 additions & 13 deletions boa_engine/src/builtins/promise/tests.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,20 @@
use crate::{context::ContextBuilder, job::SimpleJobQueue, run_test_actions_with, TestAction};
use crate::{run_test_actions, TestAction};
use indoc::indoc;

#[test]
fn promise() {
let queue = SimpleJobQueue::new();
let context = &mut ContextBuilder::new().job_queue(&queue).build().unwrap();
run_test_actions_with(
[
TestAction::run(indoc! {r#"
run_test_actions([
TestAction::run(indoc! {r#"
let count = 0;
const promise = new Promise((resolve, reject) => {
count += 1;
resolve(undefined);
}).then((_) => (count += 1));
count += 1;
"#}),
TestAction::assert_eq("count", 2),
#[allow(clippy::redundant_closure_for_method_calls)]
TestAction::inspect_context(|ctx| ctx.run_jobs()),
TestAction::assert_eq("count", 3),
],
context,
);
TestAction::assert_eq("count", 2),
#[allow(clippy::redundant_closure_for_method_calls)]
TestAction::inspect_context(|ctx| ctx.run_jobs()),
TestAction::assert_eq("count", 3),
]);
}
4 changes: 2 additions & 2 deletions boa_engine/src/context/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ use super::intrinsics::Intrinsics;
/// Err(JsNativeError::typ().with_message("eval calls not available").into())
/// }
/// }
/// let hooks = Hooks; // Can have additional state.
/// let context = &mut ContextBuilder::new().host_hooks(&hooks).build().unwrap();
/// let hooks: &dyn HostHooks = &Hooks; // Can have additional state.
/// let context = &mut ContextBuilder::new().host_hooks(hooks).build().unwrap();
/// let result = context.eval_script(Source::from_bytes(r#"eval("let a = 5")"#));
/// assert_eq!(result.unwrap_err().to_string(), "TypeError: eval calls not available");
/// ```
Expand Down
9 changes: 4 additions & 5 deletions boa_engine/src/context/icu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ where
M::Yokeable: ZeroFrom<'static, M::Yokeable> + MaybeSendSync,
{
fn load(&self, req: DataRequest<'_>) -> Result<DataResponse<M>, DataError> {
match *self {
match self {
BoaProvider::Buffer(provider) => provider.as_deserializing().load(req),
BoaProvider::Any(provider) => provider.as_downcasting().load(req),
}
Expand Down Expand Up @@ -125,8 +125,8 @@ impl BoaProvider<'_> {
}
}

/// Collection of tools initialized from a [`DataProvider`] that are used
/// for the functionality of `Intl`.
/// Collection of tools initialized from a [`DataProvider`] that are used for the functionality of
/// `Intl`.
pub(crate) struct Icu<'provider> {
provider: BoaProvider<'provider>,
locale_canonicalizer: LocaleCanonicalizer,
Expand All @@ -148,8 +148,7 @@ impl<'provider> Icu<'provider> {
///
/// # Errors
///
/// This method will return an error if any of the tools
/// required cannot be constructed.
/// Returns an error if any of the tools required cannot be constructed.
pub(crate) fn new(
provider: BoaProvider<'provider>,
) -> Result<Icu<'provider>, LocaleTransformError> {
Expand Down
42 changes: 42 additions & 0 deletions boa_engine/src/context/maybe_shared.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
use std::{ops::Deref, rc::Rc};

/// A [`Cow`][std::borrow::Cow]-like pointer where the `Owned` variant is an [`Rc`].
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
pub enum MaybeShared<'a, T: ?Sized> {
/// Borrowed data.
Borrowed(&'a T),
/// `Rc` shared data.
Shared(Rc<T>),
}

impl<T: ?Sized> Clone for MaybeShared<'_, T> {
fn clone(&self) -> Self {
match self {
Self::Borrowed(b) => Self::Borrowed(b),
Self::Shared(sh) => Self::Shared(sh.clone()),
}
}
}

impl<T: ?Sized> Deref for MaybeShared<'_, T> {
type Target = T;

fn deref(&self) -> &Self::Target {
match self {
MaybeShared::Borrowed(b) => b,
MaybeShared::Shared(sh) => sh,
}
}
}

impl<'a, T: ?Sized> From<&'a T> for MaybeShared<'a, T> {
fn from(value: &'a T) -> Self {
Self::Borrowed(value)
}
}

impl<T: ?Sized> From<Rc<T>> for MaybeShared<'static, T> {
fn from(value: Rc<T>) -> Self {
Self::Shared(value)
}
}
71 changes: 46 additions & 25 deletions boa_engine/src/context/mod.rs
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
//! The ECMAScript context.
mod hooks;
pub mod intrinsics;
pub use hooks::{DefaultHooks, HostHooks};

#[cfg(feature = "intl")]
pub(crate) mod icu;
pub mod intrinsics;
mod maybe_shared;

pub use hooks::{DefaultHooks, HostHooks};
#[cfg(feature = "intl")]
pub use icu::BoaProvider;

use intrinsics::Intrinsics;
pub use maybe_shared::MaybeShared;

use std::io::Read;
#[cfg(not(feature = "intl"))]
pub use std::marker::PhantomData;
use std::{io::Read, rc::Rc};

use crate::{
builtins,
bytecompiler::ByteCompiler,
class::{Class, ClassBuilder},
job::{IdleJobQueue, JobQueue, NativeJob},
job::{JobQueue, NativeJob, SimpleJobQueue},
native_function::NativeFunction,
object::{FunctionObjectBuilder, JsObject},
optimizer::{Optimizer, OptimizerOptions, OptimizerStatistics},
Expand Down Expand Up @@ -99,9 +99,9 @@ pub struct Context<'host> {
#[cfg(feature = "intl")]
icu: icu::Icu<'host>,

host_hooks: &'host dyn HostHooks,
host_hooks: MaybeShared<'host, dyn HostHooks>,

job_queue: &'host dyn JobQueue,
job_queue: MaybeShared<'host, dyn JobQueue>,

optimizer_options: OptimizerOptions,
}
Expand Down Expand Up @@ -494,12 +494,12 @@ impl<'host> Context<'host> {

/// Enqueues a [`NativeJob`] on the [`JobQueue`].
pub fn enqueue_job(&mut self, job: NativeJob) {
self.job_queue.enqueue_promise_job(job, self);
self.job_queue().enqueue_promise_job(job, self);
}

/// Runs all the jobs in the job queue.
pub fn run_jobs(&mut self) {
self.job_queue.run_jobs(self);
self.job_queue().run_jobs(self);
self.clear_kept_objects();
}

Expand All @@ -524,13 +524,13 @@ impl<'host> Context<'host> {
}

/// Gets the host hooks.
pub fn host_hooks(&self) -> &'host dyn HostHooks {
self.host_hooks
pub fn host_hooks(&self) -> MaybeShared<'host, dyn HostHooks> {
self.host_hooks.clone()
}

/// Gets the job queue.
pub fn job_queue(&mut self) -> &'host dyn JobQueue {
self.job_queue
pub fn job_queue(&self) -> MaybeShared<'host, dyn JobQueue> {
self.job_queue.clone()
}
}

Expand Down Expand Up @@ -558,8 +558,8 @@ impl<'host> Context<'host> {
#[derive(Default)]
pub struct ContextBuilder<'icu, 'hooks, 'queue> {
interner: Option<Interner>,
host_hooks: Option<&'hooks dyn HostHooks>,
job_queue: Option<&'queue dyn JobQueue>,
host_hooks: Option<MaybeShared<'hooks, dyn HostHooks>>,
job_queue: Option<MaybeShared<'queue, dyn JobQueue>>,
#[cfg(feature = "intl")]
icu: Option<icu::Icu<'icu>>,
#[cfg(not(feature = "intl"))]
Expand All @@ -570,10 +570,15 @@ pub struct ContextBuilder<'icu, 'hooks, 'queue> {

impl std::fmt::Debug for ContextBuilder<'_, '_, '_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
#[derive(Clone, Copy, Debug)]
struct JobQueue;
#[derive(Clone, Copy, Debug)]
struct HostHooks;
let mut out = f.debug_struct("ContextBuilder");

out.field("interner", &self.interner)
.field("host_hooks", &"HostHooks");
.field("host_hooks", &self.host_hooks.as_ref().map(|_| HostHooks))
.field("job_queue", &self.job_queue.as_ref().map(|_| JobQueue));

#[cfg(feature = "intl")]
out.field("icu", &self.icu);
Expand Down Expand Up @@ -632,18 +637,27 @@ impl<'icu, 'hooks, 'queue> ContextBuilder<'icu, 'hooks, 'queue> {
///
/// [`Host Hooks`]: https://tc39.es/ecma262/#sec-host-hooks-summary
#[must_use]
pub fn host_hooks(self, host_hooks: &dyn HostHooks) -> ContextBuilder<'icu, '_, 'queue> {
pub fn host_hooks<'new_hooks, H>(
self,
host_hooks: H,
) -> ContextBuilder<'icu, 'new_hooks, 'queue>
where
H: Into<MaybeShared<'new_hooks, dyn HostHooks>>,
{
ContextBuilder {
host_hooks: Some(host_hooks),
host_hooks: Some(host_hooks.into()),
..self
}
}

/// Initializes the [`JobQueue`] for the context.
#[must_use]
pub fn job_queue(self, job_queue: &dyn JobQueue) -> ContextBuilder<'icu, 'hooks, '_> {
pub fn job_queue<'new_queue, Q>(self, job_queue: Q) -> ContextBuilder<'icu, 'hooks, 'new_queue>
where
Q: Into<MaybeShared<'new_queue, dyn JobQueue>>,
{
ContextBuilder {
job_queue: Some(job_queue),
job_queue: Some(job_queue.into()),
..self
}
}
Expand All @@ -666,8 +680,11 @@ impl<'icu, 'hooks, 'queue> ContextBuilder<'icu, 'hooks, 'queue> {
'hooks: 'host,
'queue: 'host,
{
let host_hooks = self.host_hooks.unwrap_or(&DefaultHooks);
let realm = Realm::create(host_hooks);
let host_hooks = self.host_hooks.unwrap_or_else(|| {
let hooks: &dyn HostHooks = &DefaultHooks;
hooks.into()
});
let realm = Realm::create(&*host_hooks);
let vm = Vm::new(realm.environment().clone());

let mut context = Context {
Expand All @@ -677,14 +694,18 @@ impl<'icu, 'hooks, 'queue> ContextBuilder<'icu, 'hooks, 'queue> {
strict: false,
#[cfg(feature = "intl")]
icu: self.icu.unwrap_or_else(|| {
let provider = BoaProvider::Buffer(boa_icu_provider::buffer());
let buffer: &dyn icu_provider::BufferProvider = boa_icu_provider::buffer();
let provider = BoaProvider::Buffer(buffer);
icu::Icu::new(provider).expect("Failed to initialize default icu data.")
}),
#[cfg(feature = "fuzz")]
instructions_remaining: self.instructions_remaining,
kept_alive: Vec::new(),
host_hooks,
job_queue: self.job_queue.unwrap_or(&IdleJobQueue),
job_queue: self.job_queue.unwrap_or_else(|| {
let queue: Rc<dyn JobQueue> = Rc::new(SimpleJobQueue::new());
queue.into()
}),
optimizer_options: OptimizerOptions::OPTIMIZE_ALL,
};

Expand Down
25 changes: 13 additions & 12 deletions boa_engine/src/job.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,17 @@ pub trait JobQueue {

/// A job queue that does nothing.
///
/// This is the default job queue for the [`Context`], and is useful if you want to disable
/// the promise capabilities of the engine.
/// This queue is mostly useful if you want to disable the promise capabilities of the engine. This
/// can be done by passing a reference to it to the [`ContextBuilder`]:
///
/// If you want to enable running promise jobs, see [`SimpleJobQueue`].
/// ```
/// use boa_engine::{context::ContextBuilder, job::{JobQueue, IdleJobQueue}};
///
/// let queue: &dyn JobQueue = &IdleJobQueue;
/// let context = ContextBuilder::new().job_queue(queue).build();
/// ```
///
/// [`ContextBuilder`]: crate::context::ContextBuilder
#[derive(Debug, Clone, Copy)]
pub struct IdleJobQueue;

Expand All @@ -215,16 +222,10 @@ impl JobQueue for IdleJobQueue {

/// A simple FIFO job queue that bails on the first error.
///
/// To enable running promise jobs on the engine, you need to pass it to the [`ContextBuilder`]:
/// This is the default job queue for the [`Context`], but it is mostly pretty limited for
/// custom event queues.
///
/// ```
/// use boa_engine::{context::ContextBuilder, job::SimpleJobQueue};
///
/// let queue = SimpleJobQueue::new();
/// let context = ContextBuilder::new().job_queue(&queue).build();
/// ```
///
/// [`ContextBuilder`]: crate::context::ContextBuilder
/// To disable running promise jobs on the engine, see [`IdleJobQueue`].
#[derive(Default)]
pub struct SimpleJobQueue(RefCell<VecDeque<NativeJob>>);

Expand Down
3 changes: 1 addition & 2 deletions boa_engine/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,15 +151,14 @@ pub mod error;
pub mod job;
pub mod native_function;
pub mod object;
pub mod optimizer;
pub mod property;
pub mod realm;
pub mod string;
pub mod symbol;
pub mod value;
pub mod vm;

pub mod optimizer;

#[cfg(feature = "console")]
pub mod console;

Expand Down
Loading

0 comments on commit 0d6ba53

Please sign in to comment.