Skip to content

Commit 67aa5f3

Browse files
authored
[turbopack] Drop duration and allocation tracking from CaptureFuture (#85534)
This data was mostly unused * allocation tracking was completely dead. So this saves a bit of memory and some data copies * duration tracking was only used for task statistics. This was a relatively new feature and not particularly useful. I considered wrapping it in a `feature` but dropping is simpler and this data is very redundant with tracing output anyway.
1 parent 575c8a7 commit 67aa5f3

File tree

7 files changed

+16
-167
lines changed

7 files changed

+16
-167
lines changed

turbopack/crates/turbo-tasks-backend/benches/overhead.rs

Lines changed: 1 addition & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::time::{Duration, Instant};
33
use criterion::{BenchmarkId, Criterion, black_box};
44
use futures::{FutureExt, StreamExt, stream::FuturesUnordered};
55
use tokio::spawn;
6-
use turbo_tasks::{TurboTasks, TurboTasksApi};
6+
use turbo_tasks::TurboTasks;
77
use turbo_tasks_backend::{BackendOptions, TurboTasksBackend, noop_backing_storage};
88

99
#[global_allocator]
@@ -79,15 +79,6 @@ pub fn overhead(c: &mut Criterion) {
7979
run_turbo::<Uncached>(&rt, b, d, false);
8080
},
8181
);
82-
// Same as turbo-uncached but reports the time as measured by turbotasks itself
83-
// This allows us to understand the cost of the indirection within turbotasks
84-
group.bench_with_input(
85-
BenchmarkId::new("turbo-uncached-stats", micros),
86-
&duration,
87-
|b, &d| {
88-
run_turbo_stats(&rt, b, d);
89-
},
90-
);
9182

9283
group.bench_with_input(
9384
BenchmarkId::new("turbo-cached-same-keys", micros),
@@ -224,29 +215,3 @@ fn run_turbo<Mode: TurboMode>(
224215
}
225216
});
226217
}
227-
228-
fn run_turbo_stats(rt: &tokio::runtime::Runtime, b: &mut criterion::Bencher<'_>, d: Duration) {
229-
b.to_async(rt).iter_custom(|iters| {
230-
// It is important to create the tt instance here to ensure the cache is not shared across
231-
// iterations.
232-
let tt = TurboTasks::new(TurboTasksBackend::new(
233-
BackendOptions {
234-
storage_mode: None,
235-
..Default::default()
236-
},
237-
noop_backing_storage(),
238-
));
239-
let stats = tt.task_statistics().enable().clone();
240-
241-
async move {
242-
tt.run_once(async move {
243-
for i in 0..iters {
244-
black_box(busy_turbo(i, black_box(d)).await?);
245-
}
246-
Ok(stats.get(&BUSY_TURBO_FUNCTION).duration)
247-
})
248-
.await
249-
.unwrap()
250-
}
251-
});
252-
}

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

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -391,14 +391,6 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
391391
self.task_statistics
392392
.map(|stats| stats.increment_cache_miss(task_type.native_fn));
393393
}
394-
395-
fn track_task_duration(&self, task_id: TaskId, duration: std::time::Duration) {
396-
self.task_statistics.map(|stats| {
397-
if let Some(task_type) = self.task_cache.lookup_reverse(&task_id) {
398-
stats.increment_execution_duration(task_type.native_fn, duration);
399-
}
400-
});
401-
}
402394
}
403395

