Skip to content

Commit 9a53adc

Browse files
sokrasokra
authored
[Turbopack] Perf improvements new backend (#71249)
### What? A bunch of small performance optimizations * split operation_suspend_point in inlined and cold part * keep children count instead of iterating children every time * avoid bubbling find and schedule into already scheduling parts of the graph * reduce number of turbo-tasks function calls in EsmAssertReference::code_generation * avoid creating empty maps Sadly no measurable effect...
1 parent b1f5d13 commit 9a53adc

File tree

10 files changed

+85
-66
lines changed

10 files changed

+85
-66
lines changed

‎turbopack/crates/turbo-tasks-backend/src/backend/mod.rs‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -206,28 +206,36 @@ impl TurboTasksBackendInner {
206206
}
207207

208208
fn operation_suspend_point(&self, suspend: impl FnOnce() -> AnyOperation) {
209-
if self.suspending_requested() {
209+
#[cold]
210+
fn operation_suspend_point_cold(
211+
this: &TurboTasksBackendInner,
212+
suspend: impl FnOnce() -> AnyOperation,
213+
) {
210214
let operation = Arc::new(suspend());
211-
let mut snapshot_request = self.snapshot_request.lock();
215+
let mut snapshot_request = this.snapshot_request.lock();
212216
if snapshot_request.snapshot_requested {
213217
snapshot_request
214218
.suspended_operations
215219
.insert(operation.clone().into());
216-
let value = self.in_progress_operations.fetch_sub(1, Ordering::AcqRel) - 1;
220+
let value = this.in_progress_operations.fetch_sub(1, Ordering::AcqRel) - 1;
217221
assert!((value & SNAPSHOT_REQUESTED_BIT) != 0);
218222
if value == SNAPSHOT_REQUESTED_BIT {
219-
self.operations_suspended.notify_all();
223+
this.operations_suspended.notify_all();
220224
}
221-
self.snapshot_completed
225+
this.snapshot_completed
222226
.wait_while(&mut snapshot_request, |snapshot_request| {
223227
snapshot_request.snapshot_requested
224228
});
225-
self.in_progress_operations.fetch_add(1, Ordering::AcqRel);
229+
this.in_progress_operations.fetch_add(1, Ordering::AcqRel);
226230
snapshot_request
227231
.suspended_operations
228232
.remove(&operation.into());
229233
}
230234
}
235+
236+
if self.suspending_requested() {
237+
operation_suspend_point_cold(self, suspend);
238+
}
231239
}
232240

233241
pub(crate) fn start_operation(&self) -> OperationGuard<'_> {

‎turbopack/crates/turbo-tasks-backend/src/backend/operation/aggregation_update.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -614,18 +614,18 @@ impl AggregationUpdateQueue {
614614
}
615615
}
616616
if is_aggregating_node(get_aggregation_number(&task)) {
617-
// TODO if it has an `AggregateRoot` we can skip visiting the nested nodes since
617+
// if it has an `AggregateRoot` we can skip visiting the nested nodes since
618618
// this would already be scheduled by the `AggregateRoot`
619619
if !task.has_key(&CachedDataItemKey::AggregateRoot {}) {
620620
task.insert(CachedDataItem::AggregateRoot {
621621
value: RootState::new(ActiveType::CachedActiveUntilClean, task_id),
622622
});
623-
}
624-
let dirty_containers: Vec<_> = get_many!(task, AggregatedDirtyContainer { task } count if count.get(session_id) > 0 => *task);
625-
if !dirty_containers.is_empty() {
626-
self.push(AggregationUpdateJob::FindAndScheduleDirty {
627-
task_ids: dirty_containers,
628-
});
623+
let dirty_containers: Vec<_> = get_many!(task, AggregatedDirtyContainer { task } count if count.get(session_id) > 0 => *task);
624+
if !dirty_containers.is_empty() {
625+
self.push(AggregationUpdateJob::FindAndScheduleDirty {
626+
task_ids: dirty_containers,
627+
});
628+
}
629629
}
630630
}
631631
}

