Skip to content

Commit 55d3ca4

Browse files
committed
Bugfix: Bulk-skip credit no longer pollutes throughput estimator
After e7f657d made bulk-skip correctly credit `bytes_done`, the ETA estimator started treating the instant counter jump as throughput: the bulk-skip delta over ε time pinned `bytes_per_second` at GB/s level on the first sample, and EWMA took 10+ samples (many seconds) to decay it. User-visible symptom: speeds opened with bogus values (1.91 GB/s instead of ~15 MB/s, 63 files/s instead of ~1 file/s, ~20 s ETA instead of ~20 min) before settling. - `EtaEstimator::reseed_baseline(now, bytes, files)` re-anchors `last_t/last_bytes/last_files` and resets `samples` to 0 without changing the phase. The bulk-skip jump becomes the new starting point, not a delta. - Both driver preludes (`drive_transfer_serial_sync`, `drive_transfer_serial_async`) and the inline concurrent path in `copy_volumes_with_progress` call `reseed_baseline` right before emitting the bulk-skip event. - New unit test `bulk_skip_baseline_jump_does_not_pollute_rate` pins the property: after a `(0, 0)` → `(22 GB, 250 files)` jump over 1 ms followed by a real `15 MB / 1 file` in 1 s emit, `bytes_per_second < 50 MB/s` and `files_per_second < 5`. Pre-fix this test fails with GB/s-level rates.
1 parent b1f707b commit 55d3ca4

3 files changed

Lines changed: 98 additions & 0 deletions

File tree

apps/desktop/src-tauri/src/file_system/write_operations/eta.rs

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,33 @@ impl EtaEstimator {
9393
Self::default()
9494
}
9595

96+
/// Re-anchor the estimator's baseline to the given counters without
97+
/// computing a rate from the jump. Use this when the caller wants to
98+
/// advance the absolute counters by a chunk that is NOT throughput
99+
/// (e.g. the bulk-skip prelude credits N files / B bytes instantly to
100+
/// reflect "Skip-All for pre-known conflicts" — those files were never
101+
/// actually copied, so feeding the delta into the EWMA pins the first
102+
/// sample at GB/s and pollutes the rate display for many seconds).
103+
///
104+
/// No-op if no phase is active (the next `update` will seed normally).
105+
/// Does NOT change the phase; the next `update` keeps the current phase
106+
/// unless it actually transitions.
107+
pub fn reseed_baseline(&mut self, now: Instant, bytes_done: u64, files_done: usize) {
108+
if let Some(state) = self.state.as_mut() {
109+
state.last_t = now;
110+
state.last_bytes = bytes_done;
111+
state.last_files = files_done;
112+
// Reset samples to 0 so the next `update` takes the fast-path
113+
// first-sample seed (initialize EWMA from the instantaneous rate
114+
// rather than smoothing from zero, per the existing first-sample
115+
// rationale). The samples gate also keeps `eta_seconds = None`
116+
// until two real samples land.
117+
state.samples = 0;
118+
state.bytes_rate = 0.0;
119+
state.files_rate = 0.0;
120+
}
121+
}
122+
96123
/// Update the estimator with the latest counters and return the current stats.
97124
///
98125
/// `now` is injected (not read from `Instant::now()` internally) so tests can
@@ -291,6 +318,57 @@ mod tests {
291318
assert_eq!(stats, EtaStats::ZERO);
292319
}
293320

