From 85cb13f274c2951d3b02916745923ee5e708ee0d Mon Sep 17 00:00:00 2001 From: hrmny <8845940+ForsakenHarmony@users.noreply.github.com> Date: Fri, 13 Sep 2024 16:05:14 +0200 Subject: [PATCH] get rid of `iter_custom` because codspeed doesn't support it --- .../crates/turbo-tasks-fs/benches/mod.rs | 38 +- turbopack/crates/turbopack-bench/src/lib.rs | 497 +++++++++--------- .../crates/turbopack-bench/src/util/mod.rs | 130 +---- .../turbopack-bench/src/util/page_guard.rs | 20 +- .../turbopack-bench/src/util/prepared_app.rs | 57 +- 5 files changed, 312 insertions(+), 430 deletions(-) diff --git a/turbopack/crates/turbo-tasks-fs/benches/mod.rs b/turbopack/crates/turbo-tasks-fs/benches/mod.rs index be3d045e473c4..e3421973fee9a 100644 --- a/turbopack/crates/turbo-tasks-fs/benches/mod.rs +++ b/turbopack/crates/turbo-tasks-fs/benches/mod.rs @@ -5,11 +5,8 @@ use std::{ time::{Duration, Instant}, }; -use codspeed_criterion_compat::{ - criterion_group, criterion_main, - measurement::{Measurement, WallTime}, - BenchmarkId, Criterion, -}; +use codspeed_criterion_compat::{criterion_group, criterion_main, BenchmarkId, Criterion}; +use criterion::BatchSize; use notify::{Config, RecommendedWatcher, RecursiveMode, Watcher}; use tokio::runtime::Runtime; use turbo_tasks::event::Event; @@ -48,26 +45,17 @@ fn bench_file_watching(c: &mut Criterion) { } }); - b.to_async(Runtime::new().unwrap()) - .iter_custom(move |iters| { - let event = event.clone(); - async move { - let m = WallTime; - let mut value = m.zero(); - for _ in 0..iters { - std::thread::sleep(Duration::from_millis(1)); - let l = event.listen(); - let path = temp_path.join("file.txt"); - let content = start.elapsed().as_micros().to_string(); - let s = m.start(); - fs::write(path, content).unwrap(); - l.await; - let duration = m.end(s); - value = m.add(&value, &duration); - } - value - } - }); + b.to_async(Runtime::new().unwrap()).iter_batched( + || event.clone(), + |event| async move { + let l = event.listen(); + let path = temp_path.join("file.txt"); + let content = start.elapsed().as_micros().to_string(); + fs::write(path, content).unwrap(); + l.await; + }, + BatchSize::SmallInput, + ); drop(watcher); t.join().unwrap(); diff --git a/turbopack/crates/turbopack-bench/src/lib.rs b/turbopack/crates/turbopack-bench/src/lib.rs index c24ac62ec798b..74a60831d8537 100644 --- a/turbopack/crates/turbopack-bench/src/lib.rs +++ b/turbopack/crates/turbopack-bench/src/lib.rs @@ -6,20 +6,20 @@ use std::{ atomic::{AtomicUsize, Ordering}, Arc, }, - time::Duration, + time::{Duration, Instant}, }; use anyhow::{anyhow, Context, Result}; +use chromiumoxide::Browser; use codspeed_criterion_compat::{ - measurement::{Measurement, WallTime}, - BenchmarkGroup, BenchmarkId, Criterion, + measurement::WallTime, BatchSize, Bencher, BenchmarkGroup, BenchmarkId, Criterion, }; use once_cell::sync::Lazy; use tokio::{ runtime::Runtime, + sync::Mutex, time::{sleep, timeout}, }; -use turbo_tasks::util::FormatDuration; use util::{ build_test, create_browser, env::{read_env, read_env_bool, read_env_list}, @@ -81,36 +81,27 @@ fn bench_startup_internal( } }; for module_count in get_module_counts() { - let test_app = Lazy::new(|| build_test(module_count, bundler.as_ref())); - let input = (bundler.as_ref(), &test_app); + let input = (bundler.as_ref(), module_count); resume_on_error(AssertUnwindSafe(|| { g.bench_with_input( BenchmarkId::new(bundler.get_name(), format!("{} modules", module_count)), &input, - |b, &(bundler, test_app)| { - let test_app = &**test_app; + |b, &(bundler, module_count)| { + let test_app = build_test(module_count, bundler); let browser = &*browser; - b.to_async(&runtime).try_iter_custom(|iters, m| async move { - let mut value = m.zero(); - - for _ in 0..iters { - let mut app = - PreparedApp::new(bundler, test_app.path().to_path_buf()) - .await?; - let start = m.start(); + + b.to_async(&runtime).try_iter_batched( + || PreparedApp::new(bundler, test_app.path().to_path_buf()), + |mut app| async move { app.start_server()?; - let mut guard = app.with_page(browser).await?; + let mut guard = app.open_page(browser).await?; if wait_for_hydration { guard.wait_for_hydration().await?; } - let duration = m.end(start); - value = m.add(&value, &duration); - - // Defer the dropping of the guard. - drop(guard); - } - Ok(value) - }); + Ok((app, guard)) + }, + BatchSize::PerIteration, + ); }, ); })); @@ -151,7 +142,6 @@ fn bench_hmr_internal( let runtime = Runtime::new().unwrap(); let browser = Lazy::new(|| runtime.block_on(create_browser())); - let hmr_warmup = read_env("TURBOPACK_BENCH_HMR_WARMUP", 10).unwrap(); for bundler in bundlers { if matches!( @@ -165,172 +155,22 @@ fn bench_hmr_internal( continue; } for module_count in get_module_counts() { - let test_app = Lazy::new(|| build_test(module_count, bundler.as_ref())); - let input = (bundler.as_ref(), &test_app); - let module_picker = - Lazy::new(|| Arc::new(ModulePicker::new(test_app.modules().to_vec()))); + let input = (bundler.as_ref(), module_count); resume_on_error(AssertUnwindSafe(|| { g.bench_with_input( BenchmarkId::new(bundler.get_name(), format!("{} modules", module_count)), &input, - |b, &(bundler, test_app)| { - let test_app = &**test_app; - let modules = test_app.modules(); - let module_picker = &*module_picker; - let browser = &*browser; - - let max_init_update_timeout = bundler.max_init_update_timeout(module_count); - let max_update_timeout = bundler.max_update_timeout(module_count); - - b.to_async(&runtime).try_iter_async( + |b, &(bundler, module_count)| { + bench_hmr_internal_bench( + b, + location, &runtime, - || async { - let mut app = PreparedApp::new_without_copy( - bundler, - test_app.path().to_path_buf(), - ) - .await?; - app.start_server()?; - let mut guard = app.with_page(browser).await?; - if bundler.has_hydration_event() { - guard.wait_for_hydration().await?; - } else { - guard.page().wait_for_navigation().await?; - } - guard - .page() - .evaluate_expression("globalThis.HMR_IS_HAPPENING = true") - .await - .context( - "Unable to evaluate JavaScript in the page for HMR check \ - flag", - )?; - - // There's a possible race condition between hydration and - // connection to the HMR server. We attempt to make updates with an - // exponential backoff until one succeeds. - let mut exponential_duration = Duration::from_millis(100); - loop { - match make_change( - &modules[0].0, - bundler, - &mut guard, - location, - exponential_duration, - &WallTime, - ) - .await - { - Ok(_) => { - break; - } - Err(e) => { - exponential_duration *= 2; - if exponential_duration > max_init_update_timeout { - return Err( - e.context("failed to make warmup change") - ); - } - } - } - } - - // Once we know the HMR server is connected, we make a few warmup - // changes. - let mut hmr_warmup_iter = 0; - let mut hmr_warmup_dropped = 0; - while hmr_warmup_iter < hmr_warmup { - match make_change( - &modules[0].0, - bundler, - &mut guard, - location, - max_update_timeout, - &WallTime, - ) - .await - { - Err(_) => { - // We don't care about dropped updates during warmup. - hmr_warmup_dropped += 1; - - if hmr_warmup_dropped >= hmr_warmup { - return Err(anyhow!( - "failed to make warmup change {} times", - hmr_warmup_dropped - )); - } - } - Ok(_) => { - hmr_warmup_iter += 1; - } - } - } - - Ok(guard) - }, - |mut guard, iters, m, verbose| { - let module_picker = Arc::clone(module_picker); - async move { - let mut value = m.zero(); - let mut dropped = 0; - let mut iter = 0; - while iter < iters { - let module = module_picker.pick(); - let duration = match make_change( - module, - bundler, - &mut guard, - location, - max_update_timeout, - &m, - ) - .await - { - Err(_) => { - // Some bundlers (e.g. Turbopack and Vite) can drop - // updates under certain conditions. We don't want - // to crash or stop the benchmark - // because of this. Instead, we keep going and - // report the number of dropped updates at the end. - dropped += 1; - continue; - } - Ok(duration) => duration, - }; - value = m.add(&value, &duration); - - iter += 1; - if verbose && iter != iters && iter.count_ones() == 1 { - eprint!( - " [{:?} {:?}/{}{}]", - duration, - FormatDuration(value / (iter as u32)), - iter, - if dropped > 0 { - format!(" ({} dropped)", dropped) - } else { - "".to_string() - } - ); - } - } - - Ok((guard, value)) - } - }, - |guard| async move { - let hmr_is_happening = guard - .page() - .evaluate_expression("globalThis.HMR_IS_HAPPENING") - .await - .unwrap(); - // Make sure that we are really measuring HMR and not accidentically - // full refreshing the page - assert!(hmr_is_happening.value().unwrap().as_bool().unwrap()); - }, - ); + &browser, + bundler, + module_count, + ) + .expect("failed to run `bench_hmr_internal_bench`") }, ); })); @@ -338,6 +178,169 @@ fn bench_hmr_internal( } } +fn bench_hmr_internal_bench( + b: &mut Bencher, + location: CodeLocation, + runtime: &Runtime, + browser: &Browser, + bundler: &dyn Bundler, + module_count: usize, +) -> Result<()> { + let hmr_warmup = read_env("TURBOPACK_BENCH_HMR_WARMUP", 10)?; + + let test_app = build_test(module_count, bundler); + let module_picker = Arc::new(ModulePicker::new(test_app.modules().to_vec())); + + let modules = test_app.modules(); + + let max_init_update_timeout = bundler.max_init_update_timeout(module_count); + let max_update_timeout = bundler.max_update_timeout(module_count); + + let mut app = PreparedApp::new_without_copy(bundler, test_app.path().to_path_buf()); + app.start_server()?; + + let mut guard = runtime.block_on(app.open_page(browser))?; + + runtime.block_on(async { + if bundler.has_hydration_event() { + guard.wait_for_hydration().await?; + } else { + guard.page().wait_for_navigation().await?; + } + guard + .page() + .evaluate_expression("globalThis.HMR_IS_HAPPENING = true") + .await + .context("Unable to evaluate JavaScript in the page for HMR check flag")?; + + // There's a possible race condition between hydration and + // connection to the HMR server. We attempt to make updates with an + // exponential backoff until one succeeds. + let mut exponential_duration = Duration::from_millis(100); + loop { + match make_change( + &modules[0].0, + bundler, + &mut guard, + location, + exponential_duration, + ) + .await + { + Ok(duration) => { + if cfg!(target_os = "linux") { + // TODO(sokra) triggering HMR updates too fast can have weird effects on + // Linux + tokio::time::sleep(std::cmp::max(duration, Duration::from_millis(100))) + .await; + } + + break; + } + Err(e) => { + exponential_duration *= 2; + if exponential_duration > max_init_update_timeout { + return Err(e.context("failed to make warmup change")); + } + } + } + } + + // Once we know the HMR server is connected, we make a few warmup + // changes. + let mut hmr_warmup_iter = 0; + let mut hmr_warmup_dropped = 0; + while hmr_warmup_iter < hmr_warmup { + match make_change( + &modules[0].0, + bundler, + &mut guard, + location, + max_update_timeout, + ) + .await + { + Err(_) => { + // We don't care about dropped updates during warmup. + hmr_warmup_dropped += 1; + + if hmr_warmup_dropped >= hmr_warmup { + return Err(anyhow!( + "failed to make warmup change {} times", + hmr_warmup_dropped + )); + } + } + Ok(duration) => { + if cfg!(target_os = "linux") { + // TODO(sokra) triggering HMR updates too fast can have weird effects on + // Linux + tokio::time::sleep(std::cmp::max(duration, Duration::from_millis(100))) + .await; + } + + hmr_warmup_iter += 1; + } + } + } + + anyhow::Ok(()) + })?; + + let guard = Mutex::new(guard); + + b.to_async(runtime).try_iter_batched( + || { + if cfg!(target_os = "linux") { + // TODO(sokra) triggering HMR updates too fast can have weird effects on Linux + std::thread::sleep(Duration::from_millis(100)); + } + + Ok(guard.try_lock()?) + }, + |mut guard| { + let module_picker = module_picker.clone(); + + async move { + let module = module_picker.pick(); + if make_change(module, bundler, &mut guard, location, max_update_timeout) + .await + .is_err() + { + // Some bundlers (e.g. Turbopack and Vite) can drop + // updates under certain conditions. We don't want + // to crash or stop the benchmark + // because of this. Instead, we keep going and + // report the number of dropped updates at the end. + + // dropped += 1; + // continue; + }; + + Ok(()) + } + }, + BatchSize::PerIteration, + ); + + // teardown + runtime.block_on(async move { + let hmr_is_happening = guard + .try_lock() + .unwrap() + .page() + .evaluate_expression("globalThis.HMR_IS_HAPPENING") + .await + .unwrap(); + + // Make sure that we are really measuring HMR and not accidentically + // full refreshing the page + assert!(hmr_is_happening.value().unwrap().as_bool().unwrap()); + }); + + Ok(()) +} + fn insert_code( path: &Path, bundler: &dyn Bundler, @@ -387,13 +390,12 @@ fn insert_code( static CHANGE_TIMEOUT_MESSAGE: &str = "update was not registered by bundler"; -async fn make_change<'a>( +async fn make_change( module: &Path, bundler: &dyn Bundler, - guard: &mut PageGuard<'a>, + guard: &mut PageGuard, location: CodeLocation, timeout_duration: Duration, - measurement: &WallTime, ) -> Result { static CHANGE_COUNTER: AtomicUsize = AtomicUsize::new(0); @@ -405,7 +407,7 @@ async fn make_change<'a>( // Keep the IO out of the measurement. let commit = insert_code(module, bundler, &msg, location)?; - let start = measurement.start(); + let start = Instant::now(); commit()?; @@ -415,12 +417,8 @@ async fn make_change<'a>( .await .context(CHANGE_TIMEOUT_MESSAGE)??; - let duration = measurement.end(start); + let duration = start.elapsed(); - if cfg!(target_os = "linux") { - // TODO(sokra) triggering HMR updates too fast can have weird effects on Linux - tokio::time::sleep(std::cmp::max(duration, Duration::from_millis(100))).await; - } Ok(duration) } @@ -476,53 +474,22 @@ fn bench_startup_cached_internal( } }; for module_count in get_module_counts() { - let test_app = Lazy::new(|| build_test(module_count, bundler.as_ref())); - let input = (bundler.as_ref(), &test_app); + let input = (bundler.as_ref(), module_count); resume_on_error(AssertUnwindSafe(|| { g.bench_with_input( BenchmarkId::new(bundler.get_name(), format!("{} modules", module_count)), &input, - |b, &(bundler, test_app)| { - let test_app = &**test_app; - let browser = &*browser; - b.to_async(&runtime).try_iter_custom(|iters, m| async move { - // Run a complete build, shut down, and test running it again - let mut app = - PreparedApp::new(bundler, test_app.path().to_path_buf()).await?; - app.start_server()?; - let mut guard = app.with_page(browser).await?; - if bundler.has_hydration_event() { - guard.wait_for_hydration().await?; - } else { - guard.page().wait_for_navigation().await?; - } - - let mut app = guard.close_page().await?; - - // Give it 4 seconds time to store the cache - sleep(Duration::from_secs(4)).await; - - app.stop_server()?; - - let mut value = m.zero(); - for _ in 0..iters { - let start = m.start(); - app.start_server()?; - let mut guard = app.with_page(browser).await?; - if wait_for_hydration { - guard.wait_for_hydration().await?; - } - let duration = m.end(start); - value = m.add(&value, &duration); - - app = guard.close_page().await?; - app.stop_server()?; - } - - drop(app); - Ok(value) - }); + |b, &(bundler, module_count)| { + bench_startup_cached_internal_bench( + b, + wait_for_hydration, + &runtime, + &browser, + bundler, + module_count, + ) + .expect("failed to run `bench_startup_cached_internal_bench`") }, ); })); @@ -530,6 +497,62 @@ fn bench_startup_cached_internal( } } +fn bench_startup_cached_internal_bench( + b: &mut Bencher, + wait_for_hydration: bool, + runtime: &Runtime, + browser: &Browser, + bundler: &dyn Bundler, + module_count: usize, +) -> Result<()> { + let test_app = build_test(module_count, bundler); + + let mut app = PreparedApp::new(bundler, test_app.path().to_path_buf())?; + + runtime.block_on(async { + // Run a complete build, shut down, and test running it again + app.start_server()?; + let mut page = app.open_page(browser).await?; + if bundler.has_hydration_event() { + page.wait_for_hydration().await?; + } else { + page.page().wait_for_navigation().await?; + } + + page.close_page().await?; + + // Give it 4 seconds time to store the cache + sleep(Duration::from_secs(4)).await; + + anyhow::Ok(()) + })?; + + let app = Mutex::new(app); + + b.to_async(runtime).try_iter_batched( + || { + let mut app = app.try_lock()?; + + // this needs to happen after each iteration + // we also left the server running above so the first iteration will work + app.stop_server()?; + + Ok(app) + }, + |mut app| async move { + app.start_server()?; + let mut page = app.open_page(browser).await?; + if wait_for_hydration { + page.wait_for_hydration().await?; + } + + Ok(page) + }, + BatchSize::PerIteration, + ); + + Ok(()) +} fn get_module_counts() -> Vec { read_env_list("TURBOPACK_BENCH_COUNTS", vec![1_000usize]).unwrap() } diff --git a/turbopack/crates/turbopack-bench/src/util/mod.rs b/turbopack/crates/turbopack-bench/src/util/mod.rs index c03e9703c94a3..972140aa28d76 100644 --- a/turbopack/crates/turbopack-bench/src/util/mod.rs +++ b/turbopack/crates/turbopack-bench/src/util/mod.rs @@ -4,7 +4,7 @@ use std::{ }, panic::UnwindSafe, process::Command, - time::{Duration, Instant}, + time::Duration, }; use anyhow::Result; @@ -12,16 +12,13 @@ use chromiumoxide::{ browser::{Browser, BrowserConfig}, error::CdpError::Ws, }; -use codspeed_criterion_compat::{ - async_executor::AsyncExecutor, black_box, measurement::WallTime, AsyncBencher, -}; +use codspeed_criterion_compat::{async_executor::AsyncExecutor, AsyncBencher}; +use criterion::BatchSize; use futures::{Future, StreamExt}; pub use page_guard::PageGuard; -use parking_lot::Mutex; pub use prepared_app::PreparedApp; use regex::Regex; use tungstenite::{error::ProtocolError::ResetWithoutClosingHandshake, Error::Protocol}; -use turbo_tasks::util::FormatDuration; use turbo_tasks_testing::retry::{retry, retry_async}; use turbopack_create_test_app::test_app_builder::{ EffectMode, PackageJsonConfig, TestApp, TestAppBuilder, @@ -46,15 +43,6 @@ where retry(args, f, 3, Duration::from_secs(5)) } -async fn retry_async_default(args: A, f: F) -> Result -where - F: Fn(&mut A) -> Fut, - Fut: Future>, -{ - // waits 5, 10, 20, 40 seconds = 75 seconds total - retry_async(args, f, 3, Duration::from_secs(5)).await -} - pub fn build_test(module_count: usize, bundler: &dyn Bundler) -> TestApp { let test_app = TestAppBuilder { module_count, @@ -137,111 +125,25 @@ pub fn resume_on_error(f: F) { } pub trait AsyncBencherExtension { - fn try_iter_custom(&mut self, routine: R) + fn try_iter_batched(&mut self, setup: S, routine: R, size: BatchSize) where - R: Fn(u64, WallTime) -> F, - F: Future>; - - fn try_iter_async( - &mut self, - runner: A, - setup: S, - routine: R, - teardown: T, - ) where - S: Fn() -> SF, - SF: Future>, - R: Fn(I, u64, WallTime, bool) -> F, - F: Future>, - T: Fn(I) -> TF, - TF: Future; + S: Fn() -> Result, + R: Fn(I) -> F, + F: Future>; } impl<'a, 'b, A: AsyncExecutor> AsyncBencherExtension for AsyncBencher<'a, 'b, A> { - fn try_iter_custom(&mut self, routine: R) + fn try_iter_batched(&mut self, setup: S, routine: R, size: BatchSize) where - R: Fn(u64, WallTime) -> F, - F: Future>, + S: Fn() -> Result, + R: Fn(I) -> F, + F: Future>, { - let log_progress = read_env_bool("TURBOPACK_BENCH_PROGRESS"); - - let routine = &routine; - self.iter_custom(|iters| async move { - let measurement = WallTime; - let value = routine(iters, measurement).await.expect("routine failed"); - if log_progress { - eprint!(" {:?}/{}", FormatDuration(value / (iters as u32)), iters); - } - value - }); - } - - fn try_iter_async( - &mut self, - runner: A, - setup: S, - routine: R, - teardown: T, - ) where - S: Fn() -> SF, - SF: Future>, - R: Fn(I, u64, WallTime, bool) -> F, - F: Future>, - T: Fn(I) -> TF, - TF: Future, - { - let log_progress = read_env_bool("TURBOPACK_BENCH_PROGRESS"); - - let setup = &setup; - let routine = &routine; - let teardown = &teardown; - let input_mutex = &Mutex::new(Some(black_box(runner.block_on(async { - if log_progress { - eprint!(" setup..."); - } - let start = Instant::now(); - let input = retry_async_default((), |_| setup()) - .await - .expect("failed to setup"); - if log_progress { - let duration = start.elapsed(); - eprint!(" [{:?}]", FormatDuration(duration)); - } - input - })))); - - self.iter_custom(|iters| async move { - let measurement = WallTime; - - let input = input_mutex - .lock() - .take() - .expect("iter_custom only executes its closure once"); - - let (output, value) = routine(input, iters, measurement, log_progress) - .await - .expect("Routine failed"); - let output = black_box(output); - - if log_progress { - eprint!(" {:?}/{}", FormatDuration(value / (iters as u32)), iters); - } - - input_mutex.lock().replace(output); - - value - }); - - let input = input_mutex.lock().take().unwrap(); - if log_progress { - eprint!(" teardown..."); - } - let start = Instant::now(); - runner.block_on(teardown(input)); - let duration = start.elapsed(); - if log_progress { - eprintln!(" [{:?}]", FormatDuration(duration)); - } + self.iter_batched( + || setup().expect("setup failed"), + |input| async { routine(input).await.expect("routine failed") }, + size, + ) } } diff --git a/turbopack/crates/turbopack-bench/src/util/page_guard.rs b/turbopack/crates/turbopack-bench/src/util/page_guard.rs index 5848d9085780e..59f55f8797b99 100644 --- a/turbopack/crates/turbopack-bench/src/util/page_guard.rs +++ b/turbopack/crates/turbopack-bench/src/util/page_guard.rs @@ -9,15 +9,14 @@ use chromiumoxide::{ use futures::{Stream, StreamExt}; use tokio::time::timeout; -use crate::{PreparedApp, BINDING_NAME}; +use crate::BINDING_NAME; const MAX_HYDRATION_TIMEOUT: Duration = Duration::from_secs(120); const TEST_APP_HYDRATION_DONE: &str = "Hydration done"; /// Closes a browser page on Drop. -pub struct PageGuard<'a> { +pub struct PageGuard { page: Option, - app: Option>, events: Box + Unpin>, } @@ -26,17 +25,15 @@ enum Event { EventExceptionThrown(Arc), } -impl<'a> PageGuard<'a> { +impl PageGuard { /// Creates a new guard for the given page. pub fn new( page: Page, events: EventStream, errors: EventStream, - app: PreparedApp<'a>, ) -> Self { Self { page: Some(page), - app: Some(app), events: Box::new(futures::stream::select( events.map(Event::EventBindingCalled), errors.map(Event::EventExceptionThrown), @@ -50,14 +47,11 @@ impl<'a> PageGuard<'a> { self.page.as_ref().unwrap() } - /// Closes the page, returns the app. - pub async fn close_page(mut self) -> Result> { + /// Closes the page. + pub async fn close_page(mut self) -> Result<()> { // Invariant: the page is always Some while the guard is alive. self.page.take().unwrap().close().await?; - Ok( - // Invariant: the app is always Some while the guard is alive. - self.app.take().unwrap(), - ) + Ok(()) } /// Waits until the binding is called with the given payload. @@ -94,7 +88,7 @@ impl<'a> PageGuard<'a> { } } -impl<'a> Drop for PageGuard<'a> { +impl Drop for PageGuard { fn drop(&mut self) { // The page might have been closed already in `close_page`. if let Some(page) = self.page.take() { diff --git a/turbopack/crates/turbopack-bench/src/util/prepared_app.rs b/turbopack/crates/turbopack-bench/src/util/prepared_app.rs index 80a29817f0792..508607a9ddb6a 100644 --- a/turbopack/crates/turbopack-bench/src/util/prepared_app.rs +++ b/turbopack/crates/turbopack-bench/src/util/prepared_app.rs @@ -1,5 +1,5 @@ use std::{ - future::Future, + fs, path::{Path, PathBuf}, process::Child, }; @@ -13,46 +13,24 @@ use chromiumoxide::{ Browser, Page, }; use futures::{FutureExt, StreamExt}; -use tokio::task::spawn_blocking; use url::Url; use crate::{bundlers::Bundler, util::PageGuard, BINDING_NAME}; -// HACK: Needed so that `copy_dir`'s `Future` can be inferred as `Send`: -// https://github.com/rust-lang/rust/issues/123072 -fn copy_dir_send(from: PathBuf, to: PathBuf) -> impl Future> + Send { - copy_dir(from, to) -} - -async fn copy_dir(from: PathBuf, to: PathBuf) -> anyhow::Result<()> { - let dir = spawn_blocking(|| std::fs::read_dir(from)).await??; - let mut jobs = Vec::new(); - let mut file_futures = Vec::new(); +fn copy_dir, TP: AsRef>(from: FP, to: TP) -> Result<()> { + let dir = fs::read_dir(from)?; for entry in dir { let entry = entry?; let ty = entry.file_type()?; - let to = to.join(entry.file_name()); + let to = to.as_ref().join(entry.file_name()); if ty.is_dir() { - jobs.push(tokio::spawn(async move { - tokio::fs::create_dir(&to).await?; - copy_dir_send(entry.path(), to).await - })); + fs::create_dir(&to)?; + copy_dir(entry.path(), to)?; } else if ty.is_file() { - file_futures.push(async move { - tokio::fs::copy(entry.path(), to).await?; - Ok::<_, anyhow::Error>(()) - }); + fs::copy(entry.path(), to)?; } } - for future in file_futures { - jobs.push(tokio::spawn(future)); - } - - for job in jobs { - job.await??; - } - Ok(()) } @@ -68,11 +46,11 @@ pub struct PreparedApp<'a> { } impl<'a> PreparedApp<'a> { - pub async fn new(bundler: &'a dyn Bundler, template_dir: PathBuf) -> Result> { + pub fn new(bundler: &'a dyn Bundler, template_dir: PathBuf) -> Result> { let test_dir = tempfile::tempdir()?; - tokio::fs::create_dir_all(&test_dir).await?; - copy_dir(template_dir, test_dir.path().to_path_buf()).await?; + fs::create_dir_all(&test_dir)?; + copy_dir(template_dir, test_dir.path())?; Ok(Self { bundler, @@ -81,15 +59,12 @@ impl<'a> PreparedApp<'a> { }) } - pub async fn new_without_copy( - bundler: &'a dyn Bundler, - template_dir: PathBuf, - ) -> Result> { - Ok(Self { + pub fn new_without_copy(bundler: &'a dyn Bundler, template_dir: PathBuf) -> PreparedApp<'a> { + Self { bundler, server: None, test_dir: PreparedDir::Path(template_dir), - }) + } } pub fn start_server(&mut self) -> Result<()> { @@ -100,7 +75,7 @@ impl<'a> PreparedApp<'a> { Ok(()) } - pub async fn with_page(self, browser: &Browser) -> Result> { + pub async fn open_page(&self, browser: &Browser) -> Result { let server = self.server.as_ref().context("Server must be started")?; let page = browser .new_page("about:blank") @@ -150,13 +125,13 @@ impl<'a> PreparedApp<'a> { // Make sure no runtime errors occurred when loading the page assert!(errors.next().now_or_never().is_none()); - let page_guard = PageGuard::new(page, binding_events, errors, self); + let page_guard = PageGuard::new(page, binding_events, errors); Ok(page_guard) } pub fn stop_server(&mut self) -> Result<()> { - let mut proc = self.server.take().expect("Server never started").0; + let mut proc = self.server.take().context("Server never started")?.0; stop_process(&mut proc)?; Ok(()) }