Skip to content

Commit

Permalink
minor clean ups in TS compiler (#3394)
Browse files Browse the repository at this point in the history
  • Loading branch information
bartlomieju authored and ry committed Nov 22, 2019
1 parent 8d977d0 commit 363b968
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 141 deletions.
2 changes: 1 addition & 1 deletion cli/compilers/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use crate::compilers::CompiledModule;
use crate::compilers::CompiledModuleFuture;
use crate::file_fetcher::SourceFile;
use crate::futures::future::FutureExt;
use futures::future::FutureExt;
use std::pin::Pin;
use std::str;

Expand Down
155 changes: 55 additions & 100 deletions cli/compilers/ts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use deno::Buf;
use deno::ErrBox;
use deno::ModuleSpecifier;
use futures::future::FutureExt;
use futures::future::TryFutureExt;
use futures::Future;
use regex::Regex;
use std::collections::HashSet;
Expand Down Expand Up @@ -272,33 +271,22 @@ impl TsCompiler {

let worker = TsCompiler::setup_worker(global_state.clone());
let worker_ = worker.clone();
let first_msg_fut = async move {
worker.post_message(req_msg).await.unwrap();
let result = worker.await;
if let Err(err) = result {
// TODO(ry) Need to forward the error instead of exiting.
eprintln!("{}", err.to_string());
std::process::exit(1);
}
debug!("Sent message to worker");
worker_.get_message().await
};

first_msg_fut.map_err(|_| panic!("not handled")).and_then(
move |maybe_msg: Option<Buf>| {
debug!("Received message from worker");

if let Some(msg) = maybe_msg {
let json_str = std::str::from_utf8(&msg).unwrap();
debug!("Message: {}", json_str);
if let Some(diagnostics) = Diagnostic::from_emit_result(json_str) {
return futures::future::err(ErrBox::from(diagnostics));
}
async move {
worker.post_message(req_msg).await?;
worker.await?;
debug!("Sent message to worker");
let maybe_msg = worker_.get_message().await?;
debug!("Received message from worker");
if let Some(msg) = maybe_msg {
let json_str = std::str::from_utf8(&msg).unwrap();
debug!("Message: {}", json_str);
if let Some(diagnostics) = Diagnostic::from_emit_result(json_str) {
return Err(ErrBox::from(diagnostics));
}

futures::future::ok(())
},
)
}
Ok(())
}
}

/// Mark given module URL as compiled to avoid multiple compilations of same module
Expand Down Expand Up @@ -382,63 +370,27 @@ impl TsCompiler {
.add("Compile", &module_url.to_string());
let global_state_ = global_state.clone();

let first_msg_fut = async move {
worker.post_message(req_msg).await.unwrap();
let result = worker.await;
if let Err(err) = result {
// TODO(ry) Need to forward the error instead of exiting.
eprintln!("{}", err.to_string());
std::process::exit(1);
}
async move {
worker.post_message(req_msg).await?;
worker.await?;
debug!("Sent message to worker");
worker_.get_message().await
};

let fut = first_msg_fut
.map_err(|_| panic!("not handled"))
.and_then(move |maybe_msg: Option<Buf>| {
debug!("Received message from worker");

if let Some(msg) = maybe_msg {
let json_str = std::str::from_utf8(&msg).unwrap();
debug!("Message: {}", json_str);
if let Some(diagnostics) = Diagnostic::from_emit_result(json_str) {
return futures::future::err(ErrBox::from(diagnostics));
}
let maybe_msg = worker_.get_message().await?;
if let Some(msg) = maybe_msg {
let json_str = std::str::from_utf8(&msg).unwrap();
debug!("Message: {}", json_str);
if let Some(diagnostics) = Diagnostic::from_emit_result(json_str) {
return Err(ErrBox::from(diagnostics));
}

futures::future::ok(())
})
.and_then(move |_| {
// if we are this far it means compilation was successful and we can
// load compiled filed from disk
futures::future::ready(
global_state_
.ts_compiler
.get_compiled_module(&source_file_.url)
.map_err(|e| {
// TODO: this situation shouldn't happen
panic!(
"Expected to find compiled file: {} {}",
e, source_file_.url
)
}),
)
})
.and_then(move |compiled_module| {
// Explicit drop to keep reference alive until future completes.
drop(compiling_job);

futures::future::ok(compiled_module)
})
.then(move |r| {
debug!(">>>>> compile_sync END");
// TODO(ry) do this in worker's destructor.
// resource.close();
futures::future::ready(r)
});

fut.boxed()
}
let compiled_module = global_state_
.ts_compiler
.get_compiled_module(&source_file_.url)
.expect("Expected to find compiled file");
drop(compiling_job);
debug!(">>>>> compile_sync END");
Ok(compiled_module)
}
.boxed()
}

/// Get associated `CompiledFileMetadata` for given module if it exists.
Expand Down Expand Up @@ -684,20 +636,22 @@ mod tests {
String::from("hello.js"),
]);

tokio_util::run(
mock_state
let fut = async move {
let result = mock_state
.ts_compiler
.compile_async(mock_state.clone(), &out)
.then(|result| {
assert!(result.is_ok());
assert!(result
.unwrap()
.code
.as_bytes()
.starts_with("console.log(\"Hello World\");".as_bytes()));
futures::future::ok(())
}),
)
.await;

assert!(result.is_ok());
assert!(result
.unwrap()
.code
.as_bytes()
.starts_with("console.log(\"Hello World\");".as_bytes()));
Ok(())
};

tokio_util::run(fut.boxed())
}

#[test]
Expand All @@ -718,19 +672,20 @@ mod tests {
String::from("$deno$/bundle.js"),
]);

