Skip to content

Commit 2aa59ed

Browse files
bgwbgw
authored
chore(turbo-tasks): Remove argument resolution from backends, only allow local-task-based argument resolution (#75214)
Removes the (now default) feature flag for local task argument resolution. With local-task based argument resolution in place, we don't need to support argument resolution in the backend, which simplifies some code.
1 parent 7b8fc75 commit 2aa59ed

File tree

8 files changed

+295
-579
lines changed

8 files changed

+295
-579
lines changed

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

Lines changed: 7 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -866,7 +866,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
866866
panic!(
867867
"Calling transient function {} from persistent function function {} is not allowed",
868868
task_type.get_name(),
869-
parent_task_type.map_or_else(|| "unknown".into(), |t| t.get_name())
869+
parent_task_type.map_or("unknown", |t| t.get_name())
870870
);
871871
}
872872
if let Some(task_id) = self.task_cache.lookup_forward(&task_type) {
@@ -963,10 +963,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
963963

964964
fn try_get_function_id(&self, task_id: TaskId) -> Option<FunctionId> {
965965
self.lookup_task_type(task_id)
966-
.and_then(|task_type| match &*task_type {
967-
CachedTaskType::Native { fn_type, .. } => Some(*fn_type),
968-
_ => None,
969-
})
966+
.map(|task_type| task_type.fn_type)
970967
}
971968

972969
fn try_start_task_execution(
@@ -1105,64 +1102,13 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
11051102
}
11061103

11071104
let (span, future) = match task_type {
1108-
TaskType::Cached(task_type) => match &*task_type {
1109-
CachedTaskType::Native { fn_type, this, arg } => (
1105+
TaskType::Cached(task_type) => {
1106+
let CachedTaskType { fn_type, this, arg } = &*task_type;
1107+
(
11101108
registry::get_function(*fn_type).span(task_id.persistence()),
11111109
registry::get_function(*fn_type).execute(*this, &**arg),
1112-
),
1113-
CachedTaskType::ResolveNative { fn_type, .. } => {
1114-
let span = registry::get_function(*fn_type).resolve_span(task_id.persistence());
1115-
let turbo_tasks = turbo_tasks.pin();
1116-
(
1117-
span,
1118-
Box::pin(async move {
1119-
let CachedTaskType::ResolveNative { fn_type, this, arg } = &*task_type
1120-
else {
1121-
unreachable!()
1122-
};
1123-
CachedTaskType::run_resolve_native(
1124-
*fn_type,
1125-
*this,
1126-
&**arg,
1127-
task_id.persistence(),
1128-
turbo_tasks,
1129-
)
1130-
.await
1131-
}) as Pin<Box<dyn Future<Output = _> + Send + '_>>,
1132-
)
1133-
}
1134-
CachedTaskType::ResolveTrait {
1135-
trait_type,
1136-
method_name,
1137-
..
1138-
} => {
1139-
let span = registry::get_trait(*trait_type).resolve_span(method_name);
1140-
let turbo_tasks = turbo_tasks.pin();
1141-
(
1142-
span,
1143-
Box::pin(async move {
1144-
let CachedTaskType::ResolveTrait {
1145-
trait_type,
1146-
method_name,
1147-
this,
1148-
arg,
1149-
} = &*task_type
1150-
else {
1151-
unreachable!()
1152-
};
1153-
CachedTaskType::run_resolve_trait(
1154-
*trait_type,
1155-
method_name.clone(),
1156-
*this,
1157-
&**arg,
1158-
task_id.persistence(),
1159-
turbo_tasks,
1160-
)
1161-
.await
1162-
}) as Pin<Box<dyn Future<Output = _> + Send + '_>>,
1163-
)
1164-
}
1165-
},
1110+
)
1111+
}
11661112
TaskType::Transient(task_type) => {
11671113
let task_type = task_type.clone();
11681114
let span = tracing::trace_span!("turbo_tasks::root_task");

‎turbopack/crates/turbo-tasks-memory/src/memory_backend.rs‎

Lines changed: 8 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -337,72 +337,14 @@ impl MemoryBackend {
337337
&self.task_statistics
338338
}
339339

340-
fn track_cache_hit(
341-
&self,
342-
task_type: &PreHashed<CachedTaskType>,
343-
turbo_tasks: &dyn TurboTasksBackendApi<MemoryBackend>,
344-
) {
345-
self.task_statistics().map(|stats| match &**task_type {
346-
CachedTaskType::ResolveNative {
347-
fn_type: function_id,
348-
this: _,
349-
arg: _,
350-
}
351-
| CachedTaskType::Native {
352-
fn_type: function_id,
353-
this: _,
354-
arg: _,
355-
} => {
356-
stats.increment_cache_hit(*function_id);
357-
}
358-
CachedTaskType::ResolveTrait {
359-
trait_type,
360-
method_name: name,
361-
this,
362-
arg: _,
363-
} => {
364-
// HACK: Resolve the this argument (`self`) in order to attribute the cache hit
365-
// to the concrete trait implementation, rather than the dynamic trait method.
366-
// This ensures cache hits and misses are both attributed to the same thing.
367-
//
368-
// Because this task already resolved, in most cases `self` should either be
369-
// resolved, or already in the process of being resolved.
370-
//
371-
// However, `self` could become unloaded due to cache eviction, and this might
372-
// trigger an otherwise unnecessary re-evalutation.
373-
//
374-
// This is a potentially okay trade-off as long as we don't log statistics by
375-
// default. The alternative would be to store function ids on completed
376-
// ResolveTrait tasks.
377-
let trait_type = *trait_type;
378-
let name = name.clone();
379-
let this = *this;
380-
let stats = Arc::clone(stats);
381-
turbo_tasks.run_once(Box::pin(async move {
382-
let function_id =
383-
CachedTaskType::resolve_trait_method(trait_type, name, this).await?;
384-
stats.increment_cache_hit(function_id);
385-
Ok(())
386-
}));
387-
}
388-
});
340+
fn track_cache_hit(&self, task_type: &PreHashed<CachedTaskType>) {
341+
self.task_statistics()
342+
.map(|stats| stats.increment_cache_hit(task_type.fn_type));
389343
}
390344

391345
fn track_cache_miss(&self, task_type: &PreHashed<CachedTaskType>) {
392-
self.task_statistics().map(|stats| match &**task_type {
393-
CachedTaskType::Native {
394-
fn_type: function_id,
395-
this: _,
396-
arg: _,
397-
} => {
398-
stats.increment_cache_miss(*function_id);
399-
}
400-
CachedTaskType::ResolveTrait { .. } | CachedTaskType::ResolveNative { .. } => {
401-
// these types re-execute themselves as `Native` after
402-
// resolving their arguments, skip counting their
403-
// executions here to avoid double-counting
404-
}
405-
});
346+
self.task_statistics()
347+
.map(|stats| stats.increment_cache_miss(task_type.fn_type));
406348
}
407349
}
408350

@@ -711,7 +653,7 @@ impl Backend for MemoryBackend {
711653
self.lookup_and_connect_task(parent_task, &self.task_cache, &task_type, turbo_tasks)
712654
{
713655
// fast pass without creating a new task
714-
self.track_cache_hit(&task_type, turbo_tasks);
656+
self.track_cache_hit(&task_type);
715657
task
716658
} else {
717659
self.track_cache_miss(&task_type);
@@ -754,7 +696,7 @@ impl Backend for MemoryBackend {
754696
turbo_tasks,
755697
) {
756698
// fast pass without creating a new task
757-
self.track_cache_hit(&task_type, turbo_tasks);
699+
self.track_cache_hit(&task_type);
758700
task
759701
} else {
760702
self.track_cache_miss(&task_type);
@@ -785,7 +727,7 @@ impl Backend for MemoryBackend {
785727

786728
fn try_get_function_id(&self, task_id: TaskId) -> Option<FunctionId> {
787729
self.with_task(task_id, |task| match &task.ty {
788-
TaskType::Persistent { ty } => ty.try_get_function_id(),
730+
TaskType::Persistent { ty } => Some(ty.fn_type),
789731
_ => None,
790732
})
791733
}

‎turbopack/crates/turbo-tasks-memory/src/task.rs‎

Lines changed: 12 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ impl Task {
602602
aggregation_context.apply_queued_updates();
603603
}
604604

605-
pub(crate) fn get_function_name(&self) -> Option<Cow<'static, str>> {
605+
pub(crate) fn get_function_name(&self) -> Option<&'static str> {
606606
if let TaskType::Persistent { ty, .. } | TaskType::Transient { ty, .. } = &self.ty {
607607
Some(ty.get_name())
608608
} else {
@@ -743,7 +743,7 @@ impl Task {
743743
)
744744
}
745745
};
746-
self.make_execution_future(backend, turbo_tasks)
746+
self.make_execution_future()
747747
};
748748
aggregation_context.apply_queued_updates();
749749
Some(TaskExecutionSpec { future, span })
@@ -752,8 +752,6 @@ impl Task {
752752
/// Prepares task execution and returns a future that will execute the task.
753753
fn make_execution_future<'a>(
754754
self: &'a Task,
755-
_backend: &MemoryBackend,
756-
turbo_tasks: &dyn TurboTasksBackendApi<MemoryBackend>,
757755
) -> (
758756
Pin<Box<dyn Future<Output = Result<RawVc>> + Send + 'a>>,
759757
Span,
@@ -766,58 +764,19 @@ impl Task {
766764
mutex.lock().take().expect("Task can only be executed once"),
767765
tracing::trace_span!("turbo_tasks::once_task"),
768766
),
769-
TaskType::Persistent { ty, .. } | TaskType::Transient { ty, .. } => match &***ty {
770-
CachedTaskType::Native {
771-
fn_type: native_fn_id,
772-
this,
773-
arg,
774-
} => {
775-
let func = registry::get_function(*native_fn_id);
776-
let span = func.span(self.id.persistence());
777-
let entered = span.enter();
778-
let future = func.execute(*this, &**arg);
779-
drop(entered);
780-
(future, span)
781-
}
782-
CachedTaskType::ResolveNative {
767+
TaskType::Persistent { ty, .. } | TaskType::Transient { ty, .. } => {
768+
let CachedTaskType {
783769
fn_type: native_fn_id,
784770
this,
785771
arg,
786-
} => {
787-
let func = registry::get_function(*native_fn_id);
788-
let span = func.resolve_span(self.id.persistence());
789-
let entered = span.enter();
790-
let future = Box::pin(CachedTaskType::run_resolve_native(
791-
*native_fn_id,
792-
*this,
793-
&**arg,
794-
self.id.persistence(),
795-
turbo_tasks.pin(),
796-
));
797-
drop(entered);
798-
(future, span)
799-
}
800-
CachedTaskType::ResolveTrait {
801-
trait_type: trait_type_id,
802-
method_name: name,
803-
this,
804-
arg,
805-
} => {
806-
let trait_type = registry::get_trait(*trait_type_id);
807-
let span = trait_type.resolve_span(name);
808-
let entered = span.enter();
809-
let future = Box::pin(CachedTaskType::run_resolve_trait(
810-
*trait_type_id,
811-
name.clone(),
812-
*this,
813-
&**arg,
814-
self.id.persistence(),
815-
turbo_tasks.pin(),
816-
));
817-
drop(entered);
818-
(future, span)
819-
}
820-
},
772+
} = &***ty;
773+
let func = registry::get_function(*native_fn_id);
774+
let span = func.span(self.id.persistence());
775+
let entered = span.enter();
776+
let future = func.execute(*this, &**arg);
777+
drop(entered);
778+
(future, span)
779+
}
821780
}
822781
}
823782

‎turbopack/crates/turbo-tasks-memory/tests/task_statistics.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,13 +186,13 @@ async fn test_no_execution() {
186186
.await;
187187
}
188188

189-
// Internally, this function uses `CachedTaskType::Native`.
189+
// Internally, this function uses `PersistentTaskType`.
190190
#[turbo_tasks::function]
191191
fn double(val: u64) -> Vc<u64> {
192192
Vc::cell(val * 2)
193193
}
194194

195-
// Internally, this function uses `CachedTaskType::ResolveNative`.
195+
// Internally, this function uses `LocalTaskType::ResolveNative`.
196196
#[turbo_tasks::function]
197197
async fn double_vc(val: Vc<u64>) -> Result<Vc<u64>> {
198198
let val = *val.await?;

‎turbopack/crates/turbo-tasks/Cargo.toml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,8 @@ edition = "2021"
99
bench = false
1010

1111
[features]
12-
default = ["local_resolution"]
1312
tokio_tracing = ["tokio/tracing"]
1413
hanging_detection = []
15-
local_resolution = []
1614

1715
# TODO(bgw): This feature is here to unblock turning on local tasks by default. It's only turned on
1816
# in unit tests. This will be removed very soon.

0 commit comments

Comments
 (0)