Skip to content

Commit 61f0853

Browse files
authored
[turbopack] Fix a crash under turbopackPersistentCaching (#82592)
Address a crash found at head when testing caching while switching back and forth from `dev` and `build`. ``` thread '<unnamed>' panicked at turbopack/crates/turbo-persistence/src/db.rs:798:17: attempt to subtract with overflow stack backtrace: 0: __rustc::rust_begin_unwind 1: core::panicking::panic_fmt 2: core::panicking::panic_const::panic_const_sub_overflow 3: <core::iter::adapters::map::Map<core::slice::iter::Iter<alloc::vec::Vec<<turbo_persistence::db::TurboPersistence>::compact_internal::SstWithRange>>, <turbo_persistence::db::TurboPersistence>::compact_internal::{closure#2}> as core::iter::traits::iterator::Iterator>::fold::<(), core::iter::traits::iterator::Iterator::for_each::call<alloc::vec::Vec<smallvec::SmallVec<[usize; 1]>>, <alloc::vec::Vec<alloc::vec::Vec<smallvec::SmallVec<[usize; 1]>>>>::extend_trusted<core::iter::adapters::map::Map<core::slice::iter::Iter<alloc::vec::Vec<<turbo_persistence::db::TurboPersistence>::compact_internal::SstWithRange>>, <turbo_persistence::db::TurboPersistence>::compact_internal::{closure#2}>>::{closure#0}>::{closure#0}> 4: <alloc::vec::Vec<alloc::vec::Vec<smallvec::SmallVec<[usize; 1]>>> as alloc::vec::spec_from_iter_nested::SpecFromIterNested<alloc::vec::Vec<smallvec::SmallVec<[usize; 1]>>, core::iter::adapters::map::Map<core::slice::iter::Iter<alloc::vec::Vec<<turbo_persistence::db::TurboPersistence>::compact_internal::SstWithRange>>, <turbo_persistence::db::TurboPersistence>::compact_internal::{closure#2}>>>::from_iter 5: <turbo_persistence::db::TurboPersistence>::compact ``` Scenario: * build turbopack and next * cd bench/module-cost * enable persistent caching * pnpm i * pnpm dev-turbopack * load localhost:3000 and click all the buttons * pnpm build-turbopack * pnpm dev-turbopack * load localhost:3000 and see the crash ## What? The `get_merge_segments` function and its caller had a different idea about what `max_merge_segment_count` means. Unify how this is accounted to avoid underflow.
1 parent 6881843 commit 61f0853

File tree

2 files changed

+11
-7
lines changed

2 files changed

+11
-7
lines changed

turbopack/crates/turbo-persistence/src/compaction/selector.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -200,10 +200,13 @@ fn total_duplication_size(duplication: &IntervalMap<Option<DuplicationInfo>>) ->
200200

201201
type MergeSegments = Vec<SmallVec<[usize; 1]>>;
202202

203+
/// Computes the set of merge segments that should be run to compact the given compactables.
204+
///
205+
/// Returns both the mergeable segments and the actual number of merge segments that were created.
203206
pub fn get_merge_segments<T: Compactable>(
204207
compactables: &[T],
205208
config: &CompactConfig,
206-
) -> MergeSegments {
209+
) -> (MergeSegments, usize) {
207210
// Process all compactables in reverse order.
208211
// For each compactable, find the smallest set of compactables that overlaps with it and matches
209212
// the conditions.
@@ -259,7 +262,7 @@ pub fn get_merge_segments<T: Compactable>(
259262
continue 'outer;
260263
}
261264

262-
// If we are limited by size or count, we might also crate a merge segment if it's
265+
// If we are limited by size or count, we might also create a merge segment if it's
263266
// within the limits.
264267
let valid_merge_job = current_set.len() >= config.min_merge_count
265268
&& duplication_size >= config.min_merge_duplication_bytes;
@@ -361,7 +364,7 @@ pub fn get_merge_segments<T: Compactable>(
361364
true
362365
});
363366

364-
merge_segments
367+
(merge_segments, real_merge_segments)
365368
}
366369

367370
#[cfg(test)]
@@ -398,7 +401,7 @@ mod tests {
398401
.into_iter()
399402
.map(|range| TestCompactable { range, size: 100 })
400403
.collect::<Vec<_>>();
401-
let jobs = get_merge_segments(&compactables, config);
404+
let (jobs, _) = get_merge_segments(&compactables, config);
402405
jobs.into_iter()
403406
.map(|job| job.into_iter().collect())
404407
.collect()
@@ -613,7 +616,7 @@ mod tests {
613616
optimal_merge_duplication_bytes: 500,
614617
max_merge_segment_count: 4,
615618
};
616-
let jobs = get_merge_segments(&containers, &config);
619+
let (jobs, _) = get_merge_segments(&containers, &config);
617620
if !jobs.is_empty() {
618621
println!("{jobs:?}");
619622

turbopack/crates/turbo-persistence/src/db.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -794,8 +794,9 @@ impl TurboPersistence {
794794
if compact_config.max_merge_segment_count == 0 {
795795
return Vec::new();
796796
}
797-
let merge_jobs = get_merge_segments(ssts_with_ranges, &compact_config);
798-
compact_config.max_merge_segment_count -= merge_jobs.len();
797+
let (merge_jobs, real_merge_job_size) =
798+
get_merge_segments(ssts_with_ranges, &compact_config);
799+
compact_config.max_merge_segment_count -= real_merge_job_size;
799800
merge_jobs
800801
})
801802
.collect::<Vec<_>>();

0 commit comments

Comments
 (0)