tokio_util::run(
state
let fut = async move {
let result = state
.ts_compiler
.bundle_async(
state.clone(),
module_name,
Some(String::from("$deno$/bundle.js")),
)
.then(|result| {
assert!(result.is_ok());
futures::future::ok(())
}),
)
.await;

assert!(result.is_ok());
Ok(())
};
tokio_util::run(fut.boxed())
}

#[test]
Expand Down
76 changes: 36 additions & 40 deletions cli/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,17 +128,19 @@ impl Worker {
maybe_code,
loader,
modules,
)
.get_future(isolate);
recursive_load.and_then(move |id| {
);

async move {
let id = recursive_load.get_future(isolate).await?;
worker.state.global_state.progress.done();
if is_prefetch {
futures::future::ok(())
} else {

if !is_prefetch {
let mut isolate = worker.isolate.lock().unwrap();
futures::future::ready(isolate.mod_evaluate(id))
return isolate.mod_evaluate(id);
}
})

Ok(())
}
}

/// Post message to worker as a host.
Expand Down Expand Up @@ -235,15 +237,13 @@ mod tests {
tokio_util::run(async move {
let mut worker =
Worker::new("TEST".to_string(), StartupData::None, state, ext);
worker
let result = worker
.execute_mod_async(&module_specifier, None, false)
.then(|result| {
if let Err(err) = result {
eprintln!("execute_mod err {:?}", err);
}
tokio_util::panic_on_error(worker)
})
.await
.await;
if let Err(err) = result {
eprintln!("execute_mod err {:?}", err);
}
tokio_util::panic_on_error(worker).await
});

let metrics = &state_.metrics;
Expand Down Expand Up @@ -277,15 +277,13 @@ mod tests {
tokio_util::run(async move {
let mut worker =
Worker::new("TEST".to_string(), StartupData::None, state, ext);
worker
let result = worker
.execute_mod_async(&module_specifier, None, false)
.then(|result| {
if let Err(err) = result {
eprintln!("execute_mod err {:?}", err);
}
tokio_util::panic_on_error(worker)
})
.await
.await;
if let Err(err) = result {
eprintln!("execute_mod err {:?}", err);
}
tokio_util::panic_on_error(worker).await
});

let metrics = &state_.metrics;
Expand Down Expand Up @@ -328,15 +326,14 @@ mod tests {
ext,
);
worker.execute("denoMain()").unwrap();
worker
let result = worker
.execute_mod_async(&module_specifier, None, false)
.then(|result| {
if let Err(err) = result {
eprintln!("execute_mod err {:?}", err);
}
tokio_util::panic_on_error(worker)
})
.await
.await;

if let Err(err) = result {
eprintln!("execute_mod err {:?}", err);
}
tokio_util::panic_on_error(worker).await
});

assert_eq!(state_.metrics.resolve_count.load(Ordering::SeqCst), 3);
Expand Down Expand Up @@ -386,14 +383,13 @@ mod tests {

let worker_ = worker.clone();

tokio::spawn(
worker
.then(move |r| {
r.unwrap();
futures::future::ok(())
})
.compat(),
);
let fut = async move {
let r = worker.await;
r.unwrap();
Ok(())
};

tokio::spawn(fut.boxed().compat());

let msg = json!("hi").to_string().into_boxed_str().into_boxed_bytes();

Expand Down

0 comments on commit 363b968

Please sign in to comment.