From 1564595f6bb21e477ec3fdb91aa2c8a4bba31b6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sun, 16 Apr 2023 16:05:13 +0200 Subject: [PATCH] =?UTF-8?q?Revert=20"perf(core):=20immediately=20schedule?= =?UTF-8?q?=20another=20tick=20if=20there=20are=20un=E2=80=A6=20(#18718)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …polled ops (#18611)" This reverts commit 4317055a3e49c7a75648480bbc86e4b466c60b5e. Reverting because we discovered while working on https://github.com/denoland/deno/pull/18619 that it doesn't work correctly (sometimes waker just stops working). --- .../workers/permissions_data_local.ts.out | 3 +- core/runtime.rs | 272 +++++++++--------- 2 files changed, 134 insertions(+), 141 deletions(-) diff --git a/cli/tests/testdata/workers/permissions_data_local.ts.out b/cli/tests/testdata/workers/permissions_data_local.ts.out index 994ca98cb1..4e0f727798 100644 --- a/cli/tests/testdata/workers/permissions_data_local.ts.out +++ b/cli/tests/testdata/workers/permissions_data_local.ts.out @@ -1,5 +1,4 @@ error: Uncaught (in worker "") Requires read access to "[WILDCARD]local_file.ts", run again with the --allow-read flag at data:application/javascript;base64,[WILDCARD]:1:8 error: Uncaught (in promise) Error: Unhandled error in child worker. - at Worker.#pollControl ([WILDCARD]) - at eventLoopTick (ext:core/01_core.js:[WILDCARD]) + at Worker.#pollControl[WILDCARD] diff --git a/core/runtime.rs b/core/runtime.rs index af6b37ed2f..bf321e0557 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -1182,161 +1182,155 @@ impl JsRuntime { state.waker.register(cx.waker()); } - loop { + if has_inspector { + // We poll the inspector first. + let _ = self.inspector().borrow_mut().poll_unpin(cx); + } + + self.pump_v8_message_loop()?; + + // Dynamic module loading - ie. modules loaded using "import()" + { + // Run in a loop so that dynamic imports that only depend on another + // dynamic import can be resolved in this event loop iteration. + // + // For example, a dynamically imported module like the following can be + // immediately resolved after `dependency.ts` is fully evaluated, but it + // wouldn't if not for this loop. + // + // await delay(1000); + // await import("./dependency.ts"); + // console.log("test") + // + loop { + let poll_imports = self.prepare_dyn_imports(cx)?; + assert!(poll_imports.is_ready()); + + let poll_imports = self.poll_dyn_imports(cx)?; + assert!(poll_imports.is_ready()); + + if !self.evaluate_dyn_imports() { + break; + } + } + } + + // Resolve async ops, run all next tick callbacks and macrotasks callbacks + // and only then check for any promise exceptions (`unhandledrejection` + // handlers are run in macrotasks callbacks so we need to let them run + // first). + self.do_js_event_loop_tick(cx)?; + self.check_promise_rejections()?; + + // Event loop middlewares + let mut maybe_scheduling = false; + { + let op_state = self.state.borrow().op_state.clone(); + for f in &self.event_loop_middlewares { + if f(op_state.clone(), cx) { + maybe_scheduling = true; + } + } + } + + // Top level module + self.evaluate_pending_module(); + + let pending_state = self.event_loop_pending_state(); + if !pending_state.is_pending() && !maybe_scheduling { if has_inspector { - // We poll the inspector first. - let _ = self.inspector().borrow_mut().poll_unpin(cx); - } + let inspector = self.inspector(); + let has_active_sessions = inspector.borrow().has_active_sessions(); + let has_blocking_sessions = inspector.borrow().has_blocking_sessions(); - self.pump_v8_message_loop()?; - - // Dynamic module loading - ie. modules loaded using "import()" - { - // Run in a loop so that dynamic imports that only depend on another - // dynamic import can be resolved in this event loop iteration. - // - // For example, a dynamically imported module like the following can be - // immediately resolved after `dependency.ts` is fully evaluated, but it - // wouldn't if not for this loop. - // - // await delay(1000); - // await import("./dependency.ts"); - // console.log("test") - // - loop { - let poll_imports = self.prepare_dyn_imports(cx)?; - assert!(poll_imports.is_ready()); - - let poll_imports = self.poll_dyn_imports(cx)?; - assert!(poll_imports.is_ready()); - - if !self.evaluate_dyn_imports() { - break; + if wait_for_inspector && has_active_sessions { + // If there are no blocking sessions (eg. REPL) we can now notify + // debugger that the program has finished running and we're ready + // to exit the process once debugger disconnects. + if !has_blocking_sessions { + let context = self.global_context(); + let scope = &mut self.handle_scope(); + inspector.borrow_mut().context_destroyed(scope, context); + println!("Program finished. Waiting for inspector to disconnect to exit the process..."); } + + return Poll::Pending; } } - // Resolve async ops, run all next tick callbacks and macrotasks callbacks - // and only then check for any promise exceptions (`unhandledrejection` - // handlers are run in macrotasks callbacks so we need to let them run - // first). - self.do_js_event_loop_tick(cx)?; - self.check_promise_rejections()?; + return Poll::Ready(Ok(())); + } - // Event loop middlewares - let mut maybe_scheduling = false; - { - let op_state = self.state.borrow().op_state.clone(); - for f in &self.event_loop_middlewares { - if f(op_state.clone(), cx) { - maybe_scheduling = true; - } - } - } + let state = self.state.borrow(); - // Top level module - self.evaluate_pending_module(); + // Check if more async ops have been dispatched + // during this turn of event loop. + // If there are any pending background tasks, we also wake the runtime to + // make sure we don't miss them. + // TODO(andreubotella) The event loop will spin as long as there are pending + // background tasks. We should look into having V8 notify us when a + // background task is done. + if state.have_unpolled_ops + || pending_state.has_pending_background_tasks + || pending_state.has_tick_scheduled + || maybe_scheduling + { + state.waker.wake(); + } - let pending_state = self.event_loop_pending_state(); - if !pending_state.is_pending() && !maybe_scheduling { - if has_inspector { - let inspector = self.inspector(); - let has_active_sessions = inspector.borrow().has_active_sessions(); - let has_blocking_sessions = - inspector.borrow().has_blocking_sessions(); + drop(state); - if wait_for_inspector && has_active_sessions { - // If there are no blocking sessions (eg. REPL) we can now notify - // debugger that the program has finished running and we're ready - // to exit the process once debugger disconnects. - if !has_blocking_sessions { - let context = self.global_context(); - let scope = &mut self.handle_scope(); - inspector.borrow_mut().context_destroyed(scope, context); - println!("Program finished. Waiting for inspector to disconnect to exit the process..."); - } - - return Poll::Pending; - } - } - - return Poll::Ready(Ok(())); - } - - let state = self.state.borrow(); - - // Check if more async ops have been dispatched during this turn of - // event loop. In such case immediately do another turn of the event loop. - if state.have_unpolled_ops { - continue; - } - - // If there are any pending background tasks, we also wake the runtime to - // make sure we don't miss them. - // TODO(andreubotella) The event loop will spin as long as there are pending - // background tasks. We should look into having V8 notify us when a - // background task is done. - if pending_state.has_pending_background_tasks + if pending_state.has_pending_module_evaluation { + if pending_state.has_pending_refed_ops + || pending_state.has_pending_dyn_imports + || pending_state.has_pending_dyn_module_evaluation + || pending_state.has_pending_background_tasks || pending_state.has_tick_scheduled || maybe_scheduling { + // pass, will be polled again + } else { + let scope = &mut self.handle_scope(); + let messages = find_stalled_top_level_await(scope); + // We are gonna print only a single message to provide a nice formatting + // with source line of offending promise shown. Once user fixed it, then + // they will get another error message for the next promise (but this + // situation is gonna be very rare, if ever happening). + assert!(!messages.is_empty()); + let msg = v8::Local::new(scope, messages[0].clone()); + let js_error = JsError::from_v8_message(scope, msg); + return Poll::Ready(Err(js_error.into())); + } + } + + if pending_state.has_pending_dyn_module_evaluation { + if pending_state.has_pending_refed_ops + || pending_state.has_pending_dyn_imports + || pending_state.has_pending_background_tasks + || pending_state.has_tick_scheduled + { + // pass, will be polled again + } else if self.state.borrow().dyn_module_evaluate_idle_counter >= 1 { + let scope = &mut self.handle_scope(); + let messages = find_stalled_top_level_await(scope); + // We are gonna print only a single message to provide a nice formatting + // with source line of offending promise shown. Once user fixed it, then + // they will get another error message for the next promise (but this + // situation is gonna be very rare, if ever happening). + assert!(!messages.is_empty()); + let msg = v8::Local::new(scope, messages[0].clone()); + let js_error = JsError::from_v8_message(scope, msg); + return Poll::Ready(Err(js_error.into())); + } else { + let mut state = self.state.borrow_mut(); + // Delay the above error by one spin of the event loop. A dynamic import + // evaluation may complete during this, in which case the counter will + // reset. + state.dyn_module_evaluate_idle_counter += 1; state.waker.wake(); } - - drop(state); - - if pending_state.has_pending_module_evaluation { - if pending_state.has_pending_refed_ops - || pending_state.has_pending_dyn_imports - || pending_state.has_pending_dyn_module_evaluation - || pending_state.has_pending_background_tasks - || pending_state.has_tick_scheduled - || maybe_scheduling - { - // pass, will be polled again - } else { - let scope = &mut self.handle_scope(); - let messages = find_stalled_top_level_await(scope); - // We are gonna print only a single message to provide a nice formatting - // with source line of offending promise shown. Once user fixed it, then - // they will get another error message for the next promise (but this - // situation is gonna be very rare, if ever happening). - assert!(!messages.is_empty()); - let msg = v8::Local::new(scope, messages[0].clone()); - let js_error = JsError::from_v8_message(scope, msg); - return Poll::Ready(Err(js_error.into())); - } - } - - if pending_state.has_pending_dyn_module_evaluation { - if pending_state.has_pending_refed_ops - || pending_state.has_pending_dyn_imports - || pending_state.has_pending_background_tasks - || pending_state.has_tick_scheduled - { - // pass, will be polled again - } else if self.state.borrow().dyn_module_evaluate_idle_counter >= 1 { - let scope = &mut self.handle_scope(); - let messages = find_stalled_top_level_await(scope); - // We are gonna print only a single message to provide a nice formatting - // with source line of offending promise shown. Once user fixed it, then - // they will get another error message for the next promise (but this - // situation is gonna be very rare, if ever happening). - assert!(!messages.is_empty()); - let msg = v8::Local::new(scope, messages[0].clone()); - let js_error = JsError::from_v8_message(scope, msg); - return Poll::Ready(Err(js_error.into())); - } else { - let mut state = self.state.borrow_mut(); - // Delay the above error by one spin of the event loop. A dynamic import - // evaluation may complete during this, in which case the counter will - // reset. - state.dyn_module_evaluate_idle_counter += 1; - state.waker.wake(); - } - } - break; } + Poll::Pending }