404396
pub(crate) struct OperationGuard<'a, B: BackingStorage> {
@@ -1751,8 +1743,6 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
17511743
fn task_execution_completed(
17521744
&self,
17531745
task_id: TaskId,
1754-
duration: Duration,
1755-
_memory_usage: usize,
17561746
result: Result<RawVc, TurboTasksExecutionError>,
17571747
cell_counters: &AutoMap<ValueTypeId, u32, BuildHasherDefault<FxHasher>, 8>,
17581748
stateful: bool,
@@ -1776,8 +1766,6 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
17761766
let span = tracing::trace_span!("task execution completed", immutable = Empty).entered();
17771767
let mut ctx = self.execute_context(turbo_tasks);
17781768

1779-
self.track_task_duration(task_id, duration);
1780-
17811769
let Some(TaskExecutionCompletePrepareResult {
17821770
new_children,
17831771
mut removed_data,
@@ -3194,8 +3182,6 @@ impl<B: BackingStorage> Backend for TurboTasksBackend<B> {
31943182
fn task_execution_completed(
31953183
&self,
31963184
task_id: TaskId,
3197-
_duration: Duration,
3198-
_memory_usage: usize,
31993185
result: Result<RawVc, TurboTasksExecutionError>,
32003186
cell_counters: &AutoMap<ValueTypeId, u32, BuildHasherDefault<FxHasher>, 8>,
32013187
stateful: bool,
@@ -3204,8 +3190,6 @@ impl<B: BackingStorage> Backend for TurboTasksBackend<B> {
32043190
) -> bool {
32053191
self.0.task_execution_completed(
32063192
task_id,
3207-
_duration,
3208-
_memory_usage,
32093193
result,
32103194
cell_counters,
32113195
stateful,

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use std::{
66
hash::{BuildHasherDefault, Hash},
77
pin::Pin,
88
sync::Arc,
9-
time::Duration,
109
};
1110

1211
use anyhow::{Result, anyhow};
@@ -541,8 +540,6 @@ pub trait Backend: Sync + Send {
541540
fn task_execution_completed(
542541
&self,
543542
task: TaskId,
544-
duration: Duration,
545-
memory_usage: usize,
546543
result: Result<RawVc, TurboTasksExecutionError>,
547544
cell_counters: &AutoMap<ValueTypeId, u32, BuildHasherDefault<FxHasher>, 8>,
548545
stateful: bool,
Lines changed: 4 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,74 +1,31 @@
11
use std::{
22
borrow::Cow,
3-
cell::RefCell,
43
fmt::Display,
54
future::Future,
65
panic,
76
pin::Pin,
87
task::{Context, Poll},
9-
time::{Duration, Instant},
108
};
119

1210
use anyhow::Result;
1311
use pin_project_lite::pin_project;
1412
use serde::{Deserialize, Serialize};
15-
use turbo_tasks_malloc::{AllocationInfo, TurboMalloc};
1613

1714
use crate::{backend::TurboTasksExecutionErrorMessage, panic_hooks::LAST_ERROR_LOCATION};
1815

19-
struct ThreadLocalData {
20-
duration: Duration,
21-
allocations: usize,
22-
deallocations: usize,
23-
}
24-
25-
thread_local! {
26-
static EXTRA: RefCell<Option<*mut ThreadLocalData>> = const { RefCell::new(None) };
27-
}
28-
2916
pin_project! {
3017
pub struct CaptureFuture<T, F: Future<Output = T>> {
3118
#[pin]
3219
future: F,
33-
duration: Duration,
34-
allocations: AllocationInfo,
3520
}
3621
}
3722

3823
impl<T, F: Future<Output = T>> CaptureFuture<T, F> {
3924
pub fn new(future: F) -> Self {
40-
Self {
41-
future,
42-
duration: Duration::ZERO,
43-
allocations: AllocationInfo::ZERO,
44-
}
25+
Self { future }
4526
}
4627
}
4728

48-
fn try_with_thread_local_data(f: impl FnOnce(&mut ThreadLocalData)) {
49-
EXTRA.with_borrow(|cell| {
50-
if let Some(data) = cell {
51-
// Safety: This data is thread local and only accessed in this thread
52-
unsafe {
53-
f(&mut **data);
54-
}
55-
}
56-
});
57-
}
58-
59-
pub fn add_duration(duration: Duration) {
60-
try_with_thread_local_data(|data| {
61-
data.duration += duration;
62-
});
63-
}
64-
65-
pub fn add_allocation_info(alloc_info: AllocationInfo) {
66-
try_with_thread_local_data(|data| {
67-
data.allocations += alloc_info.allocations;
68-
data.deallocations += alloc_info.deallocations;
69-
});
70-
}
71-
7229
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
7330
pub struct TurboTasksPanic {
7431
pub message: TurboTasksExecutionErrorMessage,
@@ -93,21 +50,10 @@ impl Display for TurboTasksPanic {
9350
}
9451

9552
impl<T, F: Future<Output = T>> Future for CaptureFuture<T, F> {
96-
type Output = (Result<T, TurboTasksPanic>, Duration, AllocationInfo);
53+
type Output = Result<T, TurboTasksPanic>;
9754

9855
fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
9956
let this = self.project();
100-
let start = Instant::now();
101-
let start_allocations = TurboMalloc::allocation_counters();
102-
let guard = ThreadLocalDataDropGuard;
103-
let mut data = ThreadLocalData {
104-
duration: Duration::ZERO,
105-
allocations: 0,
106-
deallocations: 0,
107-
};
108-
EXTRA.with_borrow_mut(|cell| {
109-
*cell = Some(&mut data as *mut ThreadLocalData);
110-
});
11157

11258
let result =
11359
panic::catch_unwind(panic::AssertUnwindSafe(|| this.future.poll(cx))).map_err(|err| {
@@ -132,25 +78,10 @@ impl<T, F: Future<Output = T>> Future for CaptureFuture<T, F> {
13278
})
13379
});
13480

135-
drop(guard);
136-
let elapsed = start.elapsed();
137-
let allocations = start_allocations.until_now();
138-
*this.duration += elapsed + data.duration;
139-
*this.allocations += allocations;
14081
match result {
141-
Err(err) => Poll::Ready((Err(err), *this.duration, this.allocations.clone())),
142-
Ok(Poll::Ready(r)) => Poll::Ready((Ok(r), *this.duration, this.allocations.clone())),
82+
Err(err) => Poll::Ready(Err(err)),
83+
Ok(Poll::Ready(r)) => Poll::Ready(Ok(r)),
14384
Ok(Poll::Pending) => Poll::Pending,
14485
}
14586
}
14687
}
147-
148-
struct ThreadLocalDataDropGuard;
149-
150-
impl Drop for ThreadLocalDataDropGuard {
151-
fn drop(&mut self) {
152-
EXTRA.with_borrow_mut(|cell| {
153-
*cell = None;
154-
});
155-
}
156-
}

turbopack/crates/turbo-tasks/src/manager.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ impl<B: Backend + 'static> TurboTasks<B> {
578578
.scope(
579579
self.pin(),
580580
CURRENT_TASK_STATE.scope(current_task_state, async {
581-
let (result, _duration, _alloc_info) = CaptureFuture::new(future).await;
581+
let result = CaptureFuture::new(future).await;
582582

583583
// wait for all spawned local tasks using `local` to finish
584584
let ltt =
@@ -736,7 +736,7 @@ impl<B: Backend + 'static> TurboTasks<B> {
736736
};
737737

738738
async {
739-
let (result, duration, alloc_info) = CaptureFuture::new(future).await;
739+
let result = CaptureFuture::new(future).await;
740740

741741
// wait for all spawned local tasks using `local` to finish
742742
let ltt = CURRENT_TASK_STATE
@@ -758,8 +758,6 @@ impl<B: Backend + 'static> TurboTasks<B> {
758758
.with(|ts| ts.write().unwrap().cell_counters.take().unwrap());
759759
this.backend.task_execution_completed(
760760
task_id,
761-
duration,
762-
alloc_info.memory_usage(),
763761
result,
764762
&cell_counters,
765763
stateful,
@@ -835,7 +833,7 @@ impl<B: Backend + 'static> TurboTasks<B> {
835833
let TaskExecutionSpec { future, span } =
836834
crate::task::local_task::get_local_task_execution_spec(&*this, &ty, persistence);
837835
async move {
838-
let (result, _duration, _memory_usage) = CaptureFuture::new(future).await;
836+
let result = CaptureFuture::new(future).await;
839837

840838
let result = match result {
841839
Ok(Ok(raw_vc)) => Ok(raw_vc),

turbopack/crates/turbo-tasks/src/spawn.rs

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,24 +3,20 @@ use std::{
33
pin::Pin,
44
task::{Context, Poll},
55
thread,
6-
time::{Duration, Instant},
76
};
87

98
use anyhow::Result;
109
use futures::{FutureExt, ready};
1110
use tokio::runtime::Handle;
1211
use tracing::{Instrument, Span, info_span};
13-
use turbo_tasks_malloc::{AllocationInfo, TurboMalloc};
1412

1513
use crate::{
16-
TurboTasksPanic,
17-
capture_future::{self, CaptureFuture},
18-
manager::turbo_tasks_future_scope,
19-
turbo_tasks, turbo_tasks_scope,
14+
TurboTasksPanic, capture_future::CaptureFuture, manager::turbo_tasks_future_scope, turbo_tasks,
15+
turbo_tasks_scope,
2016
};
2117

2218
pub struct JoinHandle<T> {
23-
join_handle: tokio::task::JoinHandle<(Result<T, TurboTasksPanic>, Duration, AllocationInfo)>,
19+
join_handle: tokio::task::JoinHandle<Result<T, TurboTasksPanic>>,
2420
}
2521

2622
impl<T: Send + 'static> JoinHandle<T> {
@@ -35,14 +31,10 @@ impl<T> Future for JoinHandle<T> {
3531
fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
3632
let this = self.get_mut();
3733
match ready!(this.join_handle.poll_unpin(cx)) {
38-
Ok((res, duration, alloc_info)) => {
39-
capture_future::add_duration(duration);
40-
capture_future::add_allocation_info(alloc_info);
41-
match res {
42-
Ok(res) => Poll::Ready(res),
43-
Err(e) => resume_unwind(e.into_panic()),
44-
}
45-
}
34+
Ok(res) => match res {
35+
Ok(res) => Poll::Ready(res),
36+
Err(e) => resume_unwind(e.into_panic()),
37+
},
4638
Err(e) => resume_unwind(e.into_panic()),
4739
}
4840
}
@@ -71,10 +63,7 @@ pub fn spawn_blocking<T: Send + 'static>(
7163
let span = Span::current();
7264
let join_handle = tokio::task::spawn_blocking(|| {
7365
let _guard = span.entered();
74-
let start = Instant::now();
75-
let start_allocations = TurboMalloc::allocation_counters();
76-
let r = turbo_tasks_scope(turbo_tasks, func);
77-
(Ok(r), start.elapsed(), start_allocations.until_now())
66+
Ok(turbo_tasks_scope(turbo_tasks, func))
7867
});
7968
JoinHandle { join_handle }
8069
}

turbopack/crates/turbo-tasks/src/task_statistics.rs

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -46,17 +46,6 @@ impl TaskStatistics {
4646
self.with_task_type_statistics(native_fn, |stats| stats.cache_miss += 1)
4747
}
4848

49-
pub fn increment_execution_duration(
50-
&self,
51-
native_fn: &'static NativeFunction,
52-
duration: std::time::Duration,
53-
) {
54-
self.with_task_type_statistics(native_fn, |stats| {
55-
stats.executions += 1;
56-
stats.duration += duration
57-
})
58-
}
59-
6049
fn with_task_type_statistics(
6150
&self,
6251
native_fn: &'static NativeFunction,
@@ -75,10 +64,6 @@ impl TaskStatistics {
7564
pub struct TaskFunctionStatistics {
7665
pub cache_hit: u32,
7766
pub cache_miss: u32,
78-
// Generally executions == cache_miss, however they can diverge when there are invalidations.
79-
// The caller gets one cache miss but we might execute multiple times.
80-
pub executions: u32,
81-
pub duration: std::time::Duration,
8267
}
8368

8469
impl Serialize for TaskStatistics {

0 commit comments

Comments
 (0)