Skip to content

Commit 70978c8

Browse files
vdavidvdavid
committed
MTP: Batch scan for copy to avoid per-file USB round-trips
When copying many files from MTP (for example, 14,825 photos from DCIM/Camera), the scan phase previously called `scan_for_copy` per file, each triggering a USB `GetObjectHandles` that failed (files aren't directories), then falling back to re-listing the parent. This meant ~15k unnecessary USB round-trips. - Add `Volume::scan_for_copy_batch` trait method with a default that iterates (fine for local FS) - `MtpVolume` overrides: groups paths by parent dir, lists each parent once, resolves files from the listing - `run_volume_scan_preview` calls the batch method instead of looping - 5 new tests for the default batch implementation
1 parent 5518bcb commit 70978c8

4 files changed

Lines changed: 174 additions & 38 deletions

File tree

‎apps/desktop/src-tauri/src/file_system/volume/in_memory_test.rs‎

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Tests for InMemoryVolume.
22
33
use super::*;
4-
use std::path::Path;
4+
use std::path::{Path, PathBuf};
55

66
#[test]
77
fn test_new_creates_empty_volume() {
@@ -478,6 +478,82 @@ fn test_scan_for_copy_nested_directory_tree() {
478478
assert_eq!(result.total_bytes, 9); // 3 + 5 + 1
479479
}
480480

481+
// ============================================================================
482+
// scan_for_copy_batch tests (default implementation via Volume trait)
483+
// ============================================================================
484+
485+
#[test]
486+
fn test_scan_for_copy_batch_multiple_files_same_dir() {
487+
let volume = InMemoryVolume::new("Test");
488+
volume.create_directory(Path::new("/photos")).unwrap();
489+
volume.create_file(Path::new("/photos/a.jpg"), &[0; 100]).unwrap();
490+
volume.create_file(Path::new("/photos/b.jpg"), &[0; 200]).unwrap();
491+
volume.create_file(Path::new("/photos/c.jpg"), &[0; 300]).unwrap();
492+
493+
let paths = vec![
494+
PathBuf::from("/photos/a.jpg"),
495+
PathBuf::from("/photos/b.jpg"),
496+
PathBuf::from("/photos/c.jpg"),
497+
];
498+
let result = volume.scan_for_copy_batch(&paths).unwrap();
499+
assert_eq!(result.file_count, 3);
500+
assert_eq!(result.dir_count, 0);
501+
assert_eq!(result.total_bytes, 600);
502+
}
503+
504+
#[test]
505+
fn test_scan_for_copy_batch_mixed_files_and_dirs() {
506+
let volume = InMemoryVolume::new("Test");
507+
volume.create_directory(Path::new("/stuff")).unwrap();
508+
volume.create_file(Path::new("/stuff/readme.txt"), b"hello").unwrap();
509+
volume.create_directory(Path::new("/stuff/subdir")).unwrap();
510+
volume
511+
.create_file(Path::new("/stuff/subdir/deep.txt"), &[0; 50])
512+
.unwrap();
513+
514+
let paths = vec![PathBuf::from("/stuff/readme.txt"), PathBuf::from("/stuff/subdir")];
515+
let result = volume.scan_for_copy_batch(&paths).unwrap();
516+
assert_eq!(result.file_count, 2); // readme.txt + deep.txt
517+
assert_eq!(result.dir_count, 0); // subdir's children don't include extra dirs
518+
assert_eq!(result.total_bytes, 55); // 5 + 50
519+
}
520+
521+
#[test]
522+
fn test_scan_for_copy_batch_empty_input() {
523+
let volume = InMemoryVolume::new("Test");
524+
let result = volume.scan_for_copy_batch(&[]).unwrap();
525+
assert_eq!(result.file_count, 0);
526+
assert_eq!(result.dir_count, 0);
527+
assert_eq!(result.total_bytes, 0);
528+
}
529+
530+
#[test]
531+
fn test_scan_for_copy_batch_single_item_matches_single_scan() {
532+
let volume = InMemoryVolume::new("Test");
533+
volume.create_directory(Path::new("/docs")).unwrap();
534+
volume.create_file(Path::new("/docs/a.txt"), b"data").unwrap();
535+
536+
let single = volume.scan_for_copy(Path::new("/docs/a.txt")).unwrap();
537+
let batch = volume.scan_for_copy_batch(&[PathBuf::from("/docs/a.txt")]).unwrap();
538+
assert_eq!(single.file_count, batch.file_count);
539+
assert_eq!(single.dir_count, batch.dir_count);
540+
assert_eq!(single.total_bytes, batch.total_bytes);
541+
}
542+
543+
#[test]
544+
fn test_scan_for_copy_batch_files_from_different_dirs() {
545+
let volume = InMemoryVolume::new("Test");
546+
volume.create_directory(Path::new("/a")).unwrap();
547+
volume.create_directory(Path::new("/b")).unwrap();
548+
volume.create_file(Path::new("/a/file1.txt"), &[0; 10]).unwrap();
549+
volume.create_file(Path::new("/b/file2.txt"), &[0; 20]).unwrap();
550+
551+
let paths = vec![PathBuf::from("/a/file1.txt"), PathBuf::from("/b/file2.txt")];
552+
let result = volume.scan_for_copy_batch(&paths).unwrap();
553+
assert_eq!(result.file_count, 2);
554+
assert_eq!(result.total_bytes, 30);
555+
}
556+
481557
// ============================================================================
482558
// get_space_info tests
483559
// ============================================================================

‎apps/desktop/src-tauri/src/file_system/volume/mod.rs‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use crate::indexing::scanner::{ScanConfig, ScanError, ScanHandle, ScanSummary};
1111
use crate::indexing::watcher::{DriveWatcher, FsChangeEvent, WatcherError};
1212
use crate::indexing::writer::IndexWriter;
1313
use serde::{Deserialize, Serialize};
14-
use std::path::Path;
14+
use std::path::{Path, PathBuf};
1515
use std::sync::atomic::AtomicBool;
1616
use tokio::sync::mpsc;
1717

@@ -437,6 +437,28 @@ pub trait Volume: Send + Sync {
437437
Err(VolumeError::NotSupported)
438438
}
439439

440+
/// Scans multiple paths to get aggregate copy statistics.
441+
///
442+
/// The default iterates over `scan_for_copy` per path, which is correct for
443+
/// volumes where per-path I/O is cheap (local FS, in-memory). Volume types
444+
/// with expensive per-path I/O (MTP, SMB, FTP, S3) should override this to
445+
/// batch — typically by grouping paths by parent directory, listing each
446+
/// parent once, and resolving files from the listing.
447+
fn scan_for_copy_batch(&self, paths: &[PathBuf]) -> Result<CopyScanResult, VolumeError> {
448+
let mut result = CopyScanResult {
449+
file_count: 0,
450+
dir_count: 0,
451+
total_bytes: 0,
452+
};
453+
for path in paths {
454+
let scan = self.scan_for_copy(path)?;
455+
result.file_count += scan.file_count;
456+
result.dir_count += scan.dir_count;
457+
result.total_bytes += scan.total_bytes;
458+
}
459+
Ok(result)
460+
}
461+
440462
/// Downloads/exports a file or directory from this volume to a local path.
441463
/// For local volumes, this is a file copy. For MTP, this downloads.
442464
/// Returns bytes transferred.
@@ -516,7 +538,7 @@ pub trait Volume: Send + Sync {
516538
/// Returns the local filesystem path if this volume is backed by one.
517539
/// Used to optimize local-to-local copies using native OS APIs (such as copyfile on macOS).
518540
/// Returns None for non-local volumes (MTP, S3, FTP, etc.).
519-
fn local_path(&self) -> Option<std::path::PathBuf> {
541+
fn local_path(&self) -> Option<PathBuf> {
520542
None
521543
}
522544

‎apps/desktop/src-tauri/src/file_system/volume/mtp.rs‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,62 @@ impl Volume for MtpVolume {
509509
.map_err(map_mtp_error)
510510
}
511511

512+
fn scan_for_copy_batch(&self, paths: &[PathBuf]) -> Result<CopyScanResult, VolumeError> {
513+
if paths.is_empty() {
514+
return Ok(CopyScanResult {
515+
file_count: 0,
516+
dir_count: 0,
517+
total_bytes: 0,
518+
});
519+
}
520+
521+
// Group paths by parent directory so we list each parent at most once
522+
let mut by_parent: std::collections::HashMap<PathBuf, Vec<&PathBuf>> = std::collections::HashMap::new();
523+
for path in paths {
524+
let mtp_path = self.to_mtp_path(path);
525+
let mtp_path_buf = PathBuf::from(&mtp_path);
526+
let parent = mtp_path_buf.parent().unwrap_or(Path::new("")).to_path_buf();
527+
by_parent.entry(parent).or_default().push(path);
528+
}
529+
530+
debug!(
531+
"MtpVolume::scan_for_copy_batch: {} paths across {} unique parent dirs",
532+
paths.len(),
533+
by_parent.len()
534+
);
535+
536+
let mut result = CopyScanResult {
537+
file_count: 0,
538+
dir_count: 0,
539+
total_bytes: 0,
540+
};
541+
542+
for (parent, children) in &by_parent {
543+
// List the parent directory once (goes through the listing cache)
544+
let parent_str = parent.to_string_lossy();
545+
let entries = self.list_directory(Path::new(parent_str.as_ref()))?;
546+
547+
for child_path in children {
548+
let mtp_path = self.to_mtp_path(child_path);
549+
let name = Path::new(&mtp_path).file_name().and_then(|n| n.to_str()).unwrap_or("");
550+
551+
if let Some(entry) = entries.iter().find(|e| e.name == name) {
552+
if entry.is_directory {
553+
let scan = self.scan_for_copy(child_path)?;
554+
result.file_count += scan.file_count;
555+
result.dir_count += scan.dir_count;
556+
result.total_bytes += scan.total_bytes;
557+
} else {
558+
result.file_count += 1;
559+
result.total_bytes += entry.size.unwrap_or(0);
560+
}
561+
}
562+
}
563+
}
564+
565+
Ok(result)
566+
}
567+
512568
fn export_to_local_with_progress(
513569
&self,
514570
source: &Path,

‎apps/desktop/src-tauri/src/file_system/write_operations/scan_preview.rs‎

Lines changed: 17 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use super::types::{
1818
ScanPreviewStartResult,
1919
};
2020
use crate::file_system::listing::{SortColumn, SortOrder};
21+
use crate::file_system::volume::CopyScanResult;
2122
use crate::file_system::volume::Volume;
2223

2324
/// Starts a scan preview for the Copy dialog.
@@ -189,7 +190,8 @@ fn run_scan_preview(
189190

190191
/// Runs a volume-based scan preview (for MTP and other non-local volumes).
191192
///
192-
/// Uses `Volume::scan_for_copy()` per source path instead of `walk_dir_recursive`.
193+
/// Uses `Volume::scan_for_copy_batch()` to scan all sources in one call, allowing
194+
/// volume implementations to batch I/O (for example, MTP groups by parent directory).
193195
/// Emits the same events as `run_scan_preview` so the frontend can't tell the difference.
194196
fn run_volume_scan_preview(
195197
app: tauri::AppHandle,
@@ -200,43 +202,23 @@ fn run_volume_scan_preview(
200202
) {
201203
use tauri::Emitter;
202204

203-
let mut total_files = 0usize;
204-
let mut total_dirs = 0usize;
205-
let mut total_bytes = 0u64;
206-
let mut last_progress_time = Instant::now();
207-
208-
let result: Result<(), String> = (|| {
209-
for source in &sources {
210-
if state.cancelled.load(Ordering::Relaxed) {
211-
return Err("Cancelled".to_string());
212-
}
213-
214-
let scan = volume
215-
.scan_for_copy(source)
216-
.map_err(|e| format!("Scan failed for {}: {}", source.display(), e))?;
217-
218-
total_files += scan.file_count;
219-
total_dirs += scan.dir_count;
220-
total_bytes += scan.total_bytes;
221-
222-
// Emit progress between source items
223-
if last_progress_time.elapsed() >= state.progress_interval {
224-
let _ = app.emit(
225-
"scan-preview-progress",
226-
ScanPreviewProgressEvent {
227-
preview_id: preview_id.clone(),
228-
files_found: total_files,
229-
dirs_found: total_dirs,
230-
bytes_found: total_bytes,
231-
current_path: source.file_name().map(|n| n.to_string_lossy().to_string()),
232-
},
233-
);
234-
last_progress_time = Instant::now();
235-
}
205+
let result: Result<CopyScanResult, String> = (|| {
206+
if state.cancelled.load(Ordering::Relaxed) {
207+
return Err("Cancelled".to_string());
236208
}
237-
Ok(())
209+
210+
volume
211+
.scan_for_copy_batch(&sources)
212+
.map_err(|e| format!("Scan failed: {}", e))
238213
})();
239214

215+
// Extract stats from the result for the completion event
216+
let (total_files, total_dirs, total_bytes) = match &result {
217+
Ok(scan) => (scan.file_count, scan.dir_count, scan.total_bytes),
218+
Err(_) => (0, 0, 0),
219+
};
220+
let result = result.map(|_| ());
221+
240222
// Clean up state
241223
if let Ok(mut cache) = SCAN_PREVIEW_STATE.write() {
242224
cache.remove(&preview_id);

0 commit comments

Comments
 (0)