Skip to content

Commit

Permalink
Sample many modules in benchmarks + reliability fixes (vercel/turbo#2750
Browse files Browse the repository at this point in the history
)

* Sample many modules in benchmarks + reliability fixes

* Fix depth sampling to be uniform

* Fix Webpack benchmark

* Only use detector component for RSC

* Clippy

* Clippy
  • Loading branch information
alexkirsz committed Nov 28, 2022
1 parent 78d07e3 commit eae0451
Show file tree
Hide file tree
Showing 10 changed files with 294 additions and 186 deletions.
1 change: 1 addition & 0 deletions crates/next-dev/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ fs_extra = "1.2.0"
lazy_static = "1.4.0"
once_cell = "1.13.0"
parking_lot = "0.12.1"
rand = "0.8.5"
regex = "1.6.0"
tempfile = "3.3.0"
test-generator = "0.3.0"
Expand Down
11 changes: 6 additions & 5 deletions crates/next-dev/benches/bundlers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use self::{
vite::Vite,
webpack::Webpack,
};
use crate::util::env::read_env;

mod nextjs;
mod parcel;
Expand Down Expand Up @@ -59,16 +60,16 @@ pub trait Bundler {
}

pub fn get_bundlers() -> Vec<Box<dyn Bundler>> {
let config = std::env::var("TURBOPACK_BENCH_BUNDLERS").ok();
let config: String = read_env("TURBOPACK_BENCH_BUNDLERS", String::from("turbopack")).unwrap();
let mut turbopack = false;
let mut others = false;
match config.as_deref() {
Some("all") => {
match config.as_ref() {
"all" => {
turbopack = true;
others = true
}
Some("others") => others = true,
None | Some("") => {
"others" => others = true,
"turbopack" => {
turbopack = true;
}
_ => panic!("Invalid value for TURBOPACK_BENCH_BUNDLERS"),
Expand Down
10 changes: 4 additions & 6 deletions crates/next-dev/benches/bundlers/vite/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,17 @@ impl Bundler for Vite {
} else {
"Vite SSR"
}
} else if self.swc {
"Vite SWC CSR"
} else {
if self.swc {
"Vite SWC CSR"
} else {
"Vite CSR"
}
"Vite CSR"
}
}

fn prepare(&self, install_dir: &Path) -> Result<()> {
let mut packages = vec![NpmPackage::new("vite", "^3.2.4")];
if self.swc {
packages.push(NpmPackage::new("vite-plugin-swc-react-refresh", "^2.2.0"));
packages.push(NpmPackage::new("vite-plugin-swc-react-refresh", "^2.2.1"));
} else {
packages.push(NpmPackage::new("@vitejs/plugin-react", "^2.2.0"));
};
Expand Down
1 change: 1 addition & 0 deletions crates/next-dev/benches/bundlers/webpack/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ impl Bundler for Webpack {
&[
NpmPackage::new("@pmmmwh/react-refresh-webpack-plugin", "^0.5.7"),
NpmPackage::new("@swc/core", "^1.2.249"),
NpmPackage::new("@swc/helpers", "^0.4.13"),
NpmPackage::new("react-refresh", "^0.14.0"),
NpmPackage::new("swc-loader", "^0.2.3"),
NpmPackage::new("webpack", "^5.75.0"),
Expand Down
304 changes: 173 additions & 131 deletions crates/next-dev/benches/mod.rs

Large diffs are not rendered by default.

47 changes: 47 additions & 0 deletions crates/next-dev/benches/util/env.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
use std::{error::Error, str::FromStr};

use anyhow::{anyhow, Context, Result};

/// Reads an environment variable.
pub fn read_env<T>(name: &str, default: T) -> Result<T>
where
T: FromStr,
<T as FromStr>::Err: Error + Send + Sync + 'static,
{
let config = std::env::var(name).ok();
match config.as_deref() {
None | Some("") => Ok(default),
Some(config) => config
.parse()
.with_context(|| anyhow!("Invalid value for {}", name)),
}
}

/// Reads an boolean-like environment variable, where any value but "0", "no",
/// or "false" is are considered true.
pub fn read_env_bool(name: &str) -> bool {
let config = std::env::var(name).ok();
!matches!(
config.as_deref(),
None | Some("") | Some("0") | Some("no") | Some("false")
)
}

/// Reads a comma-separated environment variable as a vector.
pub fn read_env_list<T>(name: &str, default: Vec<T>) -> Result<Vec<T>>
where
T: FromStr,
<T as FromStr>::Err: Error + Send + Sync + 'static,
{
let config = std::env::var(name).ok();
match config.as_deref() {
None | Some("") => Ok(default),
Some(config) => config
.split(',')
.map(|s| {
s.parse()
.with_context(|| anyhow!("Invalid value for {}", name))
})
.collect(),
}
}
40 changes: 16 additions & 24 deletions crates/next-dev/benches/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,15 @@ 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::{PackageJsonConfig, TestApp, TestAppBuilder};
use turbopack_create_test_app::test_app_builder::{
EffectMode, PackageJsonConfig, TestApp, TestAppBuilder,
};

use crate::bundlers::Bundler;
use self::env::read_env_bool;
use crate::bundlers::{Bundler, RenderType};

pub mod env;
pub mod module_picker;
pub mod npm;
mod page_guard;
mod prepared_app;
Expand Down Expand Up @@ -53,6 +58,10 @@ pub fn build_test(module_count: usize, bundler: &dyn Bundler) -> TestApp {
package_json: Some(PackageJsonConfig {
react_version: bundler.react_version().to_string(),
}),
effect_mode: match bundler.render_type() {
RenderType::ServerSideRenderedWithEvents => EffectMode::Component,
_ => EffectMode::Hook,
},
..Default::default()
}
.build()
Expand All @@ -76,14 +85,8 @@ pub fn build_test(module_count: usize, bundler: &dyn Bundler) -> TestApp {
}

pub async fn create_browser() -> Browser {
let with_head = !matches!(
std::env::var("TURBOPACK_BENCH_HEAD").ok().as_deref(),
None | Some("") | Some("no") | Some("false")
);
let with_devtools = !matches!(
std::env::var("TURBOPACK_BENCH_DEVTOOLS").ok().as_deref(),
None | Some("") | Some("no") | Some("false")
);
let with_head = read_env_bool("TURBOPACK_BENCH_WITH_HEAD");
let with_devtools = read_env_bool("TURBOPACK_BENCH_DEVTOOLS");
let mut builder = BrowserConfig::builder();
if with_head {
builder = builder.with_head();
Expand Down Expand Up @@ -117,12 +120,7 @@ pub async fn create_browser() -> Browser {

pub fn resume_on_error<F: FnOnce() + UnwindSafe>(f: F) {
let runs_as_bench = std::env::args().find(|a| a == "--bench");
let ignore_errors = !matches!(
std::env::var("TURBOPACK_BENCH_IGNORE_ERRORS")
.ok()
.as_deref(),
None | Some("") | Some("no") | Some("false")
);
let ignore_errors = read_env_bool("TURBOPACK_BENCH_IGNORE_ERRORS");

if runs_as_bench.is_some() || ignore_errors {
use std::panic::catch_unwind;
Expand Down Expand Up @@ -160,10 +158,7 @@ impl<'a, 'b, A: AsyncExecutor> AsyncBencherExtension<A> for AsyncBencher<'a, 'b,
R: Fn(u64, WallTime) -> F,
F: Future<Output = Result<Duration>>,
{
let log_progress = !matches!(
std::env::var("TURBOPACK_BENCH_PROGRESS").ok().as_deref(),
None | Some("") | Some("no") | Some("false")
);
let log_progress = read_env_bool("TURBOPACK_BENCH_PROGRESS");

let routine = &routine;
self.iter_custom(|iters| async move {
Expand All @@ -190,10 +185,7 @@ impl<'a, 'b, A: AsyncExecutor> AsyncBencherExtension<A> for AsyncBencher<'a, 'b,
T: Fn(I) -> TF,
TF: Future<Output = ()>,
{
let log_progress = !matches!(
std::env::var("TURBOPACK_BENCH_PROGRESS").ok().as_deref(),
None | Some("") | Some("no") | Some("false")
);
let log_progress = read_env_bool("TURBOPACK_BENCH_PROGRESS");

let setup = &setup;
let routine = &routine;
Expand Down
46 changes: 46 additions & 0 deletions crates/next-dev/benches/util/module_picker.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
use std::{collections::HashMap, path::PathBuf};

use rand::{rngs::StdRng, seq::SliceRandom, SeedableRng};

/// Picks modules at random, but with a fixed seed so runs are somewhat
/// reproducible.
///
/// This must be initialized outside of `bench_with_input` so we don't repeat
/// the same sequence in different samples.
pub struct ModulePicker {
depths: Vec<usize>,
modules_by_depth: HashMap<usize, Vec<PathBuf>>,
rng: parking_lot::Mutex<StdRng>,
}

impl ModulePicker {
/// Creates a new module picker.
pub fn new(mut modules: Vec<(PathBuf, usize)>) -> Self {
let rng = StdRng::seed_from_u64(42);

// Ensure the module order is deterministic.
modules.sort();

let mut modules_by_depth: HashMap<_, Vec<_>> = HashMap::new();
for (module, depth) in modules {
modules_by_depth.entry(depth).or_default().push(module);
}
let mut depths: Vec<_> = modules_by_depth.keys().copied().collect();
// Ensure the depth order is deterministic.
depths.sort();

Self {
depths,
modules_by_depth,
rng: parking_lot::Mutex::new(rng),
}
}

/// Picks a random module with a uniform distribution over all depths.
pub fn pick(&self) -> &PathBuf {
let mut rng = self.rng.lock();
// Sample from all depths uniformly.
let depth = self.depths.choose(&mut *rng).unwrap();
self.modules_by_depth[depth].choose(&mut *rng).unwrap()
}
}
12 changes: 0 additions & 12 deletions crates/next-dev/benches/util/page_guard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,12 @@ impl<'a> PageGuard<'a> {
}
}

/// Returns a reference to the app.
pub fn app(&self) -> &PreparedApp<'a> {
// Invariant: app is always Some while the guard is alive.
self.app.as_ref().unwrap()
}

/// Returns a reference to the page.
pub fn page(&self) -> &Page {
// Invariant: page is always Some while the guard is alive.
self.page.as_ref().unwrap()
}

/// Returns a mutable reference to the app.
pub fn app_mut(&mut self) -> &mut PreparedApp<'a> {
// Invariant: app is always Some while the guard is alive.
self.app.as_mut().unwrap()
}

/// Closes the page, returns the app.
pub async fn close_page(mut self) -> Result<PreparedApp<'a>> {
// Invariant: the page is always Some while the guard is alive.
Expand Down
8 changes: 0 additions & 8 deletions crates/next-dev/benches/util/prepared_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ pub struct PreparedApp<'a> {
bundler: &'a dyn Bundler,
server: Option<(Child, String)>,
test_dir: PreparedDir,
counter: usize,
}

impl<'a> PreparedApp<'a> {
Expand All @@ -81,7 +80,6 @@ impl<'a> PreparedApp<'a> {
bundler,
server: None,
test_dir: PreparedDir::TempDir(test_dir),
counter: 0,
})
}

Expand All @@ -93,15 +91,9 @@ impl<'a> PreparedApp<'a> {
bundler,
server: None,
test_dir: PreparedDir::Path(template_dir),
counter: 0,
})
}

pub fn counter(&mut self) -> usize {
self.counter += 1;
self.counter
}

pub fn start_server(&mut self) -> Result<()> {
assert!(self.server.is_none(), "Server already started");

Expand Down

0 comments on commit eae0451

Please sign in to comment.