diff --git a/clippy.toml b/clippy.toml index 3ec3b727548ac..d51608965b896 100644 --- a/clippy.toml +++ b/clippy.toml @@ -8,4 +8,7 @@ disallowed-methods = [ # sets), but the hash **is not stable** and must not be observed. # Use Xxh3Hash64Hasher::write with value's bytes directly. "std::hash::Hasher::hash", + # We forbid the use of VecDeque::new as it allocates, which is kind of unexpected + # Instead use VecDeque::with_capacity to make it explicit or opt-out of that. + "std::collections::VecDeque::new", ] diff --git a/crates/turbo-tasks-memory/src/task.rs b/crates/turbo-tasks-memory/src/task.rs index 6a7f13a4141d8..a0f5880e3d166 100644 --- a/crates/turbo-tasks-memory/src/task.rs +++ b/crates/turbo-tasks-memory/src/task.rs @@ -1,7 +1,7 @@ use std::{ borrow::Cow, cell::RefCell, - cmp::Ordering, + cmp::{max, Ordering}, collections::VecDeque, fmt::{self, Debug, Display, Formatter, Write}, future::Future, @@ -743,6 +743,9 @@ impl Task { } } + if queue.capacity() == 0 { + queue.reserve(max(children.len(), SPLIT_OFF_QUEUE_AT * 2)); + } queue.extend(children.iter().copied().map(|child| (child, depth + 1))); // add to dirty list of the scope (potentially schedule) @@ -764,7 +767,8 @@ impl Task { backend: &MemoryBackend, turbo_tasks: &dyn TurboTasksBackendApi, ) { - let mut queue = VecDeque::new(); + // VecDeque::new() would allocate with 7 items capacity. We don't want that. + let mut queue = VecDeque::with_capacity(0); self.add_to_scope_internal_shallow( id, is_optimization_scope, @@ -908,6 +912,9 @@ impl Task { TaskScopes::Inner(ref mut set, _) => { if set.remove(id) { self.remove_self_from_scope(&mut state, id, backend, turbo_tasks); + if queue.capacity() == 0 { + queue.reserve(max(state.children.len(), SPLIT_OFF_QUEUE_AT * 2)); + } queue.extend(state.children.iter().copied()); drop(state); } @@ -921,7 +928,8 @@ impl Task { backend: &MemoryBackend, turbo_tasks: &dyn TurboTasksBackendApi, ) { - let mut queue = VecDeque::new(); + // VecDeque::new() would allocate with 7 items capacity. We don't want that. + let mut queue = VecDeque::with_capacity(0); self.remove_from_scope_internal_shallow(id, backend, turbo_tasks, &mut queue); run_remove_from_scope_queue(queue, id, backend, turbo_tasks); } @@ -1540,8 +1548,8 @@ pub fn run_add_to_scope_queue( &mut queue, ); }); - if queue.len() > SPLIT_OFF_QUEUE_AT { - let split_off_queue = queue.split_off(SPLIT_OFF_QUEUE_AT); + while queue.len() > SPLIT_OFF_QUEUE_AT { + let split_off_queue = queue.split_off(queue.len() - SPLIT_OFF_QUEUE_AT); turbo_tasks.schedule_backend_foreground_job(backend.create_backend_job( Job::AddToScopeQueue(split_off_queue, id, is_optimization_scope), )); @@ -1560,8 +1568,8 @@ pub fn run_remove_from_scope_queue( backend.with_task(child, |child| { child.remove_from_scope_internal_shallow(id, backend, turbo_tasks, &mut queue); }); - if queue.len() > SPLIT_OFF_QUEUE_AT { - let split_off_queue = queue.split_off(SPLIT_OFF_QUEUE_AT); + while queue.len() > SPLIT_OFF_QUEUE_AT { + let split_off_queue = queue.split_off(queue.len() - SPLIT_OFF_QUEUE_AT); turbo_tasks.schedule_backend_foreground_job( backend.create_backend_job(Job::RemoveFromScopeQueue(split_off_queue, id)), diff --git a/crates/turbopack-core/src/chunk/mod.rs b/crates/turbopack-core/src/chunk/mod.rs index d92a13af028d6..a2371daef2b7b 100644 --- a/crates/turbopack-core/src/chunk/mod.rs +++ b/crates/turbopack-core/src/chunk/mod.rs @@ -346,7 +346,7 @@ async fn chunk_content_internal( let mut chunks = Vec::new(); let mut async_chunk_groups = Vec::new(); let mut external_asset_references = Vec::new(); - let mut queue = VecDeque::new(); + let mut queue = VecDeque::with_capacity(32); let chunk_item = I::from_asset(context, entry).await?.unwrap(); queue.push_back(ChunkContentWorkItem::AssetReferences( diff --git a/crates/turbopack-core/src/reference/mod.rs b/crates/turbopack-core/src/reference/mod.rs index b00732356425e..609173b037414 100644 --- a/crates/turbopack-core/src/reference/mod.rs +++ b/crates/turbopack-core/src/reference/mod.rs @@ -94,7 +94,7 @@ impl SingleAssetReferenceVc { pub async fn all_referenced_assets(asset: AssetVc) -> Result { let references_set = asset.references().await?; let mut assets = Vec::new(); - let mut queue = VecDeque::new(); + let mut queue = VecDeque::with_capacity(32); for reference in references_set.iter() { queue.push_back(reference.resolve_reference()); } @@ -137,7 +137,7 @@ pub async fn all_referenced_assets(asset: AssetVc) -> Result { #[turbo_tasks::function] pub async fn all_assets(asset: AssetVc) -> Result { // TODO need to track import path here - let mut queue = VecDeque::new(); + let mut queue = VecDeque::with_capacity(32); queue.push_back((asset, all_referenced_assets(asset))); let mut assets = HashSet::new(); assets.insert(asset); diff --git a/crates/turbopack-create-test-app/src/test_app_builder.rs b/crates/turbopack-create-test-app/src/test_app_builder.rs index 5fb3286301086..e37f47ddf680c 100644 --- a/crates/turbopack-create-test-app/src/test_app_builder.rs +++ b/crates/turbopack-create-test-app/src/test_app_builder.rs @@ -82,7 +82,7 @@ const SETUP_EFFECT_PROPS: &str = indoc! {r#" let EFFECT_PROPS = {}; "#}; const SETUP_EVAL: &str = indoc! {r#" -/* @turbopack-bench:eval-start */ +/* @turbopack-bench:eval-start */ /* @turbopack-bench:eval-end */ "#}; const USE_EFFECT: &str = indoc! {r#" @@ -115,7 +115,7 @@ impl TestAppBuilder { let mut remaining_directories = self.directories_count; let mut remaining_dynamic_imports = self.dynamic_import_count; - let mut queue = VecDeque::new(); + let mut queue = VecDeque::with_capacity(32); queue.push_back((src.join("triangle.jsx"), 0)); remaining_modules -= 1; let mut is_root = true; diff --git a/crates/turbopack-dev-server/src/source/asset_graph.rs b/crates/turbopack-dev-server/src/source/asset_graph.rs index 2ac20aa8e0d68..2eefbd5324d71 100644 --- a/crates/turbopack-dev-server/src/source/asset_graph.rs +++ b/crates/turbopack-dev-server/src/source/asset_graph.rs @@ -91,7 +91,7 @@ impl AssetGraphContentSourceVc { let mut map = HashMap::new(); let root_path = this.root_path.await?; let mut assets = Vec::new(); - let mut queue = VecDeque::new(); + let mut queue = VecDeque::with_capacity(32); let mut assets_set = HashSet::new(); let root_assets = this.root_assets.await?; if let Some(state) = &this.state { diff --git a/crates/turbopack-tests/tests/snapshot.rs b/crates/turbopack-tests/tests/snapshot.rs index 192381afd9ecd..a21820b29ae32 100644 --- a/crates/turbopack-tests/tests/snapshot.rs +++ b/crates/turbopack-tests/tests/snapshot.rs @@ -211,7 +211,7 @@ async fn run_test(resource: String) -> Result { .await?; let mut seen = HashSet::new(); - let mut queue = VecDeque::new(); + let mut queue = VecDeque::with_capacity(32); for chunk in chunks { queue.push_back(chunk.as_asset()); } diff --git a/crates/turbopack/tests/helpers/mod.rs b/crates/turbopack/tests/helpers/mod.rs index 2fb89eed0ee9f..f050e9c86a94d 100644 --- a/crates/turbopack/tests/helpers/mod.rs +++ b/crates/turbopack/tests/helpers/mod.rs @@ -6,7 +6,7 @@ pub fn print_changeset(changeset: &Changeset) -> String { assert!(changeset.split == "\n"); let mut result = String::from("--- DIFF ---\n- Expected\n+ Actual\n------------\n"); const CONTEXT_LINES: usize = 3; - let mut context = VecDeque::new(); + let mut context = VecDeque::with_capacity(CONTEXT_LINES); let mut need_context = CONTEXT_LINES; let mut has_spacing = false; for diff in changeset.diffs.iter() {