321+
#[test]
322+
fn bulk_skip_baseline_jump_does_not_pollute_rate() {
323+
// Models the volume-copy Skip-All path. Caller emits `(0, 0)` at the
324+
// Copying-phase boundary (the estimator reseeds and returns ZERO);
325+
// the driver's bulk-skip prelude then jumps the counters to
326+
// `(bulk_skip_files, bulk_skip_bytes)` instantly. Without an explicit
327+
// baseline reseed, the bulk-skip delta over ε time becomes the
328+
// first-sample rate (~22 GB/s, ~250 files/s in this fixture), and
329+
// EWMA takes many seconds to decay it. The fix: call
330+
// `reseed_baseline` before the bulk-skip emit so the jump becomes
331+
// the new starting point, not throughput. The next real per-file
332+
// emit's delta is then just the actually-copied portion.
333+
let start = Instant::now();
334+
let mut est = EtaEstimator::new();
335+
336+
// t=0: initial Copying emit (phase transition Scanning -> Copying).
337+
let initial = est.update(at(start, 0), WriteOperationPhase::Copying, 0, 35_000_000_000, 0, 1051);
338+
assert_eq!(initial, EtaStats::ZERO);
339+
340+
// t=1 ms: driver bulk-skip prelude credits 22 GB / 250 files
341+
// instantly. Caller calls `reseed_baseline` immediately before the
342+
// emit so the estimator absorbs the jump as its new starting point.
343+
est.reseed_baseline(at(start, 1), 22_000_000_000, 250);
344+
345+
// t=1001 ms: first real per-file emit. Actually-copied delta vs.
346+
// the new baseline = 15 MB / 1 file over 1 s.
347+
let stats = est.update(
348+
at(start, 1001),
349+
WriteOperationPhase::Copying,
350+
22_015_000_000,
351+
35_000_000_000,
352+
251,
353+
1051,
354+
);
355+
356+
// Pre-fix: bytes_per_second is in the GB/s range and files_per_second
357+
// is in the hundreds (a 250-file / 22-GB jump over ε time pinned the
358+
// EWMA's first sample, then partially decayed). Assert single-digit
359+
// multiples of the true rate, not orders of magnitude off.
360+
assert!(
361+
stats.bytes_per_second < 50_000_000,
362+
"bytes_per_second = {} (expected ~15 MB/s, bulk-skip should not feed the rate)",
363+
stats.bytes_per_second,
364+
);
365+
assert!(
366+
stats.files_per_second < 5.0,
367+
"files_per_second = {} (expected ~1 file/s, bulk-skip should not feed the rate)",
368+
stats.files_per_second,
369+
);
370+
}
371+
294372
#[test]
295373
fn warmup_suppresses_eta_until_min_elapsed() {
296374
// Two samples 200 ms apart, below MIN_ELAPSED_FOR_ETA.

apps/desktop/src-tauri/src/file_system/write_operations/transfer_driver.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,14 @@ where
385385
bulk_skip_files,
386386
bulk_skip_bytes
387387
);
388+
// Re-anchor the rate estimator BEFORE emitting. The bulk-skip jump
389+
// is past work credited instantly, not throughput; without this
390+
// reseed the first real per-file emit's delta is computed against
391+
// `(0, 0)` and pins `bytes_per_second` at GB/s level. See
392+
// `eta::EtaEstimator::reseed_baseline` for the full rationale.
393+
if let Ok(mut est) = state.estimator.lock() {
394+
est.reseed_baseline(Instant::now(), bytes_done, files_done);
395+
}
388396
emit_progress_and_status(
389397
events,
390398
state,
@@ -610,6 +618,11 @@ where
610618
bulk_skip_files,
611619
bulk_skip_bytes
612620
);
621+
// Re-anchor the rate estimator: bulk-skip credit is past work, not
622+
// throughput. See `drive_transfer_serial_sync` for the rationale.
623+
if let Ok(mut est) = state.estimator.lock() {
624+
est.reseed_baseline(Instant::now(), bytes_done, files_done);
625+
}
613626
emit_progress_and_status(
614627
events,
615628
state,

apps/desktop/src-tauri/src/file_system/write_operations/volume_copy.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,13 @@ pub(crate) async fn copy_volumes_with_progress(
612612
bulk_skip_files,
613613
bulk_skip_bytes
614614
);
615+
// Re-anchor the rate estimator: bulk-skip credit is past work, not
616+
// throughput. Without this the first per-task progress callback's
617+
// delta against `(0, 0)` pins `bytes_per_second` at GB/s level.
618+
// Same pattern as the driver's serial preludes.
619+
if let Ok(mut est) = state.estimator.lock() {
620+
est.reseed_baseline(Instant::now(), new_bytes, new_files);
621+
}
615622
state.emit_progress_via_sink(
616623
&*events,
617624
WriteProgressEvent::new(

0 commit comments

Comments
 (0)