Skip to content

Commit 3d3def2

Browse files
sokraclaude
andauthored
Turbopack: optimize SelfTimeTree performance, memory usage, and fix range query boundaries (#92931)
### What? Optimize the `SelfTimeTree` data structure in the Turbopack trace server for better performance and lower memory usage when loading large trace files, and fix a correctness bug in range queries. ### Why? Two issues were identified: 1. **Performance**: The tree was splitting and rebalancing on every insert during bulk loading, and each `distribute_entries` call recursively triggered further split checks per entry, creating significant overhead. 2. **Correctness**: Range query boundary conditions used exclusive comparisons (`<`/`>`), incorrectly excluding entries that touched the exact start or end timestamp of the queried range. ### How? **Performance optimizations:** 1. **Batch split checks**: Add `insert_without_check` for inserting entries without triggering a split check per entry. In `distribute_entries`, entries are moved to children using `insert_without_check`, and `check_for_split` is called once per child after all entries are distributed (rather than once per entry). 2. **Lazy optimization with memory reclamation**: Add an `optimize()` method and a `for_each_in_range_optimize()` traversal method that distribute and rebalance nodes lazily: - `store.optimize()` is called when the reader reaches end-of-data (before waiting for more), doing a bulk pass to properly structure the tree after initial load. After distributing entries to children, each node calls `shrink_to_fit()` on its entries vec to release excess capacity. - `for_each_in_range_optimize()` distributes and rebalances each node it visits, so query paths are progressively optimized during normal use. 3. **Fix `rebalance()` bug**: `check_for_split()` was incorrectly called after merging subtrees during rebalance — replaced with `distribute_entries()` since the node already has children and needs redistribution, not a fresh split. **Correctness fix:** All range boundary comparisons in `for_each_in_range`, `for_each_in_range_optimize`, and `lookup_range_count` updated from exclusive (`<`/`>`) to inclusive (`<=`/`>=`), ensuring entries with timestamps exactly equal to the range endpoints are included in results. <!-- NEXT_JS_LLM_PR --> --------- Co-authored-by: Tobias Koppers <sokra@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent e2e8ba9 commit 3d3def2

3 files changed

Lines changed: 60 additions & 18 deletions

File tree

turbopack/crates/turbopack-trace-server/src/reader/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ impl TraceReader {
210210
match file.read(&mut chunk) {
211211
Ok(bytes_read) => {
212212
if bytes_read == 0 {
213+
self.store.write().optimize();
213214
if let Some(value) = self.wait_for_more_data(
214215
&mut file,
215216
&mut initial_read,
@@ -308,6 +309,7 @@ impl TraceReader {
308309
if err.kind() == io::ErrorKind::UnexpectedEof
309310
|| err.kind() == io::ErrorKind::InvalidInput
310311
{
312+
self.store.write().optimize();
311313
if let Some(value) = self.wait_for_more_data(
312314
&mut file,
313315
&mut initial_read,

turbopack/crates/turbopack-trace-server/src/self_time_tree.rs

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ impl<T> SelfTimeTree<T> {
5353
self.check_for_split();
5454
}
5555

56+
fn insert_without_check(&mut self, start: Timestamp, end: Timestamp, item: T) {
57+
self.count += 1;
58+
self.entries.push(SelfTimeEntry { start, end, item });
59+
}
60+
5661
fn check_for_split(&mut self) {
5762
if self.entries.len() >= SPLIT_COUNT {
5863
let spanning_entries = if let Some(children) = &mut self.children {
@@ -66,6 +71,17 @@ impl<T> SelfTimeTree<T> {
6671
}
6772
}
6873

74+
pub fn optimize(&mut self) {
75+
if self.children.is_some() {
76+
self.distribute_entries();
77+
self.rebalance();
78+
let children = self.children.as_mut().unwrap();
79+
children.left.optimize();
80+
children.right.optimize();
81+
}
82+
self.entries.shrink_to_fit();
83+
}
84+
6985
fn split(&mut self) {
7086
debug_assert!(!self.entries.is_empty());
7187
self.distribute_entries();
@@ -100,16 +116,18 @@ impl<T> SelfTimeTree<T> {
100116
let SelfTimeEntry { start, end, .. } = self.entries[i];
101117
if end <= children.split_point {
102118
let SelfTimeEntry { start, end, item } = self.entries.swap_remove(i);
103-
children.left.insert(start, end, item);
119+
children.left.insert_without_check(start, end, item);
104120
} else if start >= children.split_point {
105121
let SelfTimeEntry { start, end, item } = self.entries.swap_remove(i);
106-
children.right.insert(start, end, item);
122+
children.right.insert_without_check(start, end, item);
107123
} else {
108124
self.entries.swap(i, children.spanning_entries);
109125
children.spanning_entries += 1;
110126
i += 1;
111127
}
112128
}
129+
children.left.check_for_split();
130+
children.right.check_for_split();
113131
}
114132

115133
fn rebalance(&mut self) {
@@ -188,7 +206,7 @@ impl<T> SelfTimeTree<T> {
188206
self.entries.append(right_entries);
189207
*right = take(right_right);
190208
*spanning_entries = 0;
191-
self.check_for_split();
209+
self.distribute_entries();
192210
}
193211
}
194212
}
@@ -198,18 +216,18 @@ impl<T> SelfTimeTree<T> {
198216
pub fn lookup_range_count(&self, start: Timestamp, end: Timestamp) -> Timestamp {
199217
let mut total_count = Timestamp::ZERO;
200218
for entry in &self.entries {
201-
if entry.start < end && entry.end > start {
219+
if entry.start <= end && entry.end >= start {
202220
let start = std::cmp::max(entry.start, start);
203221
let end = std::cmp::min(entry.end, end);
204222
let span = end - start;
205223
total_count += span;
206224
}
207225
}
208226
if let Some(children) = &self.children {
209-
if start < children.split_point {
227+
if start <= children.split_point {
210228
total_count += children.left.lookup_range_count(start, end);
211229
}
212-
if end > children.split_point {
230+
if end >= children.split_point {
213231
total_count += children.right.lookup_range_count(start, end);
214232
}
215233
}
@@ -225,7 +243,7 @@ impl<T> SelfTimeTree<T> {
225243
}
226244
let mut current_count = 0;
227245
let mut changes = Vec::new();
228-
self.for_each_in_range(start, end, |s, e, _| {
246+
self.for_each_in_range(start, end, &mut |s, e, _| {
229247
if s <= start {
230248
current_count += 1;
231249
} else {
@@ -260,28 +278,44 @@ impl<T> SelfTimeTree<T> {
260278
&self,
261279
start: Timestamp,
262280
end: Timestamp,
263-
mut f: impl FnMut(Timestamp, Timestamp, &T),
281+
f: &mut impl FnMut(Timestamp, Timestamp, &T),
264282
) {
265-
self.for_each_in_range_ref(start, end, &mut f);
283+
for entry in &self.entries {
284+
if entry.start <= end && entry.end >= start {
285+
f(entry.start, entry.end, &entry.item);
286+
}
287+
}
288+
if let Some(children) = &self.children {
289+
if start <= children.split_point {
290+
children.left.for_each_in_range(start, end, f);
291+
}
292+
if end >= children.split_point {
293+
children.right.for_each_in_range(start, end, f);
294+
}
295+
}
266296
}
267297

268-
fn for_each_in_range_ref(
269-
&self,
298+
pub fn for_each_in_range_optimize(
299+
&mut self,
270300
start: Timestamp,
271301
end: Timestamp,
272302
f: &mut impl FnMut(Timestamp, Timestamp, &T),
273303
) {
304+
if self.children.is_some() {
305+
self.distribute_entries();
306+
self.rebalance();
307+
}
274308
for entry in &self.entries {
275-
if entry.start < end && entry.end > start {
309+
if entry.start <= end && entry.end >= start {
276310
f(entry.start, entry.end, &entry.item);
277311
}
278312
}
279-
if let Some(children) = &self.children {
280-
if start < children.split_point {
281-
children.left.for_each_in_range_ref(start, end, f);
313+
if let Some(children) = &mut self.children {
314+
if start <= children.split_point {
315+
children.left.for_each_in_range_optimize(start, end, f);
282316
}
283-
if end > children.split_point {
284-
children.right.for_each_in_range_ref(start, end, f);
317+
if end >= children.split_point {
318+
children.right.for_each_in_range_optimize(start, end, f);
285319
}
286320
}
287321
}

turbopack/crates/turbopack-trace-server/src/store.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,12 @@ impl Store {
9191
self.memory_samples.clear();
9292
}
9393

94+
pub fn optimize(&mut self) {
95+
if let Some(tree) = self.self_time_tree.as_mut() {
96+
tree.optimize();
97+
}
98+
}
99+
94100
pub fn has_time_info(&self) -> bool {
95101
self.self_time_tree
96102
.as_ref()
@@ -192,7 +198,7 @@ impl Store {
192198
) {
193199
if let Some(tree) = self.self_time_tree.as_mut() {
194200
if Timestamp::from_value(*self.max_self_time_lookup_time.get_mut()) >= start {
195-
tree.for_each_in_range(start, end, |_, _, span| {
201+
tree.for_each_in_range_optimize(start, end, &mut |_, _, span| {
196202
outdated_spans.insert(*span);
197203
});
198204
}

0 commit comments

Comments
 (0)