fix: replace recursive byte-counting with entry-based transfer progress (#395)

* fix: replace recursive byte-counting with entry-based transfer progress

Replace the expensive recursive `get_total_transfer_size` pre-calculation
with a lightweight entry-based counter (`TransferProgress`) for the
overall progress bar. This avoids deep `list_dir` traversals before
transfers begin, which could cause FTP idle-timeout disconnections on
large directory trees.

The per-file byte-level progress bar (`ProgressStates`) remains
unchanged. Bytes are still tracked via `TransferStates::add_bytes` for
notification threshold logic.

Closes #384
This commit is contained in:
Christian Visintin
2026-04-19 01:54:46 +05:30
parent 4d65e48b3c
commit d97535894c
3 changed files with 172 additions and 123 deletions
+90 -20
View File
@@ -9,14 +9,54 @@ use bytesize::ByteSize;
// -- States and progress // -- States and progress
/// TransferStates contains the states related to the transfer process /// Tracks overall transfer progress as an entry counter (e.g. "3/12").
pub struct TransferStates { ///
aborted: bool, // Describes whether the transfer process has been aborted /// Unlike the byte-based `ProgressStates`, this counts top-level entries
pub full: ProgressStates, // full transfer states /// rather than bytes, avoiding the expensive recursive size pre-calculation
pub partial: ProgressStates, // Partial transfer states /// that can cause FTP idle-timeout disconnections on large directory trees.
#[derive(Default)]
pub struct TransferProgress {
completed: usize,
total: usize,
} }
/// Progress states describes the states for the progress of a single transfer part impl fmt::Display for TransferProgress {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}/{}", self.completed, self.total)
}
}
impl TransferProgress {
/// Initialize progress with the total number of entries to transfer.
pub fn init(&mut self, total: usize) {
self.completed = 0;
self.total = total;
}
/// Mark one entry as completed.
pub fn increment(&mut self) {
self.completed += 1;
}
/// Calculate progress in a range between 0.0 and 1.0.
pub fn calc_progress(&self) -> f64 {
if self.total == 0 {
return 0.0;
}
let prog = self.completed as f64 / self.total as f64;
prog.min(1.0)
}
}
/// Contains the states related to the transfer process.
pub struct TransferStates {
aborted: bool,
pub full: TransferProgress,
pub partial: ProgressStates,
bytes_transferred: usize,
}
/// Describes the states for the progress of a single file transfer.
pub struct ProgressStates { pub struct ProgressStates {
started: Instant, started: Instant,
total: usize, total: usize,
@@ -30,33 +70,40 @@ impl Default for TransferStates {
} }
impl TransferStates { impl TransferStates {
/// Instantiates a new transfer states /// Instantiates a new transfer states.
pub fn new() -> TransferStates { pub fn new() -> TransferStates {
TransferStates { TransferStates {
aborted: false, aborted: false,
full: ProgressStates::default(), full: TransferProgress::default(),
partial: ProgressStates::default(), partial: ProgressStates::default(),
bytes_transferred: 0,
} }
} }
/// Re-intiialize transfer states /// Re-initialize transfer states.
pub fn reset(&mut self) { pub fn reset(&mut self) {
self.aborted = false; self.aborted = false;
self.bytes_transferred = 0;
} }
/// Set aborted to true /// Set aborted to true.
pub fn abort(&mut self) { pub fn abort(&mut self) {
self.aborted = true; self.aborted = true;
} }
/// Returns whether transfer has been aborted /// Returns whether transfer has been aborted.
pub fn aborted(&self) -> bool { pub fn aborted(&self) -> bool {
self.aborted self.aborted
} }
/// Returns the size of the entire transfer /// Returns total bytes transferred (used for notification threshold).
pub fn full_size(&self) -> usize { pub fn full_size(&self) -> usize {
self.full.total self.bytes_transferred
}
/// Accumulate transferred bytes for notification threshold tracking.
pub fn add_bytes(&mut self, delta: usize) {
self.bytes_transferred += delta;
} }
} }
@@ -226,23 +273,46 @@ mod test {
assert_eq!(states.calc_progress(), 0.0); assert_eq!(states.calc_progress(), 0.0);
} }
#[test]
fn test_ui_activities_filetransfer_lib_transfer_progress() {
let mut progress = TransferProgress::default();
assert_eq!(progress.calc_progress(), 0.0);
assert_eq!(progress.to_string(), "0/0");
// Init with 4 entries
progress.init(4);
assert_eq!(progress.calc_progress(), 0.0);
assert_eq!(progress.to_string(), "0/4");
// Increment
progress.increment();
assert_eq!(progress.calc_progress(), 0.25);
assert_eq!(progress.to_string(), "1/4");
// Complete all
progress.increment();
progress.increment();
progress.increment();
assert_eq!(progress.calc_progress(), 1.0);
assert_eq!(progress.to_string(), "4/4");
}
#[test] #[test]
fn test_ui_activities_filetransfer_lib_transfer_states() { fn test_ui_activities_filetransfer_lib_transfer_states() {
let mut states: TransferStates = TransferStates::default(); let mut states: TransferStates = TransferStates::default();
assert_eq!(states.aborted, false); assert!(!states.aborted());
assert_eq!(states.full.total, 0);
assert_eq!(states.full.written, 0);
assert!(states.full.started.elapsed().as_secs() < 5);
assert_eq!(states.partial.total, 0); assert_eq!(states.partial.total, 0);
assert_eq!(states.partial.written, 0); assert_eq!(states.partial.written, 0);
assert!(states.partial.started.elapsed().as_secs() < 5); assert!(states.partial.started.elapsed().as_secs() < 5);
// Aborted // Aborted
states.abort(); states.abort();
assert_eq!(states.aborted(), true); assert!(states.aborted());
states.reset(); states.reset();
assert_eq!(states.aborted(), false); assert!(!states.aborted());
states.full.total = 1024; // Bytes tracking
states.add_bytes(512);
states.add_bytes(512);
assert_eq!(states.full_size(), 1024); assert_eq!(states.full_size(), 1024);
// Reset clears bytes
states.reset();
assert_eq!(states.full_size(), 0);
} }
#[test] #[test]
@@ -211,43 +211,6 @@ impl FileTransferActivity {
} }
} }
// -- transfer sizes --
/// Get total size of transfer for the specified side.
pub(super) fn get_total_transfer_size(&mut self, entry: &File, local: bool) -> usize {
self.mount_blocking_wait("Calculating transfer size…");
let sz = if entry.is_dir() {
let list_result = if local {
self.browser.local_pane_mut().fs.list_dir(entry.path())
} else {
self.browser.remote_pane_mut().fs.list_dir(entry.path())
};
match list_result {
Ok(files) => files
.iter()
.map(|x| self.get_total_transfer_size(x, local))
.sum(),
Err(err) => {
self.log(
LogLevel::Error,
format!(
"Could not list directory {}: {}",
entry.path().display(),
err
),
);
0
}
}
} else {
entry.metadata.size as usize
};
self.umount_wait();
sz
}
// -- file changed -- // -- file changed --
/// Check whether a file has changed on the specified side, compared to the given metadata. /// Check whether a file has changed on the specified side, compared to the given metadata.
@@ -87,9 +87,8 @@ impl FileTransferActivity {
) -> Result<(), String> { ) -> Result<(), String> {
// Reset states // Reset states
self.transfer.reset(); self.transfer.reset();
// Calculate total size of transfer // Single file = 1 entry
let total_transfer_size: usize = file.metadata.size as usize; self.transfer.full.init(1);
self.transfer.full.init(total_transfer_size);
// Mount progress bar // Mount progress bar
self.mount_progress_bar(format!("Uploading {}…", file.path.display())); self.mount_progress_bar(format!("Uploading {}…", file.path.display()));
// Get remote path // Get remote path
@@ -102,54 +101,56 @@ impl FileTransferActivity {
remote_path.push(remote_file_name); remote_path.push(remote_file_name);
// Send // Send
let result = self.filetransfer_send_one(file, remote_path.as_path(), file_name); let result = self.filetransfer_send_one(file, remote_path.as_path(), file_name);
if result.is_ok() {
self.transfer.full.increment();
}
// Umount progress bar // Umount progress bar
self.umount_progress_bar(); self.umount_progress_bar();
// Return result // Return result
result.map_err(|x| x.to_string()) result.map_err(|x| x.to_string())
} }
/// Send a `TransferPayload` of type `Any` /// Send a `TransferPayload` of type `Any`.
fn filetransfer_send_any( fn filetransfer_send_any(
&mut self, &mut self,
entry: &File, entry: &File,
curr_remote_path: &Path, curr_remote_path: &Path,
dst_name: Option<String>, dst_name: Option<String>,
) -> Result<(), String> { ) -> Result<(), String> {
// Reset states
self.transfer.reset(); self.transfer.reset();
// Calculate total size of transfer if !entry.is_dir() {
let total_transfer_size: usize = self.get_total_transfer_size(entry, true); self.transfer.full.init(1);
self.transfer.full.init(total_transfer_size); }
// Mount progress bar
self.mount_progress_bar(format!("Uploading {}…", entry.path().display())); self.mount_progress_bar(format!("Uploading {}…", entry.path().display()));
// Send recurse let result = self.filetransfer_send_recurse(entry, curr_remote_path, dst_name, true);
let result = self.filetransfer_send_recurse(entry, curr_remote_path, dst_name);
// Umount progress bar
self.umount_progress_bar(); self.umount_progress_bar();
result result
} }
/// Send transfer queue entries to remote /// Send transfer queue entries to remote.
fn filetransfer_send_transfer_queue( fn filetransfer_send_transfer_queue(
&mut self, &mut self,
entries: &[(File, PathBuf)], entries: &[(File, PathBuf)],
) -> Result<(), String> { ) -> Result<(), String> {
// Reset states // Reset states
self.transfer.reset(); self.transfer.reset();
// Calculate total size of transfer // Total = number of queue entries
let total_transfer_size: usize = entries self.transfer.full.init(entries.len());
.iter()
.map(|(x, _)| self.get_total_transfer_size(x, true))
.sum();
self.transfer.full.init(total_transfer_size);
// Mount progress bar // Mount progress bar
self.mount_progress_bar(format!("Uploading {} entries…", entries.len())); self.mount_progress_bar(format!("Uploading {} entries…", entries.len()));
// Send recurse // Send each entry
let result = entries let mut result = Ok(());
.iter() for (entry, remote) in entries {
.map(|(x, remote)| self.filetransfer_send_recurse(x, remote, None)) if self.transfer.aborted() {
.find(|x| x.is_err()) break;
.unwrap_or(Ok(())); }
let r = self.filetransfer_send_recurse(entry, remote, None, false);
if r.is_err() {
result = r;
break;
}
self.transfer.full.increment();
}
// Umount progress bar // Umount progress bar
self.umount_progress_bar(); self.umount_progress_bar();
result result
@@ -160,6 +161,7 @@ impl FileTransferActivity {
entry: &File, entry: &File,
curr_remote_path: &Path, curr_remote_path: &Path,
dst_name: Option<String>, dst_name: Option<String>,
track_progress: bool,
) -> Result<(), String> { ) -> Result<(), String> {
// Write popup // Write popup
let file_name = entry.name(); let file_name = entry.name();
@@ -200,14 +202,17 @@ impl FileTransferActivity {
// Get files in dir // Get files in dir
match self.browser.local_pane_mut().fs.list_dir(entry.path()) { match self.browser.local_pane_mut().fs.list_dir(entry.path()) {
Ok(entries) => { Ok(entries) => {
// Iterate over files if track_progress {
self.transfer.full.init(entries.len());
}
for entry in entries.iter() { for entry in entries.iter() {
// If aborted; break
if self.transfer.aborted() { if self.transfer.aborted() {
break; break;
} }
// Send entry; name is always None after first call self.filetransfer_send_recurse(entry, remote_path.as_path(), None, false)?;
self.filetransfer_send_recurse(entry, remote_path.as_path(), None)? if track_progress {
self.transfer.full.increment();
}
} }
Ok(()) Ok(())
} }
@@ -262,7 +267,12 @@ impl FileTransferActivity {
} }
Err(err.to_string()) Err(err.to_string())
} }
Ok(_) => Ok(()), Ok(_) => {
if track_progress {
self.transfer.full.increment();
}
Ok(())
}
} }
}; };
// Scan dir on remote // Scan dir on remote
@@ -302,7 +312,7 @@ impl FileTransferActivity {
host_bridge.path().display() host_bridge.path().display()
), ),
); );
self.transfer.full.update_progress(metadata.size as usize); self.transfer.add_bytes(metadata.size as usize);
return Ok(()); return Ok(());
} }
// Upload file // Upload file
@@ -393,7 +403,7 @@ impl FileTransferActivity {
}; };
// Increase progress // Increase progress
self.transfer.partial.update_progress(delta); self.transfer.partial.update_progress(delta);
self.transfer.full.update_progress(delta); self.transfer.add_bytes(delta);
// Draw only if a significant progress has been made (performance improvement) // Draw only if a significant progress has been made (performance improvement)
if last_progress_val < self.transfer.partial.calc_progress() - 0.01 { if last_progress_val < self.transfer.partial.calc_progress() - 0.01 {
// Draw // Draw
@@ -467,23 +477,19 @@ impl FileTransferActivity {
/// Recv fs entry from remote. /// Recv fs entry from remote.
/// If dst_name is Some, entry will be saved with a different name. /// If dst_name is Some, entry will be saved with a different name.
/// If entry is a directory, this applies to directory only /// If entry is a directory, this applies to directory only.
fn filetransfer_recv_any( fn filetransfer_recv_any(
&mut self, &mut self,
entry: &File, entry: &File,
host_path: &Path, host_path: &Path,
dst_name: Option<String>, dst_name: Option<String>,
) -> Result<(), String> { ) -> Result<(), String> {
// Reset states
self.transfer.reset(); self.transfer.reset();
// Calculate total transfer size if !entry.is_dir() {
let total_transfer_size: usize = self.get_total_transfer_size(entry, false); self.transfer.full.init(1);
self.transfer.full.init(total_transfer_size); }
// Mount progress bar
self.mount_progress_bar(format!("Downloading {}…", entry.path().display())); self.mount_progress_bar(format!("Downloading {}…", entry.path().display()));
// Receive let result = self.filetransfer_recv_recurse(entry, host_path, dst_name, true);
let result = self.filetransfer_recv_recurse(entry, host_path, dst_name);
// Umount progress bar
self.umount_progress_bar(); self.umount_progress_bar();
result result
} }
@@ -496,40 +502,45 @@ impl FileTransferActivity {
) -> Result<(), String> { ) -> Result<(), String> {
// Reset states // Reset states
self.transfer.reset(); self.transfer.reset();
// Calculate total transfer size // Single file = 1 entry
let total_transfer_size: usize = entry.metadata.size as usize; self.transfer.full.init(1);
self.transfer.full.init(total_transfer_size);
// Mount progress bar // Mount progress bar
self.mount_progress_bar(format!("Downloading {}…", entry.path.display())); self.mount_progress_bar(format!("Downloading {}…", entry.path.display()));
// Receive // Receive
let result = self.filetransfer_recv_one(host_bridge_path, entry, entry.name()); let result = self.filetransfer_recv_one(host_bridge_path, entry, entry.name());
if result.is_ok() {
self.transfer.full.increment();
}
// Umount progress bar // Umount progress bar
self.umount_progress_bar(); self.umount_progress_bar();
// Return result // Return result
result.map_err(|x| x.to_string()) result.map_err(|x| x.to_string())
} }
/// Receive transfer queue from remote /// Receive transfer queue from remote.
fn filetransfer_recv_transfer_queue( fn filetransfer_recv_transfer_queue(
&mut self, &mut self,
entries: &[(File, PathBuf)], entries: &[(File, PathBuf)],
) -> Result<(), String> { ) -> Result<(), String> {
// Reset states // Reset states
self.transfer.reset(); self.transfer.reset();
// Calculate total size of transfer // Total = number of queue entries
let total_transfer_size: usize = entries self.transfer.full.init(entries.len());
.iter()
.map(|(x, _)| self.get_total_transfer_size(x, false))
.sum();
self.transfer.full.init(total_transfer_size);
// Mount progress bar // Mount progress bar
self.mount_progress_bar(format!("Downloading {} entries…", entries.len())); self.mount_progress_bar(format!("Downloading {} entries…", entries.len()));
// Send recurse // Receive each entry
let result = entries let mut result = Ok(());
.iter() for (entry, path) in entries {
.map(|(x, path)| self.filetransfer_recv_recurse(x, path, None)) if self.transfer.aborted() {
.find(|x| x.is_err()) break;
.unwrap_or(Ok(())); }
let r = self.filetransfer_recv_recurse(entry, path, None, false);
if r.is_err() {
result = r;
break;
}
self.transfer.full.increment();
}
// Umount progress bar // Umount progress bar
self.umount_progress_bar(); self.umount_progress_bar();
result result
@@ -540,6 +551,7 @@ impl FileTransferActivity {
entry: &File, entry: &File,
host_bridge_path: &Path, host_bridge_path: &Path,
dst_name: Option<String>, dst_name: Option<String>,
track_progress: bool,
) -> Result<(), String> { ) -> Result<(), String> {
// Write popup // Write popup
let file_name = entry.name(); let file_name = entry.name();
@@ -583,19 +595,22 @@ impl FileTransferActivity {
// Get files in dir from remote // Get files in dir from remote
match self.browser.remote_pane_mut().fs.list_dir(entry.path()) { match self.browser.remote_pane_mut().fs.list_dir(entry.path()) {
Ok(entries) => { Ok(entries) => {
// Iterate over files if track_progress {
self.transfer.full.init(entries.len());
}
for entry in entries.iter() { for entry in entries.iter() {
// If transfer has been aborted; break
if self.transfer.aborted() { if self.transfer.aborted() {
break; break;
} }
// Receive entry; name is always None after first call
// Local path becomes host_bridge_dir_path
self.filetransfer_recv_recurse( self.filetransfer_recv_recurse(
entry, entry,
host_bridge_dir_path.as_path(), host_bridge_dir_path.as_path(),
None, None,
)? false,
)?;
if track_progress {
self.transfer.full.increment();
}
} }
Ok(()) Ok(())
} }
@@ -672,6 +687,9 @@ impl FileTransferActivity {
} }
Err(err.to_string()) Err(err.to_string())
} else { } else {
if track_progress {
self.transfer.full.increment();
}
Ok(()) Ok(())
} }
}; };
@@ -704,9 +722,7 @@ impl FileTransferActivity {
remote.path().display() remote.path().display()
), ),
); );
self.transfer self.transfer.add_bytes(remote.metadata().size as usize);
.full
.update_progress(remote.metadata().size as usize);
return Ok(()); return Ok(());
} }
@@ -786,7 +802,7 @@ impl FileTransferActivity {
}; };
// Set progress // Set progress
self.transfer.partial.update_progress(delta); self.transfer.partial.update_progress(delta);
self.transfer.full.update_progress(delta); self.transfer.add_bytes(delta);
// Draw only if a significant progress has been made (performance improvement) // Draw only if a significant progress has been made (performance improvement)
if last_progress_val < self.transfer.partial.calc_progress() - 0.01 { if last_progress_val < self.transfer.partial.calc_progress() - 0.01 {
// Draw // Draw