Repository navigation
perf(sftp): size the tar decision from listings, not a stat per file (#494) - #564
Merged
Merged
Conversation
…494) When the exec size probe can't answer, has_large_remote walked the tree with its own traversal and stat'ed every entry: one round trip per file plus two per directory. SFTP listings already carry each entry's type and size. walk()'s traversal becomes visit(), with an early stop; walk() and the new large_by_walk() both use it. On SFTP the fallback now costs one round trip per directory and stops at the first large file. Listed gains `complete`: whether the listing vouches for the entry's type and size. Entries it doesn't vouch for (a bare SFTP listing, and every docker/FTP/WebDAV listing, where an unknown size reads as 0) are stat'ed exactly as before, so no endpoint routes differently. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #494.
When the exec size probe (
find … -size/Get-ChildItem) can't answer (no probe for the shell, a failure, or the 120 s timeout),has_large_remotewalked the tree itself to find a file ≥ 64 MiB. That walk duplicatedwalk()'s traversal and stat'ed every entry: one round trip per file plus two per directory. SFTP listings already carry each entry's type and size.The issue offered "go per-file directly". I didn't do that: it would send every small-file tree on a probe-less host (including Windows OpenSSH's default
cmd.exe) per file instead of tar, which is a slowdown. This keeps every routing decision as it was and makes the fallback cheaper.Change
resume/mod.rs:walk()'s traversal becomesvisit(), which takes a callback returningGo/Prune/Stop.walk()is rebuilt on it with the same output: same order, same symlink skipping, the same unsafe-name skipping, and the samereport_skippedfor unreadable metadata.resume/large.rs: the fallback is nowlarge_by_walk()onvisit(). On SFTP it costs one round trip per directory, none per file, and stops at the first large file. For a tree of 10,000 files in 200 folders, that's about 200 round trips instead of about 10,400.Listed.complete: whether the listing vouches for the entry's type and size.large_by_walkstats anything it doesn't vouch for, exactly as before:listed_completely).From<RemoteFile>): never complete, since e.g. docker's listing writesstat || echo 0and can't tell an unknown size from 0. Those endpoints keep the old per-entry lookups.stat: None): also stat'ed individually, as before.Tests
(1, 5)for a 4-folder tree); the walk stops at the first large file ((1, 1)); a listing that doesn't vouch is stat'ed entry by entry and still finds the large file;listed_completelyfor each attribute gap.TestFscounts stats and lists, and can serve bare listings.cargo test --libagainst this PR's base (69ddb1bd), in the same container: base 789 passed, this branch 794 passed (the 5 new tests). Both fail the same 3commands::pluginstests, because the JS plugin bundles weren't built in that container.cargo fmt --checkandcargo clippy --all-targets -D warningsare clean.Not measured
🤖 Generated with Claude Code