‎turbopack/crates/turbo-tasks-backend/src/backend/operation/cleanup_old_edges.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use crate::{
1313
invalidate::make_task_dirty,
1414
AggregatedDataUpdate, ExecuteContext, Operation,
1515
},
16-
storage::update_count,
16+
storage::{update, update_count},
1717
TaskDataCategory,
1818
},
1919
data::{CachedDataItemKey, CellRef, CollectibleRef, CollectiblesRef},
@@ -82,6 +82,12 @@ impl Operation for CleanupOldEdgesOperation {
8282
for &child_id in children.iter() {
8383
task.remove(&CachedDataItemKey::Child { task: child_id });
8484
}
85+
let remove_children_count = u32::try_from(children.len()).unwrap();
86+
update!(task, ChildrenCount, |count: Option<u32>| {
87+
// If this underflows, we messed up counting somewhere
88+
let count = count.unwrap_or_default() - remove_children_count;
89+
(count != 0).then_some(count)
90+
});
8591
if is_aggregating_node(get_aggregation_number(&task)) {
8692
queue.push(AggregationUpdateJob::InnerLostFollowers {
8793
upper_ids: vec![task_id],

‎turbopack/crates/turbo-tasks-backend/src/backend/operation/connect_child.rs‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ use crate::{
1111
},
1212
is_root_node, ExecuteContext, Operation,
1313
},
14-
storage::get,
14+
storage::{get, update},
1515
TaskDataCategory,
1616
},
17-
data::{CachedDataItem, CachedDataItemIndex, CachedDataItemKey},
17+
data::{CachedDataItem, CachedDataItemKey},
1818
};
1919

2020
const AGGREGATION_NUMBER_BUFFER_SPACE: u32 = 2;
@@ -44,6 +44,13 @@ impl ConnectChildOperation {
4444
task: child_task_id,
4545
value: (),
4646
}) {
47+
// Update the children count
48+
let mut children_count = 0;
49+
update!(parent_task, ChildrenCount, |count: Option<u32>| {
50+
children_count = count.unwrap_or_default() + 1;
51+
Some(children_count)
52+
});
53+
4754
// Update the task aggregation
4855
let mut queue = AggregationUpdateQueue::new();
4956

@@ -54,16 +61,6 @@ impl ConnectChildOperation {
5461
let parent_aggregation = if is_root_node(current_parent_aggregation.base) {
5562
u32::MAX
5663
} else {
57-
let children_count = parent_task
58-
.iter(CachedDataItemIndex::Children)
59-
.filter(|(k, _)| {
60-
matches!(
61-
*k,
62-
CachedDataItemKey::Child { .. }
63-
| CachedDataItemKey::OutdatedChild { .. }
64-
)
65-
})
66-
.count();
6764
let target_distance = children_count.ilog2() * 2;
6865
if target_distance != current_parent_aggregation.distance {
6966
queue.push(AggregationUpdateJob::UpdateAggregationNumber {

‎turbopack/crates/turbo-tasks-backend/src/backend/storage.rs‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -192,14 +192,22 @@ where
192192
}
193193
}
194194

195-
fn get_map_mut(&mut self, key: &T::Key) -> &mut AutoMap<T::Key, T::Value> {
195+
fn get_or_create_map_mut(&mut self, key: &T::Key) -> &mut AutoMap<T::Key, T::Value> {
196196
self.check_threshold();
197197
match self {
198198
InnerStorage::Plain { map, .. } => map,
199199
InnerStorage::Indexed { map, .. } => map.entry(key.index()).or_default(),
200200
}
201201
}
202202

203+
fn get_map_mut(&mut self, key: &T::Key) -> Option<&mut AutoMap<T::Key, T::Value>> {
204+
self.check_threshold();
205+
match self {
206+
InnerStorage::Plain { map, .. } => Some(map),
207+
InnerStorage::Indexed { map, .. } => map.get_mut(&key.index()),
208+
}
209+
}
210+
203211
fn get_map(&self, key: &T::Key) -> Option<&AutoMap<T::Key, T::Value>> {
204212
match self {
205213
InnerStorage::Plain { map, .. } => Some(map),
@@ -216,7 +224,7 @@ where
216224

217225
pub fn add(&mut self, item: T) -> bool {
218226
let (key, value) = item.into_key_and_value();
219-
match self.get_map_mut(&key).entry(key) {
227+
match self.get_or_create_map_mut(&key).entry(key) {
220228
Entry::Occupied(_) => false,
221229
Entry::Vacant(e) => {
222230
e.insert(value);
@@ -227,19 +235,19 @@ where
227235

228236
pub fn insert(&mut self, item: T) -> Option<T::Value> {
229237
let (key, value) = item.into_key_and_value();
230-
self.get_map_mut(&key).insert(key, value)
238+
self.get_or_create_map_mut(&key).insert(key, value)
231239
}
232240

233241
pub fn remove(&mut self, key: &T::Key) -> Option<T::Value> {
234-
self.get_map_mut(key).remove(key)
242+
self.get_map_mut(key).and_then(|m| m.remove(key))
235243
}
236244

237245
pub fn get(&self, key: &T::Key) -> Option<&T::Value> {
238246
self.get_map(key).and_then(|m| m.get(key))
239247
}
240248

241249
pub fn get_mut(&mut self, key: &T::Key) -> Option<&mut T::Value> {
242-
self.get_map_mut(key).get_mut(key)
250+
self.get_map_mut(key).and_then(|m| m.get_mut(key))
243251
}
244252

245253
pub fn has_key(&self, key: &T::Key) -> bool {
@@ -283,7 +291,7 @@ where
283291
key: &T::Key,
284292
update: impl FnOnce(Option<T::Value>) -> Option<T::Value>,
285293
) {
286-
let map = self.get_map_mut(key);
294+
let map = self.get_or_create_map_mut(key);
287295
if let Some(value) = map.get_mut(key) {
288296
let v = take(value);
289297
if let Some(v) = update(Some(v)) {

‎turbopack/crates/turbo-tasks-backend/src/data.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,9 @@ pub enum CachedDataItem {
313313
task: TaskId,
314314
value: (),
315315
},
316+
ChildrenCount {
317+
value: u32,
318+
},
316319

317320
// Cells
318321
CellData {
@@ -438,6 +441,7 @@ impl CachedDataItem {
438441
}
439442
CachedDataItem::Dirty { .. } => true,
440443
CachedDataItem::Child { task, .. } => !task.is_transient(),
444+
CachedDataItem::ChildrenCount { .. } => true,
441445
CachedDataItem::CellData { .. } => true,
442446
CachedDataItem::CellTypeMaxIndex { .. } => true,
443447
CachedDataItem::OutputDependency { target, .. } => !target.is_transient(),
@@ -502,6 +506,7 @@ impl CachedDataItemKey {
502506
}
503507
CachedDataItemKey::Dirty { .. } => true,
504508
CachedDataItemKey::Child { task, .. } => !task.is_transient(),
509+
CachedDataItemKey::ChildrenCount {} => true,
505510
CachedDataItemKey::CellData { .. } => true,
506511
CachedDataItemKey::CellTypeMaxIndex { .. } => true,
507512
CachedDataItemKey::OutputDependency { target, .. } => !target.is_transient(),
@@ -534,6 +539,7 @@ impl CachedDataItemKey {
534539
match self {
535540
CachedDataItemKey::Collectible { .. }
536541
| CachedDataItemKey::Child { .. }
542+
| CachedDataItemKey::ChildrenCount { .. }
537543
| CachedDataItemKey::CellData { .. }
538544
| CachedDataItemKey::CellTypeMaxIndex { .. }
539545
| CachedDataItemKey::OutputDependency { .. }
@@ -678,6 +684,7 @@ impl CachedDataItemValue {
678684
#[derive(Debug)]
679685
pub struct CachedDataUpdate {
680686
pub task: TaskId,
687+
// TODO generate CachedDataItemUpdate to avoid repeating the variant field 3 times
681688
pub key: CachedDataItemKey,
682689
pub value: Option<CachedDataItemValue>,
683690
pub old_value: Option<CachedDataItemValue>,

‎turbopack/crates/turbopack-core/src/resolve/mod.rs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ pub enum ModuleResolveResultItem {
7070
Error(Vc<RcStr>),
7171
Empty,
7272
Custom(u8),
73-
Unresolveable,
7473
}
7574

7675
#[turbo_tasks::value(shared)]
@@ -398,7 +397,6 @@ pub enum ResolveResultItem {
398397
Error(Vc<RcStr>),
399398
Empty,
400399
Custom(u8),
401-
Unresolveable,
402400
}
403401

404402
/// Represents the key for a request that leads to a certain results during
@@ -491,9 +489,6 @@ impl ValueToString for ResolveResult {
491489
ResolveResultItem::Custom(_) => {
492490
result.push_str("custom");
493491
}
494-
ResolveResultItem::Unresolveable => {
495-
result.push_str("unresolveable");
496-
}
497492
}
498493
result.push('\n');
499494
}
@@ -684,9 +679,6 @@ impl ResolveResult {
684679
ResolveResultItem::Custom(u8) => {
685680
ModuleResolveResultItem::Custom(u8)
686681
}
687-
ResolveResultItem::Unresolveable => {
688-
ModuleResolveResultItem::Unresolveable
689-
}
690682
},
691683
))
692684
}

‎turbopack/crates/turbopack-ecmascript/src/references/async_module.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ impl AsyncModule {
135135
}
136136
}
137137
ReferencedAsset::External(..) => None,
138-
ReferencedAsset::None => None,
138+
ReferencedAsset::None | ReferencedAsset::Unresolveable => None,
139139
})
140140
})
141141
.try_flat_join()

‎turbopack/crates/turbopack-ecmascript/src/references/esm/base.rs‎

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ pub enum ReferencedAsset {
4343
Some(Vc<Box<dyn EcmascriptChunkPlaceable>>),
4444
External(RcStr, ExternalType),
4545
None,
46+
Unresolveable,
4647
}
4748

4849
impl ReferencedAsset {
@@ -52,7 +53,7 @@ impl ReferencedAsset {
5253
ReferencedAsset::External(request, ty) => Some(magic_identifier::mangle(&format!(
5354
"{ty} external {request}"
5455
))),
55-
ReferencedAsset::None => None,
56+
ReferencedAsset::None | ReferencedAsset::Unresolveable => None,
5657
})
5758
}
5859

@@ -72,7 +73,11 @@ impl ReferencedAsset {
7273
#[turbo_tasks::function]
7374
pub async fn from_resolve_result(resolve_result: Vc<ModuleResolveResult>) -> Result<Vc<Self>> {
7475
// TODO handle multiple keyed results
75-
for (_key, result) in resolve_result.await?.primary.iter() {
76+
let result = resolve_result.await?;
77+
if result.primary.is_empty() {
78+
return Ok(ReferencedAsset::Unresolveable.cell());
79+
}
80+
for (_key, result) in result.primary.iter() {
7681
match result {
7782
ModuleResolveResultItem::External(request, ty) => {
7883
return Ok(ReferencedAsset::External(request.clone(), *ty).cell());
@@ -82,14 +87,14 @@ impl ReferencedAsset {
8287
Vc::try_resolve_downcast::<Box<dyn EcmascriptChunkPlaceable>>(module)
8388
.await?
8489
{
85-
return Ok(ReferencedAsset::cell(ReferencedAsset::Some(placeable)));
90+
return Ok(ReferencedAsset::Some(placeable).cell());
8691
}
8792
}
8893
// TODO ignore should probably be handled differently
8994
_ => {}
9095
}
9196
}
92-
Ok(ReferencedAsset::cell(ReferencedAsset::None))
97+
Ok(ReferencedAsset::None.cell())
9398
}
9499
}
95100

@@ -244,28 +249,21 @@ impl CodeGenerateable for EsmAssetReference {
244249
chunking_context: Vc<Box<dyn ChunkingContext>>,
245250
) -> Result<Vc<CodeGeneration>> {
246251
let this = &*self.await?;
247-
let chunking_type = self.chunking_type().await?;
248-
let resolved = self.resolve_reference().await?;
249-
250-
// Insert code that throws immediately at time of import if a request is
251-
// unresolvable
252-
if resolved.is_unresolveable_ref() {
253-
let request = request_to_string(this.request).await?.to_string();
254-
let stmt = Stmt::Expr(ExprStmt {
255-
expr: Box::new(throw_module_not_found_expr(&request)),
256-
span: DUMMY_SP,
257-
});
258-
return Ok(CodeGeneration::hoisted_stmt(
259-
format!("throw {request}").into(),
260-
stmt,
261-
));
262-
}
263252

264253
// only chunked references can be imported
265-
let result = if chunking_type.is_some() {
266-
let referenced_asset = self.get_referenced_asset().await?;
254+
let result = if this.annotations.chunking_type() != Some("none") {
267255
let import_externals = this.import_externals;
268-
if let Some(ident) = referenced_asset.get_ident().await? {
256+
let referenced_asset = self.get_referenced_asset().await?;
257+
if let ReferencedAsset::Unresolveable = &*referenced_asset {
258+
// Insert code that throws immediately at time of import if a request is
259+
// unresolvable
260+
let request = request_to_string(this.request).await?.to_string();
261+
let stmt = Stmt::Expr(ExprStmt {
262+
expr: Box::new(throw_module_not_found_expr(&request)),
263+
span: DUMMY_SP,
264+
});
265+
Some((format!("throw {request}").into(), stmt))
266+
} else if let Some(ident) = referenced_asset.get_ident().await? {
269267
let span = this
270268
.issue_source
271269
.await?
@@ -275,6 +273,9 @@ impl CodeGenerateable for EsmAssetReference {
275273
Span::new(BytePos(start as u32), BytePos(end as u32))
276274
});
277275
match &*referenced_asset {
276+
ReferencedAsset::Unresolveable => {
277+
unreachable!()
278+
}
278279
ReferencedAsset::Some(asset) => {
279280
let id = asset
280281
.as_chunk_item(Vc::upcast(chunking_context))

0 commit comments

Comments
 (0)