foster/denoland-deno
1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-21 15:04:11 -05:00
Code Issues Packages 1 Wiki Activity

Remove Config struct from core (#2502)

Browse source 
It's unnecessary indirection and is preventing the ability to easily
pass isolate references into the dispatch and dyn_import closures.

Note: this changes how StartupData::Script is executed. It's no longer done
during Isolate::new() but rather lazily on first poll or execution.
This commit is contained in:
Ryan Dahl 2019-06-12 10:53:24 -07:00 committed by GitHub
parent 8693d0e0a7
commit 2a5138a516
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 78 additions and 78 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
cli/worker.rs
View file

@ -5,7 +5,6 @@ use crate::js_errors;
use crate::state::ThreadSafeState;
use crate::tokio_util;
use deno;
use deno::Config;
use deno::JSError;
use deno::StartupData;
use futures::Async;
@ -18,7 +17,7 @@ use url::Url;
/// high-level module loading
#[derive(Clone)]
pub struct Worker {
inner: Arc<Mutex<deno::Isolate>>,
isolate: Arc<Mutex<deno::Isolate>>,
pub state: ThreadSafeState,
}
@ -29,14 +28,14 @@ impl Worker {
state: ThreadSafeState,
) -> Worker {
let state_ = state.clone();
let mut config = Config::default();
config.dispatch(move |control_buf, zero_copy_buf| {
state_.dispatch(control_buf, zero_copy_buf)
});
Self {
inner: Arc::new(Mutex::new(deno::Isolate::new(startup_data, config))),
state,
let isolate = Arc::new(Mutex::new(deno::Isolate::new(startup_data, false)));
{
let mut i = isolate.lock().unwrap();
i.set_dispatch(move |control_buf, zero_copy_buf| {
state_.dispatch(control_buf, zero_copy_buf)
});
}
Self { isolate, state }
}
/// Same as execute2() but the filename defaults to "<anonymous>".
@ -51,7 +50,7 @@ impl Worker {
js_filename: &str,
js_source: &str,
) -> Result<(), JSError> {
let mut isolate = self.inner.lock().unwrap();
let mut isolate = self.isolate.lock().unwrap();
isolate.execute(js_filename, js_source)
}
@ -64,7 +63,7 @@ impl Worker {
let worker = self.clone();
let worker_ = worker.clone();
let loader = self.state.clone();
let isolate = self.inner.clone();
let isolate = self.isolate.clone();
let modules = self.state.modules.clone();
let recursive_load =
deno::RecursiveLoad::new(js_url.as_str(), loader, isolate, modules);
@ -74,7 +73,7 @@ impl Worker {
if is_prefetch {
Ok(())
} else {
let mut isolate = worker.inner.lock().unwrap();
let mut isolate = worker.isolate.lock().unwrap();
let result = isolate.mod_evaluate(id);
if let Err(err) = result {
Err(deno::JSErrorOr::JSError(err))
@ -166,7 +165,7 @@ impl Future for Worker {
type Error = JSError;
fn poll(&mut self) -> Result<Async<()>, Self::Error> {
let mut isolate = self.inner.lock().unwrap();
let mut isolate = self.isolate.lock().unwrap();
isolate.poll().map_err(|err| self.apply_source_map(err))
}
}

5
core/examples/http_bench.rs
View file

@ -179,9 +179,8 @@ fn main() {
filename: "http_bench.js",
});
let mut config = deno::Config::default();
config.dispatch(dispatch);
let isolate = deno::Isolate::new(startup_data, config);
let mut isolate = deno::Isolate::new(startup_data, false);
isolate.set_dispatch(dispatch);
isolate.then(|r| {
js_check(r);

126
core/isolate.rs
View file

@ -42,6 +42,22 @@ pub struct Script<'a> {
pub filename: &'a str,
}
// TODO(ry) It's ugly that we have both Script and OwnedScript. Ideally we
// wouldn't expose such twiddly complexity.
struct OwnedScript {
pub source: String,
pub filename: String,
}
impl<'a> From<Script<'a>> for OwnedScript {
fn from(s: Script<'a>) -> OwnedScript {
OwnedScript {
source: s.source.to_string(),
filename: s.filename.to_string(),
}
}
}
/// Represents data used to initialize isolate at startup
/// either a binary snapshot or a javascript source file
/// in the form of the StartupScript struct.
@ -77,32 +93,6 @@ impl Future for DynImport {
}
}
#[derive(Default)]
pub struct Config {
dispatch: Option<Arc<DispatchFn>>,
dyn_import: Option<Arc<DynImportFn>>,
pub will_snapshot: bool,
}
impl Config {
/// Defines the how Deno.core.dispatch() acts.
/// Called whenever Deno.core.dispatch() is called in JavaScript. zero_copy_buf
/// corresponds to the second argument of Deno.core.dispatch().
pub fn dispatch<F>(&mut self, f: F)
where
F: Fn(&[u8], Option<PinnedBuf>) -> Op + Send + Sync + 'static,
{
self.dispatch = Some(Arc::new(f));
}
pub fn dyn_import<F>(&mut self, f: F)
where
F: Fn(&str, &str) -> DynImportFuture + Send + Sync + 'static,
{
self.dyn_import = Some(Arc::new(f));
}
}
/// A single execution context of JavaScript. Corresponds roughly to the "Web
/// Worker" concept in the DOM. An Isolate is a Future that can be used with
/// Tokio. The Isolate future complete when there is an error or when all
@ -114,12 +104,14 @@ impl Config {
pub struct Isolate {
libdeno_isolate: *const libdeno::isolate,
shared_libdeno_isolate: Arc<Mutex<Option<*const libdeno::isolate>>>,
config: Config,
dispatch: Option<Arc<DispatchFn>>,
dyn_import: Option<Arc<DynImportFn>>,
needs_init: bool,
shared: SharedQueue,
pending_ops: FuturesUnordered<OpAsyncFuture>,
pending_dyn_imports: FuturesUnordered<DynImport>,
have_unpolled_ops: bool,
startup_script: Option<OwnedScript>,
}
unsafe impl Send for Isolate {}
@ -138,9 +130,7 @@ static DENO_INIT: Once = ONCE_INIT;
impl Isolate {
/// startup_data defines the snapshot or script used at startup to initalize
/// the isolate.
// TODO(ry) move startup_data into Config. Ideally without introducing a
// generic lifetime into the Isolate struct...
pub fn new(startup_data: StartupData, config: Config) -> Self {
pub fn new(startup_data: StartupData, will_snapshot: bool) -> Self {
DENO_INIT.call_once(|| {
unsafe { libdeno::deno_init() };
});
@ -149,19 +139,20 @@ impl Isolate {
let needs_init = true;
let mut startup_script: Option<Script> = None;
let mut libdeno_config = libdeno::deno_config {
will_snapshot: if config.will_snapshot { 1 } else { 0 },
will_snapshot: will_snapshot.into(),
load_snapshot: Snapshot2::empty(),
shared: shared.as_deno_buf(),
recv_cb: Self::pre_dispatch,
dyn_import_cb: Self::dyn_import,
};
let mut startup_script: Option<OwnedScript> = None;
// Seperate into Option values for each startup type
match startup_data {
StartupData::Script(d) => {
startup_script = Some(d);
startup_script = Some(d.into());
}
StartupData::Snapshot(d) => {
libdeno_config.load_snapshot = d.into();
@ -174,23 +165,35 @@ impl Isolate {
let libdeno_isolate = unsafe { libdeno::deno_new(libdeno_config) };
let mut core_isolate = Self {
Self {
libdeno_isolate,
shared_libdeno_isolate: Arc::new(Mutex::new(Some(libdeno_isolate))),
config,
dispatch: None,
dyn_import: None,
shared,
needs_init,
pending_ops: FuturesUnordered::new(),
have_unpolled_ops: false,
pending_dyn_imports: FuturesUnordered::new(),
};
startup_script,
}
}
// If we want to use execute this has to happen here sadly.
if let Some(s) = startup_script {
core_isolate.execute(s.filename, s.source).unwrap()
};
/// Defines the how Deno.core.dispatch() acts.
/// Called whenever Deno.core.dispatch() is called in JavaScript. zero_copy_buf
/// corresponds to the second argument of Deno.core.dispatch().
pub fn set_dispatch<F>(&mut self, f: F)
where
F: Fn(&[u8], Option<PinnedBuf>) -> Op + Send + Sync + 'static,
{
self.dispatch = Some(Arc::new(f));
}
core_isolate
pub fn set_dyn_import<F>(&mut self, f: F)
where
F: Fn(&str, &str) -> DynImportFuture + Send + Sync + 'static,
{
self.dyn_import = Some(Arc::new(f));
}
/// Get a thread safe handle on the isolate.
@ -201,12 +204,16 @@ impl Isolate {
}
/// Executes a bit of built-in JavaScript to provide Deno.sharedQueue.
pub fn shared_init(&mut self) {
fn shared_init(&mut self) {
if self.needs_init {
self.needs_init = false;
js_check(
self.execute("shared_queue.js", include_str!("shared_queue.js")),
);
// Maybe execute the startup script.
if let Some(s) = self.startup_script.take() {
self.execute(&s.filename, &s.source).unwrap()
}
}
}
@ -222,7 +229,7 @@ impl Isolate {
let referrer = unsafe { CStr::from_ptr(referrer).to_str().unwrap() };
debug!("dyn_import specifier {} referrer {} ", specifier, referrer);
if let Some(ref f) = isolate.config.dyn_import {
if let Some(ref f) = isolate.dyn_import {
let inner = f(specifier, referrer);
let fut = DynImport { inner, id };
task::current().notify();
@ -242,17 +249,17 @@ impl Isolate {
let op = if control_argv0.len() > 0 {
// The user called Deno.core.send(control)
if let Some(ref f) = isolate.config.dispatch {
if let Some(ref f) = isolate.dispatch {
f(control_argv0.as_ref(), PinnedBuf::new(zero_copy_buf))
} else {
panic!("isolate.config.dispatch not set")
panic!("isolate.dispatch not set")
}
} else if let Some(c) = control_shared {
// The user called Deno.sharedQueue.push(control)
if let Some(ref f) = isolate.config.dispatch {
if let Some(ref f) = isolate.dispatch {
f(&c, PinnedBuf::new(zero_copy_buf))
} else {
panic!("isolate.config.dispatch not set")
panic!("isolate.dispatch not set")
}
} else {
// The sharedQueue is empty. The shouldn't happen usually, but it's also
@ -516,6 +523,8 @@ impl Future for Isolate {
// Lock the current thread for V8.
let _locker = LockerScope::new(self.libdeno_isolate);
self.shared_init();
let mut overflow_response: Option<Buf> = None;
loop {
@ -654,8 +663,8 @@ pub mod tests {
let dispatch_count = Arc::new(AtomicUsize::new(0));
let dispatch_count_ = dispatch_count.clone();
let mut config = Config::default();
config.dispatch(move |control, _| -> Op {
let mut isolate = Isolate::new(StartupData::None, false);
isolate.set_dispatch(move |control, _| -> Op {
dispatch_count_.fetch_add(1, Ordering::Relaxed);
match mode {
Mode::AsyncImmediate => {
@ -694,8 +703,6 @@ pub mod tests {
}
}
});
let mut isolate = Isolate::new(StartupData::None, config);
js_check(isolate.execute(
"setup.js",
r#"
@ -861,14 +868,13 @@ pub mod tests {
run_in_task(|| {
let count = Arc::new(AtomicUsize::new(0));
let count_ = count.clone();
let mut config = Config::default();
config.dyn_import(move |specifier, referrer| {
let mut isolate = Isolate::new(StartupData::None, false);
isolate.set_dyn_import(move |specifier, referrer| {
count_.fetch_add(1, Ordering::Relaxed);
assert_eq!(specifier, "foo.js");
assert_eq!(referrer, "dyn_import2.js");
Box::new(futures::future::err(()))
});
let mut isolate = Isolate::new(StartupData::None, config);
js_check(isolate.execute(
"dyn_import2.js",
r#"
@ -896,8 +902,8 @@ pub mod tests {
let mod_b = Arc::new(Mutex::new(0));
let mod_b2 = mod_b.clone();
let mut config = Config::default();
config.dyn_import(move |_specifier, referrer| {
let mut isolate = Isolate::new(StartupData::None, false);
isolate.set_dyn_import(move |_specifier, referrer| {
count_.fetch_add(1, Ordering::Relaxed);
// assert_eq!(specifier, "foo.js");
assert_eq!(referrer, "dyn_import3.js");
@ -905,8 +911,6 @@ pub mod tests {
Box::new(futures::future::ok(*mod_id))
});
let mut isolate = Isolate::new(StartupData::None, config);
// Instantiate mod_b
{
let mut mod_id = mod_b.lock().unwrap();
@ -1159,9 +1163,7 @@ pub mod tests {
#[test]
fn will_snapshot() {
let snapshot = {
let mut config = Config::default();
config.will_snapshot = true;
let mut isolate = Isolate::new(StartupData::None, config);
let mut isolate = Isolate::new(StartupData::None, true);
js_check(isolate.execute("a.js", "a = 1 + 2"));
let s = isolate.snapshot().unwrap();
drop(isolate);
@ -1169,7 +1171,7 @@ pub mod tests {
};
let startup_data = StartupData::LibdenoSnapshot(snapshot);
let mut isolate2 = Isolate::new(startup_data, Config::default());
let mut isolate2 = Isolate::new(startup_data, false);
js_check(isolate2.execute("check.js", "if (a != 3) throw Error('x')"));
}
}