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

Module resolution moved out of Behavior and made argument of Isolate::mod_instantiate #1999

Merged
merged 3 commits into from
Mar 25, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions cli/cli_behavior.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
use crate::isolate_state::*;
use crate::ops;
use deno_core::deno_buf;
use deno_core::deno_mod;
use deno_core::Behavior;
use deno_core::Op;
use deno_core::StartupData;
Expand Down Expand Up @@ -43,10 +42,6 @@ impl Behavior for CliBehavior {
self.startup_data.take()
}

fn resolve(&mut self, specifier: &str, referrer: deno_mod) -> deno_mod {
self.state_resolve(specifier, referrer)
}

fn dispatch(
&mut self,
control: &[u8],
Expand Down
5 changes: 0 additions & 5 deletions cli/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use crate::startup_data;
use crate::workers;
use crate::workers::WorkerBehavior;
use deno_core::deno_buf;
use deno_core::deno_mod;
use deno_core::Behavior;
use deno_core::Buf;
use deno_core::Op;
Expand Down Expand Up @@ -51,10 +50,6 @@ impl Behavior for CompilerBehavior {
Some(startup_data::compiler_isolate_init())
}

fn resolve(&mut self, specifier: &str, referrer: deno_mod) -> deno_mod {
self.state_resolve(specifier, referrer)
}

fn dispatch(
&mut self,
control: &[u8],
Expand Down
11 changes: 10 additions & 1 deletion cli/isolate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use deno_core::Behavior;
use deno_core::JSError;
use futures::Async;
use futures::Future;
use std::sync::atomic::Ordering;
use std::sync::Arc;

pub trait DenoBehavior: Behavior + IsolateStateContainer + Send {}
Expand Down Expand Up @@ -122,9 +123,17 @@ impl<B: DenoBehavior> Isolate<B> {

self.mod_load_deps(id)?;

let state = self.state.clone();

let mut resolve = move |specifier: &str, referrer: deno_mod| -> deno_mod {
state.metrics.resolve_count.fetch_add(1, Ordering::Relaxed);
let mut modules = state.modules.lock().unwrap();
modules.resolve_cb(&state.dir, specifier, referrer)
};

self
.inner
.mod_instantiate(id)
.mod_instantiate(id, &mut resolve)
.map_err(RustOrJsError::from)?;
if !is_prefetch {
self.inner.mod_evaluate(id).map_err(RustOrJsError::from)?;
Expand Down
14 changes: 0 additions & 14 deletions cli/isolate_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use crate::flags;
use crate::global_timer::GlobalTimer;
use crate::modules::Modules;
use crate::permissions::DenoPermissions;
use deno_core::deno_mod;
use deno_core::Buf;
use futures::sync::mpsc as async_mpsc;
use std;
Expand Down Expand Up @@ -144,16 +143,3 @@ impl IsolateState {
pub trait IsolateStateContainer {
fn state(&self) -> Arc<IsolateState>;
}

/// Provides state_resolve function for IsolateStateContainer implementors
pub trait IsolateStateModuleResolution: IsolateStateContainer {
fn state_resolve(&mut self, specifier: &str, referrer: deno_mod) -> deno_mod {
let state = self.state();
state.metrics.resolve_count.fetch_add(1, Ordering::Relaxed);
let mut modules = state.modules.lock().unwrap();
modules.resolve_cb(&state.dir, specifier, referrer)
}
}

// Auto implementation for all IsolateStateContainer implementors
impl<T> IsolateStateModuleResolution for T where T: IsolateStateContainer {}
6 changes: 3 additions & 3 deletions cli/startup_data.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
use deno_core::deno_buf;
use deno_core::{StartupData, StartupScript};
use deno_core::{Script, StartupData};

pub fn deno_isolate_init() -> StartupData {
if cfg!(feature = "no-snapshot-init") {
Expand All @@ -11,7 +11,7 @@ pub fn deno_isolate_init() -> StartupData {
#[cfg(feature = "check-only")]
let source_bytes = vec![];

StartupData::Script(StartupScript {
StartupData::Script(Script {
filename: "gen/bundle/main.js".to_string(),
source: std::str::from_utf8(source_bytes).unwrap().to_string(),
})
Expand All @@ -38,7 +38,7 @@ pub fn compiler_isolate_init() -> StartupData {
#[cfg(feature = "check-only")]
let source_bytes = vec![];

StartupData::Script(StartupScript {
StartupData::Script(Script {
filename: "gen/bundle/compiler.js".to_string(),
source: std::str::from_utf8(source_bytes).unwrap().to_string(),
})
Expand Down
7 changes: 1 addition & 6 deletions core/http_bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,17 +102,12 @@ impl Behavior for HttpBench {
fn startup_data(&mut self) -> Option<StartupData> {
let js_source = include_str!("http_bench.js");

Some(StartupData::Script(StartupScript {
Some(StartupData::Script(Script {
source: js_source.to_string(),
filename: "http_bench.js".to_string(),
}))
}

fn resolve(&mut self, _specifier: &str, _referrer: deno_mod) -> deno_mod {
// HttpBench doesn't do ES modules.
unimplemented!()
}

fn dispatch(
&mut self,
control: &[u8],
Expand Down
108 changes: 64 additions & 44 deletions core/isolate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl Future for PendingOp {
}

/// Stores a script used to initalize a Isolate
pub struct StartupScript {
pub struct Script {
pub source: String,
pub filename: String,
}
Expand All @@ -52,7 +52,7 @@ pub struct StartupScript {
/// either a binary snapshot or a javascript source file
/// in the form of the StartupScript struct.
pub enum StartupData {
Script(StartupScript),
Script(Script),
Snapshot(deno_buf),
}

Expand All @@ -63,9 +63,6 @@ pub trait Behavior {
/// Isolate is created.
fn startup_data(&mut self) -> Option<StartupData>;

/// Called during mod_instantiate() to resolve imports.
fn resolve(&mut self, specifier: &str, referrer: deno_mod) -> deno_mod;

/// Called whenever libdeno.send() is called in JavaScript. zero_copy_buf
/// corresponds to the second argument of libdeno.send().
fn dispatch(
Expand Down Expand Up @@ -329,27 +326,46 @@ impl<B: Behavior> Isolate<B> {
}
out
}
}

/// Called during mod_instantiate() to resolve imports.
type ResolveFn = dyn FnMut(&str, deno_mod) -> deno_mod;

/// Used internally by Isolate::mod_instantiate to wrap ResolveFn and
/// encapsulate pointer casts.
struct ResolveContext<'a> {
resolve_fn: &'a mut ResolveFn,
}

impl<'a> ResolveContext<'a> {
#[inline]
fn as_raw_ptr(&mut self) -> *mut c_void {
self as *mut _ as *mut c_void
}

#[inline]
unsafe fn from_raw_ptr(ptr: *mut c_void) -> &'a mut Self {
&mut *(ptr as *mut _)
}
}

pub fn mod_instantiate(&self, id: deno_mod) -> Result<(), JSError> {
impl<B: Behavior> Isolate<B> {
pub fn mod_instantiate(
&mut self,
id: deno_mod,
resolve_fn: &mut ResolveFn,
) -> Result<(), JSError> {
let libdeno_isolate = self.libdeno_isolate;
let mut ctx = ResolveContext { resolve_fn };
unsafe {
libdeno::deno_mod_instantiate(
self.libdeno_isolate,
self.as_raw_ptr(),
libdeno_isolate,
ctx.as_raw_ptr(),
id,
Self::resolve_cb,
)
};
if let Some(js_error) = self.last_exception() {
return Err(js_error);
}
Ok(())
}

pub fn mod_evaluate(&mut self, id: deno_mod) -> Result<(), JSError> {
self.shared_init();
unsafe {
libdeno::deno_mod_evaluate(self.libdeno_isolate, self.as_raw_ptr(), id)
};
if let Some(js_error) = self.last_exception() {
return Err(js_error);
}
Expand All @@ -362,10 +378,23 @@ impl<B: Behavior> Isolate<B> {
specifier_ptr: *const libc::c_char,
referrer: deno_mod,
) -> deno_mod {
let isolate = unsafe { Isolate::<B>::from_raw_ptr(user_data) };
let ResolveContext { resolve_fn } =
unsafe { ResolveContext::from_raw_ptr(user_data) };
let specifier_c: &CStr = unsafe { CStr::from_ptr(specifier_ptr) };
let specifier: &str = specifier_c.to_str().unwrap();
isolate.behavior.resolve(specifier, referrer)

resolve_fn(specifier, referrer)
}

pub fn mod_evaluate(&mut self, id: deno_mod) -> Result<(), JSError> {
self.shared_init();
unsafe {
libdeno::deno_mod_evaluate(self.libdeno_isolate, self.as_raw_ptr(), id)
};
if let Some(js_error) = self.last_exception() {
return Err(js_error);
}
Ok(())
}
}

Expand Down Expand Up @@ -498,7 +527,7 @@ pub fn js_check(r: Result<(), JSError>) {
#[cfg(test)]
mod tests {
use super::*;
use std::collections::HashMap;
use std::sync::atomic::{AtomicUsize, Ordering};

pub enum TestBehaviorMode {
AsyncImmediate,
Expand All @@ -510,18 +539,14 @@ mod tests {

pub struct TestBehavior {
pub dispatch_count: usize,
pub resolve_count: usize,
pub mod_map: HashMap<String, deno_mod>,
mode: TestBehaviorMode,
}

impl TestBehavior {
pub fn setup(mode: TestBehaviorMode) -> Isolate<Self> {
let mut isolate = Isolate::new(TestBehavior {
dispatch_count: 0,
resolve_count: 0,
mode,
mod_map: HashMap::new(),
});
js_check(isolate.execute(
"setup.js",
Expand All @@ -536,25 +561,13 @@ mod tests {
assert_eq!(isolate.behavior.dispatch_count, 0);
isolate
}

pub fn register(&mut self, name: &str, id: deno_mod) {
self.mod_map.insert(name.to_string(), id);
}
}

impl Behavior for TestBehavior {
fn startup_data(&mut self) -> Option<StartupData> {
None
}

fn resolve(&mut self, specifier: &str, _referrer: deno_mod) -> deno_mod {
self.resolve_count += 1;
match self.mod_map.get(specifier) {
Some(id) => *id,
None => 0,
}
}

fn dispatch(
&mut self,
control: &[u8],
Expand Down Expand Up @@ -632,7 +645,6 @@ mod tests {
"#,
).unwrap();
assert_eq!(isolate.behavior.dispatch_count, 0);
assert_eq!(isolate.behavior.resolve_count, 0);

let imports = isolate.mod_get_imports(mod_a);
assert_eq!(imports, vec!["b.js".to_string()]);
Expand All @@ -643,18 +655,26 @@ mod tests {
let imports = isolate.mod_get_imports(mod_b);
assert_eq!(imports.len(), 0);

js_check(isolate.mod_instantiate(mod_b));
let resolve_count = Arc::new(AtomicUsize::new(0));
let resolve_count_ = resolve_count.clone();

let mut resolve = move |specifier: &str, _referrer: deno_mod| -> deno_mod {
resolve_count_.fetch_add(1, Ordering::SeqCst);
assert_eq!(specifier, "b.js");
mod_b
};

js_check(isolate.mod_instantiate(mod_b, &mut resolve));
assert_eq!(isolate.behavior.dispatch_count, 0);
assert_eq!(isolate.behavior.resolve_count, 0);
assert_eq!(resolve_count.load(Ordering::SeqCst), 0);

isolate.behavior.register("b.js", mod_b);
js_check(isolate.mod_instantiate(mod_a));
js_check(isolate.mod_instantiate(mod_a, &mut resolve));
assert_eq!(isolate.behavior.dispatch_count, 0);
assert_eq!(isolate.behavior.resolve_count, 1);
assert_eq!(resolve_count.load(Ordering::SeqCst), 1);

js_check(isolate.mod_evaluate(mod_a));
assert_eq!(isolate.behavior.dispatch_count, 1);
assert_eq!(isolate.behavior.resolve_count, 1);
assert_eq!(resolve_count.load(Ordering::SeqCst), 1);
}

#[test]
Expand Down