From 9845361153f35f6a68a82eb3a13845fddbeab026 Mon Sep 17 00:00:00 2001 From: Matt Mastracci Date: Sun, 14 May 2023 15:40:01 -0600 Subject: refactor(core): bake single-thread assumptions into spawn/spawn_blocking (#19056) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Partially supersedes #19016. This migrates `spawn` and `spawn_blocking` to `deno_core`, and removes the requirement for `spawn` tasks to be `Send` given our single-threaded executor. While we don't need to technically do anything w/`spawn_blocking`, this allows us to have a single `JoinHandle` type that works for both cases, and allows us to more easily experiment with alternative `spawn_blocking` implementations that do not require tokio (ie: rayon). Async ops (+~35%): Before: ``` time 1310 ms rate 763358 time 1267 ms rate 789265 time 1259 ms rate 794281 time 1266 ms rate 789889 ``` After: ``` time 956 ms rate 1046025 time 954 ms rate 1048218 time 924 ms rate 1082251 time 920 ms rate 1086956 ``` HTTP serve (+~4.4%): Before: ``` Running 10s test @ http://localhost:4500 2 threads and 10 connections Thread Stats Avg Stdev Max +/- Stdev Latency 68.78us 19.77us 1.43ms 86.84% Req/Sec 68.78k 5.00k 73.84k 91.58% 1381833 requests in 10.10s, 167.36MB read Requests/sec: 136823.29 Transfer/sec: 16.57MB ``` After: ``` Running 10s test @ http://localhost:4500 2 threads and 10 connections Thread Stats Avg Stdev Max +/- Stdev Latency 63.12us 17.43us 1.11ms 85.13% Req/Sec 71.82k 3.71k 77.02k 79.21% 1443195 requests in 10.10s, 174.79MB read Requests/sec: 142921.99 Transfer/sec: 17.31MB ``` Suggested-By: alice@ryhl.io Co-authored-by: Bartek Iwańczuk --- cli/tools/bench.rs | 14 ++++++++------ cli/tools/fmt.rs | 3 ++- cli/tools/repl/mod.rs | 3 ++- cli/tools/task.rs | 14 +++++++++----- cli/tools/test.rs | 24 +++++++++++++----------- cli/tools/upgrade.rs | 3 ++- 6 files changed, 36 insertions(+), 25 deletions(-) (limited to 'cli/tools') diff --git a/cli/tools/bench.rs b/cli/tools/bench.rs index 3d5f99aba..107fd2b9b 100644 --- a/cli/tools/bench.rs +++ b/cli/tools/bench.rs @@ -27,11 +27,13 @@ use deno_core::futures::FutureExt; use deno_core::futures::StreamExt; use deno_core::located_script_name; use deno_core::serde_v8; +use deno_core::task::spawn; +use deno_core::task::spawn_blocking; use deno_core::v8; use deno_core::ModuleSpecifier; use deno_runtime::permissions::Permissions; use deno_runtime::permissions::PermissionsContainer; -use deno_runtime::tokio_util::run_local; +use deno_runtime::tokio_util::create_and_run_current_thread; use indexmap::IndexMap; use indexmap::IndexSet; use log::Level; @@ -436,7 +438,7 @@ async fn check_specifiers( /// Run a single specifier as an executable bench module. async fn bench_specifier( - worker_factory: &CliMainWorkerFactory, + worker_factory: Arc, permissions: Permissions, specifier: ModuleSpecifier, sender: UnboundedSender, @@ -522,15 +524,15 @@ async fn bench_specifiers( let specifier = specifier; let sender = sender.clone(); let options = option_for_handles.clone(); - tokio::task::spawn_blocking(move || { + spawn_blocking(move || { let future = bench_specifier( - &worker_factory, + worker_factory, permissions, specifier, sender, options.filter, ); - run_local(future) + create_and_run_current_thread(future) }) }); @@ -539,7 +541,7 @@ async fn bench_specifiers( .collect::, tokio::task::JoinError>>>(); let handler = { - tokio::task::spawn(async move { + spawn(async move { let mut used_only = false; let mut report = BenchReport::new(); let mut reporter = diff --git a/cli/tools/fmt.rs b/cli/tools/fmt.rs index 70d2bd639..f2fec9302 100644 --- a/cli/tools/fmt.rs +++ b/cli/tools/fmt.rs @@ -28,6 +28,7 @@ use deno_core::error::generic_error; use deno_core::error::AnyError; use deno_core::futures; use deno_core::parking_lot::Mutex; +use deno_core::task::spawn_blocking; use log::debug; use log::info; use log::warn; @@ -629,7 +630,7 @@ where let handles = file_paths.iter().map(|file_path| { let f = f.clone(); let file_path = file_path.clone(); - tokio::task::spawn_blocking(move || f(file_path)) + spawn_blocking(move || f(file_path)) }); let join_results = futures::future::join_all(handles).await; diff --git a/cli/tools/repl/mod.rs b/cli/tools/repl/mod.rs index 9f4b58919..dfd9931b8 100644 --- a/cli/tools/repl/mod.rs +++ b/cli/tools/repl/mod.rs @@ -8,6 +8,7 @@ use crate::factory::CliFactory; use crate::file_fetcher::FileFetcher; use deno_core::error::AnyError; use deno_core::futures::StreamExt; +use deno_core::task::spawn_blocking; use deno_runtime::permissions::Permissions; use deno_runtime::permissions::PermissionsContainer; use rustyline::error::ReadlineError; @@ -32,7 +33,7 @@ async fn read_line_and_poll( editor: ReplEditor, ) -> Result { #![allow(clippy::await_holding_refcell_ref)] - let mut line_fut = tokio::task::spawn_blocking(move || editor.readline()); + let mut line_fut = spawn_blocking(move || editor.readline()); let mut poll_worker = true; let notifications_rc = repl_session.notifications.clone(); let mut notifications = notifications_rc.borrow_mut(); diff --git a/cli/tools/task.rs b/cli/tools/task.rs index bf972e2db..37a1aa1c9 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -21,6 +21,7 @@ use indexmap::IndexMap; use std::collections::HashMap; use std::path::PathBuf; use std::rc::Rc; +use tokio::task::LocalSet; pub async fn execute_script( flags: Flags, @@ -59,9 +60,10 @@ pub async fn execute_script( let seq_list = deno_task_shell::parser::parse(&script) .with_context(|| format!("Error parsing script '{task_name}'."))?; let env_vars = collect_env_vars(); - let exit_code = - deno_task_shell::execute(seq_list, env_vars, &cwd, Default::default()) - .await; + let local = LocalSet::new(); + let future = + deno_task_shell::execute(seq_list, env_vars, &cwd, Default::default()); + let exit_code = local.run_until(future).await; Ok(exit_code) } else if let Some(script) = package_json_scripts.get(task_name) { let package_json_deps_provider = factory.package_json_deps_provider(); @@ -109,8 +111,10 @@ pub async fn execute_script( .with_context(|| format!("Error parsing script '{task_name}'."))?; let npx_commands = resolve_npm_commands(npm_resolver, node_resolver)?; let env_vars = collect_env_vars(); - let exit_code = - deno_task_shell::execute(seq_list, env_vars, &cwd, npx_commands).await; + let local = LocalSet::new(); + let future = + deno_task_shell::execute(seq_list, env_vars, &cwd, npx_commands); + let exit_code = local.run_until(future).await; Ok(exit_code) } else { eprintln!("Task not found: {task_name}"); diff --git a/cli/tools/test.rs b/cli/tools/test.rs index 50e220a46..f78e32539 100644 --- a/cli/tools/test.rs +++ b/cli/tools/test.rs @@ -34,6 +34,8 @@ use deno_core::futures::StreamExt; use deno_core::located_script_name; use deno_core::parking_lot::Mutex; use deno_core::serde_v8; +use deno_core::task::spawn; +use deno_core::task::spawn_blocking; use deno_core::url::Url; use deno_core::v8; use deno_core::ModuleSpecifier; @@ -42,7 +44,7 @@ use deno_runtime::deno_io::StdioPipe; use deno_runtime::fmt_errors::format_js_error; use deno_runtime::permissions::Permissions; use deno_runtime::permissions::PermissionsContainer; -use deno_runtime::tokio_util::run_local; +use deno_runtime::tokio_util::create_and_run_current_thread; use indexmap::IndexMap; use indexmap::IndexSet; use log::Level; @@ -916,12 +918,12 @@ pub fn format_test_error(js_error: &JsError) -> String { /// Test a single specifier as documentation containing test programs, an executable test module or /// both. pub async fn test_specifier( - worker_factory: &CliMainWorkerFactory, + worker_factory: Arc, permissions: Permissions, specifier: ModuleSpecifier, mut sender: TestEventSender, fail_fast_tracker: FailFastTracker, - options: &TestSpecifierOptions, + options: TestSpecifierOptions, ) -> Result<(), AnyError> { if fail_fast_tracker.should_stop() { return Ok(()); @@ -1316,7 +1318,7 @@ async fn test_specifiers( let concurrent_jobs = options.concurrent_jobs; let sender_ = sender.downgrade(); - let sigint_handler_handle = tokio::task::spawn(async move { + let sigint_handler_handle = spawn(async move { signal::ctrl_c().await.unwrap(); sender_.upgrade().map(|s| s.send(TestEvent::Sigint).ok()); }); @@ -1328,14 +1330,14 @@ async fn test_specifiers( let sender = sender.clone(); let fail_fast_tracker = FailFastTracker::new(options.fail_fast); let specifier_options = options.specifier.clone(); - tokio::task::spawn_blocking(move || { - run_local(test_specifier( - &worker_factory, + spawn_blocking(move || { + create_and_run_current_thread(test_specifier( + worker_factory, permissions, specifier, sender.clone(), fail_fast_tracker, - &specifier_options, + specifier_options, )) }) }); @@ -1350,7 +1352,7 @@ async fn test_specifiers( )); let handler = { - tokio::task::spawn(async move { + spawn(async move { let earlier = Instant::now(); let mut tests = IndexMap::new(); let mut test_steps = IndexMap::new(); @@ -1887,7 +1889,7 @@ pub async fn run_tests_with_watch( // run, a process-scoped basic exit handler is required due to a tokio // limitation where it doesn't unbind its own handler for the entire process // once a user adds one. - tokio::task::spawn(async move { + spawn(async move { loop { signal::ctrl_c().await.unwrap(); if !HAS_TEST_RUN_SIGINT_HANDLER.load(Ordering::Relaxed) { @@ -2070,7 +2072,7 @@ fn start_output_redirect_thread( sender: UnboundedSender, flush_state: Arc>>>, ) { - tokio::task::spawn_blocking(move || loop { + spawn_blocking(move || loop { let mut buffer = [0; 512]; let size = match pipe_reader.read(&mut buffer) { Ok(0) | Err(_) => break, diff --git a/cli/tools/upgrade.rs b/cli/tools/upgrade.rs index b5aefe479..cbd924755 100644 --- a/cli/tools/upgrade.rs +++ b/cli/tools/upgrade.rs @@ -17,6 +17,7 @@ use deno_core::anyhow::Context; use deno_core::error::AnyError; use deno_core::futures::future::BoxFuture; use deno_core::futures::FutureExt; +use deno_core::task::spawn; use deno_semver::Version; use once_cell::sync::Lazy; use std::borrow::Cow; @@ -198,7 +199,7 @@ pub fn check_for_upgrades( if update_checker.should_check_for_new_version() { let env = update_checker.env.clone(); // do this asynchronously on a separate task - tokio::spawn(async move { + spawn(async move { // Sleep for a small amount of time to not unnecessarily impact startup // time. tokio::time::sleep(UPGRADE_CHECK_FETCH_DELAY).await; -- cgit v1.2.3