Fix 100% CPU idling problem by reverting #7672 (#7911)

* Revert "refactor: Worker is not a Future (#7895)"

This reverts commit f4357f0ff9.

* Revert "refactor(core): JsRuntime is not a Future (#7855)"

This reverts commit d8879feb8c.

* Revert "fix(core): module execution with top level await (#7672)"

This reverts commit c7c7677825.
This commit is contained in:
Ryan Dahl 2020-10-10 05:41:11 -04:00 committed by GitHub
parent 782e6a2ed5
commit 08bb8b3d53
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 225 additions and 495 deletions

View file

@ -1,6 +1,7 @@
// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
use crate::colors;
use crate::inspector::DenoInspector;
use crate::inspector::InspectorSession;
use deno_core::error::AnyError;
use deno_core::serde_json;
@ -13,7 +14,8 @@ pub struct CoverageCollector {
}
impl CoverageCollector {
pub fn new(session: Box<InspectorSession>) -> Self {
pub fn new(inspector_ptr: *mut DenoInspector) -> Self {
let session = InspectorSession::new(inspector_ptr);
Self { session }
}

View file

@ -275,7 +275,7 @@ async fn eval_command(
debug!("main_module {}", &main_module);
worker.execute_module(&main_module).await?;
worker.execute("window.dispatchEvent(new Event('load'))")?;
worker.run_event_loop().await?;
(&mut *worker).await?;
worker.execute("window.dispatchEvent(new Event('unload'))")?;
Ok(())
}
@ -423,7 +423,7 @@ async fn run_repl(flags: Flags) -> Result<(), AnyError> {
ModuleSpecifier::resolve_url_or_path("./$deno$repl.ts").unwrap();
let global_state = GlobalState::new(flags)?;
let mut worker = MainWorker::new(&global_state, main_module.clone());
worker.run_event_loop().await?;
(&mut *worker).await?;
repl::run(&global_state, worker).await
}
@ -454,7 +454,7 @@ async fn run_from_stdin(flags: Flags) -> Result<(), AnyError> {
debug!("main_module {}", main_module);
worker.execute_module(&main_module).await?;
worker.execute("window.dispatchEvent(new Event('load'))")?;
worker.run_event_loop().await?;
(&mut *worker).await?;
worker.execute("window.dispatchEvent(new Event('unload'))")?;
Ok(())
}
@ -500,7 +500,7 @@ async fn run_with_watch(flags: Flags, script: String) -> Result<(), AnyError> {
debug!("main_module {}", main_module);
worker.execute_module(&main_module).await?;
worker.execute("window.dispatchEvent(new Event('load'))")?;
worker.run_event_loop().await?;
(&mut *worker).await?;
worker.execute("window.dispatchEvent(new Event('unload'))")?;
Ok(())
}
@ -525,7 +525,7 @@ async fn run_command(flags: Flags, script: String) -> Result<(), AnyError> {
debug!("main_module {}", main_module);
worker.execute_module(&main_module).await?;
worker.execute("window.dispatchEvent(new Event('load'))")?;
worker.run_event_loop().await?;
(&mut *worker).await?;
worker.execute("window.dispatchEvent(new Event('unload'))")?;
Ok(())
}
@ -578,8 +578,12 @@ async fn test_command(
.save_source_file_in_cache(&main_module, source_file);
let mut maybe_coverage_collector = if flags.coverage {
let session = worker.create_inspector_session();
let mut coverage_collector = CoverageCollector::new(session);
let inspector = worker
.inspector
.as_mut()
.expect("Inspector is not created.");
let mut coverage_collector = CoverageCollector::new(&mut **inspector);
coverage_collector.start_collecting().await?;
Some(coverage_collector)
@ -590,9 +594,9 @@ async fn test_command(
let execute_result = worker.execute_module(&main_module).await;
execute_result?;
worker.execute("window.dispatchEvent(new Event('load'))")?;
worker.run_event_loop().await?;
(&mut *worker).await?;
worker.execute("window.dispatchEvent(new Event('unload'))")?;
worker.run_event_loop().await?;
(&mut *worker).await?;
if let Some(coverage_collector) = maybe_coverage_collector.as_mut() {
let coverages = coverage_collector.collect().await?;

View file

@ -155,13 +155,6 @@ fn run_worker_thread(
if let Err(e) = result {
let mut sender = worker.internal_channels.sender.clone();
// If sender is closed it means that worker has already been closed from
// within using "globalThis.close()"
if sender.is_closed() {
return;
}
sender
.try_send(WorkerEvent::TerminalError(e))
.expect("Failed to post message to host");
@ -173,8 +166,7 @@ fn run_worker_thread(
// TODO(bartlomieju): this thread should return result of event loop
// that means that we should store JoinHandle to thread to ensure
// that it actually terminates.
rt.block_on(worker.run_event_loop())
.expect("Panic in event loop");
rt.block_on(worker).expect("Panic in event loop");
debug!("Worker thread shuts down {}", &name);
})?;

View file

@ -47,7 +47,7 @@ async fn post_message_and_poll(
return result
}
_ = worker.run_event_loop() => {
_ = &mut *worker => {
// A zero delay is long enough to yield the thread in order to prevent the loop from
// running hot for messages that are taking longer to resolve like for example an
// evaluation of top level await.
@ -75,7 +75,7 @@ async fn read_line_and_poll(
result = &mut line => {
return result.unwrap();
}
_ = worker.run_event_loop(), if poll_worker => {
_ = &mut *worker, if poll_worker => {
poll_worker = false;
}
_ = &mut timeout => {
@ -92,7 +92,12 @@ pub async fn run(
// Our inspector is unable to default to the default context id so we have to specify it here.
let context_id: u32 = 1;
let mut session = worker.create_inspector_session();
let inspector = worker
.inspector
.as_mut()
.expect("Inspector is not created.");
let mut session = InspectorSession::new(&mut **inspector);
let history_file = global_state.dir.root.join("deno_history.txt");

View file

@ -2662,16 +2662,6 @@ itest!(ignore_require {
exit_code: 0,
});
itest!(top_level_await_bug {
args: "run --allow-read top_level_await_bug.js",
output: "top_level_await_bug.out",
});
itest!(top_level_await_bug2 {
args: "run --allow-read top_level_await_bug2.js",
output: "top_level_await_bug2.out",
});
#[test]
fn cafile_env_fetch() {
use deno_core::url::Url;

View file

@ -1,2 +0,0 @@
const mod = await import("./top_level_await_bug_nested.js");
console.log(mod);

View file

@ -1 +0,0 @@
Module { default: 1, [Symbol(Symbol.toStringTag)]: "Module" }

View file

@ -1,15 +0,0 @@
const mod = await import("./top_level_await_bug_nested.js");
console.log(mod);
const sleep = (n) => new Promise((r) => setTimeout(r, n));
await sleep(100);
console.log("slept");
window.addEventListener("load", () => {
console.log("load event");
});
setTimeout(() => {
console.log("timeout");
}, 1000);

View file

@ -1,4 +0,0 @@
Module { default: 1, [Symbol(Symbol.toStringTag)]: "Module" }
slept
load event
timeout

View file

@ -1,5 +0,0 @@
const sleep = (n) => new Promise((r) => setTimeout(r, n));
await sleep(100);
export default 1;

View file

@ -3,7 +3,6 @@
use crate::fmt_errors::JsError;
use crate::global_state::GlobalState;
use crate::inspector::DenoInspector;
use crate::inspector::InspectorSession;
use crate::js;
use crate::metrics::Metrics;
use crate::ops;
@ -12,7 +11,6 @@ use crate::permissions::Permissions;
use crate::state::CliModuleLoader;
use deno_core::error::AnyError;
use deno_core::futures::channel::mpsc;
use deno_core::futures::future::poll_fn;
use deno_core::futures::future::FutureExt;
use deno_core::futures::stream::StreamExt;
use deno_core::futures::task::AtomicWaker;
@ -24,8 +22,10 @@ use deno_core::ModuleSpecifier;
use deno_core::RuntimeOptions;
use deno_core::Snapshot;
use std::env;
use std::future::Future;
use std::ops::Deref;
use std::ops::DerefMut;
use std::pin::Pin;
use std::rc::Rc;
use std::sync::atomic::AtomicBool;
use std::sync::atomic::Ordering;
@ -95,15 +95,13 @@ fn create_channels() -> (WorkerChannelsInternal, WorkerHandle) {
/// - `MainWorker`
/// - `WebWorker`
pub struct Worker {
external_channels: WorkerHandle,
inspector: Option<Box<DenoInspector>>,
// Following fields are pub because they are accessed
// when creating a new WebWorker instance.
pub name: String,
pub js_runtime: JsRuntime,
pub inspector: Option<Box<DenoInspector>>,
pub waker: AtomicWaker,
pub(crate) internal_channels: WorkerChannelsInternal,
pub(crate) js_runtime: JsRuntime,
pub(crate) name: String,
external_channels: WorkerHandle,
should_break_on_first_statement: bool,
waker: AtomicWaker,
}
impl Worker {
@ -149,13 +147,13 @@ impl Worker {
let (internal_channels, external_channels) = create_channels();
Self {
external_channels,
inspector,
internal_channels,
js_runtime,
name,
should_break_on_first_statement,
js_runtime,
inspector,
waker: AtomicWaker::new(),
internal_channels,
external_channels,
should_break_on_first_statement,
}
}
@ -191,7 +189,7 @@ impl Worker {
) -> Result<(), AnyError> {
let id = self.preload_module(module_specifier).await?;
self.wait_for_inspector_session();
self.js_runtime.mod_evaluate(id).await
self.js_runtime.mod_evaluate(id)
}
/// Loads, instantiates and executes provided source code
@ -206,7 +204,7 @@ impl Worker {
.load_module(module_specifier, Some(code))
.await?;
self.wait_for_inspector_session();
self.js_runtime.mod_evaluate(id).await
self.js_runtime.mod_evaluate(id)
}
/// Returns a way to communicate with the Worker from other threads.
@ -223,28 +221,6 @@ impl Worker {
.wait_for_session_and_break_on_next_statement()
}
}
/// Create new inspector session. This function panics if Worker
/// was not configured to create inspector.
pub fn create_inspector_session(&mut self) -> Box<InspectorSession> {
let inspector = self.inspector.as_mut().unwrap();
InspectorSession::new(&mut **inspector)
}
pub fn poll_event_loop(
&mut self,
cx: &mut Context,
) -> Poll<Result<(), AnyError>> {
// We always poll the inspector if it exists.
let _ = self.inspector.as_mut().map(|i| i.poll_unpin(cx));
self.waker.register(cx.waker());
self.js_runtime.poll_event_loop(cx)
}
pub async fn run_event_loop(&mut self) -> Result<(), AnyError> {
poll_fn(|cx| self.poll_event_loop(cx)).await
}
}
impl Drop for Worker {
@ -255,6 +231,32 @@ impl Drop for Worker {
}
}
impl Future for Worker {
type Output = Result<(), AnyError>;
fn poll(self: Pin<&mut Self>, cx: &mut Context) -> Poll<Self::Output> {
let inner = self.get_mut();
// We always poll the inspector if it exists.
let _ = inner.inspector.as_mut().map(|i| i.poll_unpin(cx));
inner.waker.register(cx.waker());
inner.js_runtime.poll_unpin(cx)
}
}
impl Deref for Worker {
type Target = JsRuntime;
fn deref(&self) -> &Self::Target {
&self.js_runtime
}
}
impl DerefMut for Worker {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.js_runtime
}
}
/// This worker is created and used by Deno executable.
///
/// It provides ops available in the `Deno` namespace.
@ -276,46 +278,45 @@ impl MainWorker {
loader,
true,
);
let js_runtime = &mut worker.js_runtime;
{
// All ops registered in this function depend on these
{
let op_state = js_runtime.op_state();
let op_state = worker.op_state();
let mut op_state = op_state.borrow_mut();
op_state.put::<Metrics>(Default::default());
op_state.put::<Arc<GlobalState>>(global_state.clone());
op_state.put::<Permissions>(global_state.permissions.clone());
}
ops::runtime::init(js_runtime, main_module);
ops::fetch::init(js_runtime, global_state.flags.ca_file.as_deref());
ops::timers::init(js_runtime);
ops::worker_host::init(js_runtime);
ops::random::init(js_runtime, global_state.flags.seed);
ops::reg_json_sync(js_runtime, "op_close", deno_core::op_close);
ops::reg_json_sync(js_runtime, "op_resources", deno_core::op_resources);
ops::runtime::init(&mut worker, main_module);
ops::fetch::init(&mut worker, global_state.flags.ca_file.as_deref());
ops::timers::init(&mut worker);
ops::worker_host::init(&mut worker);
ops::random::init(&mut worker, global_state.flags.seed);
ops::reg_json_sync(&mut worker, "op_close", deno_core::op_close);
ops::reg_json_sync(&mut worker, "op_resources", deno_core::op_resources);
ops::reg_json_sync(
js_runtime,
&mut worker,
"op_domain_to_ascii",
deno_web::op_domain_to_ascii,
);
ops::errors::init(js_runtime);
ops::fs_events::init(js_runtime);
ops::fs::init(js_runtime);
ops::io::init(js_runtime);
ops::net::init(js_runtime);
ops::os::init(js_runtime);
ops::permissions::init(js_runtime);
ops::plugin::init(js_runtime);
ops::process::init(js_runtime);
ops::runtime_compiler::init(js_runtime);
ops::signal::init(js_runtime);
ops::tls::init(js_runtime);
ops::tty::init(js_runtime);
ops::websocket::init(js_runtime);
ops::errors::init(&mut worker);
ops::fs_events::init(&mut worker);
ops::fs::init(&mut worker);
ops::io::init(&mut worker);
ops::net::init(&mut worker);
ops::os::init(&mut worker);
ops::permissions::init(&mut worker);
ops::plugin::init(&mut worker);
ops::process::init(&mut worker);
ops::runtime_compiler::init(&mut worker);
ops::signal::init(&mut worker);
ops::tls::init(&mut worker);
ops::tty::init(&mut worker);
ops::websocket::init(&mut worker);
}
{
let op_state = js_runtime.op_state();
let op_state = worker.op_state();
let mut op_state = op_state.borrow_mut();
let t = &mut op_state.resource_table;
let (stdin, stdout, stderr) = get_stdio();
@ -448,45 +449,49 @@ impl WebWorker {
{
let handle = web_worker.thread_safe_handle();
let sender = web_worker.worker.internal_channels.sender.clone();
let js_runtime = &mut web_worker.js_runtime;
// All ops registered in this function depend on these
{
let op_state = js_runtime.op_state();
let op_state = web_worker.op_state();
let mut op_state = op_state.borrow_mut();
op_state.put::<Metrics>(Default::default());
op_state.put::<Arc<GlobalState>>(global_state.clone());
op_state.put::<Permissions>(permissions);
}
ops::web_worker::init(js_runtime, sender, handle);
ops::runtime::init(js_runtime, main_module);
ops::fetch::init(js_runtime, global_state.flags.ca_file.as_deref());
ops::timers::init(js_runtime);
ops::worker_host::init(js_runtime);
ops::reg_json_sync(js_runtime, "op_close", deno_core::op_close);
ops::reg_json_sync(js_runtime, "op_resources", deno_core::op_resources);
ops::web_worker::init(&mut web_worker, sender, handle);
ops::runtime::init(&mut web_worker, main_module);
ops::fetch::init(&mut web_worker, global_state.flags.ca_file.as_deref());
ops::timers::init(&mut web_worker);
ops::worker_host::init(&mut web_worker);
ops::reg_json_sync(&mut web_worker, "op_close", deno_core::op_close);
ops::reg_json_sync(
js_runtime,
&mut web_worker,
"op_resources",
deno_core::op_resources,
);
ops::reg_json_sync(
&mut web_worker,
"op_domain_to_ascii",
deno_web::op_domain_to_ascii,
);
ops::errors::init(js_runtime);
ops::io::init(js_runtime);
ops::websocket::init(js_runtime);
ops::errors::init(&mut web_worker);
ops::io::init(&mut web_worker);
ops::websocket::init(&mut web_worker);
if has_deno_namespace {
ops::fs_events::init(js_runtime);
ops::fs::init(js_runtime);
ops::net::init(js_runtime);
ops::os::init(js_runtime);
ops::permissions::init(js_runtime);
ops::plugin::init(js_runtime);
ops::process::init(js_runtime);
ops::random::init(js_runtime, global_state.flags.seed);
ops::runtime_compiler::init(js_runtime);
ops::signal::init(js_runtime);
ops::tls::init(js_runtime);
ops::tty::init(js_runtime);
ops::fs_events::init(&mut web_worker);
ops::fs::init(&mut web_worker);
ops::net::init(&mut web_worker);
ops::os::init(&mut web_worker);
ops::permissions::init(&mut web_worker);
ops::plugin::init(&mut web_worker);
ops::process::init(&mut web_worker);
ops::random::init(&mut web_worker, global_state.flags.seed);
ops::runtime_compiler::init(&mut web_worker);
ops::signal::init(&mut web_worker);
ops::tls::init(&mut web_worker);
ops::tty::init(&mut web_worker);
}
}
@ -499,27 +504,38 @@ impl WebWorker {
pub fn thread_safe_handle(&self) -> WebWorkerHandle {
self.handle.clone()
}
}
pub async fn run_event_loop(&mut self) -> Result<(), AnyError> {
poll_fn(|cx| self.poll_event_loop(cx)).await
impl Deref for WebWorker {
type Target = Worker;
fn deref(&self) -> &Self::Target {
&self.worker
}
}
pub fn poll_event_loop(
&mut self,
cx: &mut Context,
) -> Poll<Result<(), AnyError>> {
let worker = &mut self.worker;
impl DerefMut for WebWorker {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.worker
}
}
let terminated = self.handle.terminated.load(Ordering::Relaxed);
impl Future for WebWorker {
type Output = Result<(), AnyError>;
fn poll(self: Pin<&mut Self>, cx: &mut Context) -> Poll<Self::Output> {
let inner = self.get_mut();
let worker = &mut inner.worker;
let terminated = inner.handle.terminated.load(Ordering::Relaxed);
if terminated {
return Poll::Ready(Ok(()));
}
if !self.event_loop_idle {
match worker.poll_event_loop(cx) {
if !inner.event_loop_idle {
match worker.poll_unpin(cx) {
Poll::Ready(r) => {
let terminated = self.handle.terminated.load(Ordering::Relaxed);
let terminated = inner.handle.terminated.load(Ordering::Relaxed);
if terminated {
return Poll::Ready(Ok(()));
}
@ -530,13 +546,13 @@ impl WebWorker {
.try_send(WorkerEvent::Error(e))
.expect("Failed to post message to host");
}
self.event_loop_idle = true;
inner.event_loop_idle = true;
}
Poll::Pending => {}
}
}
if let Poll::Ready(r) = self.terminate_rx.poll_next_unpin(cx) {
if let Poll::Ready(r) = inner.terminate_rx.poll_next_unpin(cx) {
// terminate_rx should never be closed
assert!(r.is_some());
return Poll::Ready(Ok(()));
@ -553,7 +569,7 @@ impl WebWorker {
if let Err(e) = worker.execute(&script) {
// If execution was terminated during message callback then
// just ignore it
if self.handle.terminated.load(Ordering::Relaxed) {
if inner.handle.terminated.load(Ordering::Relaxed) {
return Poll::Ready(Ok(()));
}
@ -565,7 +581,7 @@ impl WebWorker {
}
// Let event loop be polled again
self.event_loop_idle = false;
inner.event_loop_idle = false;
worker.waker.wake();
}
None => unreachable!(),
@ -576,19 +592,6 @@ impl WebWorker {
}
}
impl Deref for WebWorker {
type Target = Worker;
fn deref(&self) -> &Self::Target {
&self.worker
}
}
impl DerefMut for WebWorker {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.worker
}
}
#[cfg(test)]
mod tests {
use super::*;
@ -625,7 +628,7 @@ mod tests {
if let Err(err) = result {
eprintln!("execute_mod err {:?}", err);
}
if let Err(e) = worker.run_event_loop().await {
if let Err(e) = (&mut *worker).await {
panic!("Future got unexpected error: {:?}", e);
}
}
@ -643,7 +646,7 @@ mod tests {
if let Err(err) = result {
eprintln!("execute_mod err {:?}", err);
}
if let Err(e) = worker.run_event_loop().await {
if let Err(e) = (&mut *worker).await {
panic!("Future got unexpected error: {:?}", e);
}
}
@ -662,7 +665,7 @@ mod tests {
if let Err(err) = result {
eprintln!("execute_mod err {:?}", err);
}
if let Err(e) = worker.run_event_loop().await {
if let Err(e) = (&mut *worker).await {
panic!("Future got unexpected error: {:?}", e);
}
}
@ -730,7 +733,7 @@ mod tests {
worker.execute(source).unwrap();
let handle = worker.thread_safe_handle();
handle_sender.send(handle).unwrap();
let r = tokio_util::run_basic(worker.run_event_loop());
let r = tokio_util::run_basic(worker);
assert!(r.is_ok())
});
@ -777,7 +780,7 @@ mod tests {
worker.execute("onmessage = () => { close(); }").unwrap();
let handle = worker.thread_safe_handle();
handle_sender.send(handle).unwrap();
let r = tokio_util::run_basic(worker.run_event_loop());
let r = tokio_util::run_basic(worker);
assert!(r.is_ok())
});