fix: stop counting by default; the pass costs more than the transfer
Build and publish container / build (pull_request) Successful in 3m32s
Build and publish container / build (pull_request) Successful in 3m32s
The counting pass was added so the progress line could show a percentage and an estimate, and on a real card it turned out to dominate the run. Measured against the device: reading a track from the SMB mirror ran at 35 MB/s and writing to the card at 21 MB/s, while the sync itself managed tens of kilobytes per second. Neither end was slow. The cost was traversing fifty thousand files across six thousand directories on FAT, and the counting pass does that a second time, comparing both trees in full exactly as the transfer does. Counting is now opt-in behind -P. Without it the progress line still shows the running count, the transfer rate and the album in flight; the percentage and the estimate are what needed the extra walk, and they were the least useful part of the display. That the fix for "it looks hung" was itself making it slow is the sort of thing only measuring catches. The line still answers the question it was added for -- whether anything is happening -- without paying for the part that merely made it prettier.
This commit is contained in:
@@ -178,12 +178,20 @@ entirely for the first two seconds, where the window is microseconds wide and
|
|||||||
would report gigabytes per second.
|
would report gigabytes per second.
|
||||||
|
|
||||||
rsync says nothing at all while it builds its file list, which on fifty
|
rsync says nothing at all while it builds its file list, which on fifty
|
||||||
thousand files over USB is minutes of apparent hang, and its own `progress2`
|
thousand files is minutes of apparent hang, and its own `progress2` percentage
|
||||||
percentage is computed against a list it has not finished discovering. So the
|
is computed against a list it has not finished discovering. So the script
|
||||||
script counts first — files and bytes both, a second pass over the tree, which
|
renders its own.
|
||||||
is what a percentage and an estimate that mean something cost — and renders the
|
|
||||||
rest itself. Piped to a log it prints a plain line every thirty seconds
|
**The percentage and the estimate are opt-in, via `-P`.** They need a total,
|
||||||
instead, with no carriage returns, and a summary at the end either way.
|
the total needs a counting pass, and that pass walks and compares both trees in
|
||||||
|
full exactly as the transfer does. Measured on a real card: read from the
|
||||||
|
source at 35 MB/s and write to the card at 21 MB/s, yet the sync crawled —
|
||||||
|
because the traversal, not the data, was the cost, and it was being paid twice.
|
||||||
|
Without `-P` the line still shows the running count, the rate and the album in
|
||||||
|
flight; only the two figures that needed the second walk are missing.
|
||||||
|
|
||||||
|
Piped to a log it prints a plain line every thirty seconds instead, with no
|
||||||
|
carriage returns, and a summary at the end either way.
|
||||||
|
|
||||||
### The Rockbox database
|
### The Rockbox database
|
||||||
|
|
||||||
@@ -260,7 +268,7 @@ worth doing:
|
|||||||
| ----- | --- |
|
| ----- | --- |
|
||||||
| Mount the source with `actimeo=60,cache=loose` | SMB defaults to a **one second** attribute cache, so nearly every `stat` goes to the wire — twice, once per pass. This is the single biggest change and it is a mount option, not an rsync flag. |
|
| Mount the source with `actimeo=60,cache=loose` | SMB defaults to a **one second** attribute cache, so nearly every `stat` goes to the wire — twice, once per pass. This is the single biggest change and it is a mount option, not an rsync flag. |
|
||||||
| Put the card in a reader for the first load | USB 2.0 through an iPod in disk mode is the floor for the destination. No amount of source tuning gets past it. |
|
| Put the card in a reader for the first load | USB 2.0 through an iPod in disk mode is the floor for the destination. No amount of source tuning gets past it. |
|
||||||
| `-Q` | Skips the counting pass entirely. Costs the percentage and the estimate, saves a whole walk of the tree. |
|
| Counting is off by default | The percentage costs a second full traversal of both trees. On a FAT card of fifty thousand files that is slower than the transfer. `-P` asks for it. |
|
||||||
| `--whole-file`, `--omit-dir-times` | Already set. The first stops rsync checksumming destination files it is about to overwrite whole; the second drops a setattr per directory, 6,150 of them. |
|
| `--whole-file`, `--omit-dir-times` | Already set. The first stops rsync checksumming destination files it is about to overwrite whole; the second drops a setattr per directory, 6,150 of them. |
|
||||||
|
|
||||||
**NFS instead of SMB** is worth trying but is not the big win it looks like.
|
**NFS instead of SMB** is worth trying but is not the big win it looks like.
|
||||||
|
|||||||
@@ -176,16 +176,16 @@ def test_the_help_says_how_to_reach_and_leave_disk_mode():
|
|||||||
assert "holding Play" in help_text
|
assert "holding Play" in help_text
|
||||||
|
|
||||||
|
|
||||||
def test_quick_mode_skips_the_counting_pass(mirror, tmp_path):
|
def test_counting_is_off_by_default(mirror, tmp_path):
|
||||||
"""Over SMB the walk is the expensive part, and doing it twice for a
|
"""The counting pass walks and compares both trees exactly as the transfer
|
||||||
percentage is not always the trade you want."""
|
does. On a FAT card of fifty thousand files that costs more than moving the
|
||||||
|
data, so the percentage has to be asked for."""
|
||||||
destination = tmp_path / "dest"
|
destination = tmp_path / "dest"
|
||||||
destination.mkdir()
|
destination.mkdir()
|
||||||
|
|
||||||
result = run("-f", "-S", "-U", "-Q", str(mirror), str(destination))
|
result = run("-f", "-S", "-U", str(mirror), str(destination))
|
||||||
|
|
||||||
assert result.returncode == 0, result.stderr
|
assert result.returncode == 0, result.stderr
|
||||||
assert "skipping the count" in result.stderr
|
|
||||||
assert "files to copy" not in result.stderr
|
assert "files to copy" not in result.stderr
|
||||||
assert (destination / "Album" / "track.mp3").is_file()
|
assert (destination / "Album" / "track.mp3").is_file()
|
||||||
|
|
||||||
@@ -314,7 +314,7 @@ def test_the_database_lands_at_the_device_root_not_the_music_folder(tmp_path):
|
|||||||
script = (
|
script = (
|
||||||
f"mount --bind {card} {device} && "
|
f"mount --bind {card} {device} && "
|
||||||
f"XDG_CACHE_HOME={tmp_path / 'cache'} MUSIC_MIRROR_DATABASE_TOOL={tool} "
|
f"XDG_CACHE_HOME={tmp_path / 'cache'} MUSIC_MIRROR_DATABASE_TOOL={tool} "
|
||||||
f"bash {SCRIPT} -f -S -U -Q {tmp_path / 'mirror'} {device / 'Music'}"
|
f"bash {SCRIPT} -f -S -U {tmp_path / 'mirror'} {device / 'Music'}"
|
||||||
)
|
)
|
||||||
result = subprocess.run(
|
result = subprocess.run(
|
||||||
["unshare", "-Umr", "sh", "-c", script], capture_output=True, text=True
|
["unshare", "-Umr", "sh", "-c", script], capture_output=True, text=True
|
||||||
@@ -335,3 +335,14 @@ def test_the_database_lands_at_the_device_root_not_the_music_folder(tmp_path):
|
|||||||
# prefix, which is how the paths come out as /Music/... while the bytes are
|
# prefix, which is how the paths come out as /Music/... while the bytes are
|
||||||
# read from somewhere else entirely.
|
# read from somewhere else entirely.
|
||||||
assert "Pendulum" in (scratch / "saw.txt").read_text()
|
assert "Pendulum" in (scratch / "saw.txt").read_text()
|
||||||
|
|
||||||
|
|
||||||
|
def test_counting_can_be_asked_for(mirror, tmp_path):
|
||||||
|
"""When the destination is cheap to traverse, the percentage is worth it."""
|
||||||
|
destination = tmp_path / "dest"
|
||||||
|
destination.mkdir()
|
||||||
|
|
||||||
|
result = run("-f", "-S", "-U", "-P", str(mirror), str(destination))
|
||||||
|
|
||||||
|
assert result.returncode == 0, result.stderr
|
||||||
|
assert "files to copy" in result.stderr
|
||||||
|
|||||||
+13
-8
@@ -22,8 +22,9 @@ usage() {
|
|||||||
usage: sync-to-ipod.sh [options] <mirror> <destination>
|
usage: sync-to-ipod.sh [options] <mirror> <destination>
|
||||||
|
|
||||||
-n dry run; show what would change and touch nothing
|
-n dry run; show what would change and touch nothing
|
||||||
-Q skip the counting pass; no percentage or estimate, but one less walk
|
-P count what needs copying first, so progress can show a percentage and
|
||||||
of the source tree, which over SMB is the expensive part
|
an estimate. Costs a second full traversal of both trees, which on a
|
||||||
|
FAT card of fifty thousand files is slower than the transfer itself
|
||||||
-f copy even if the FAT32 check finds unacceptable paths
|
-f copy even if the FAT32 check finds unacceptable paths
|
||||||
-S skip submitting the Rockbox scrobbler log to Last.fm
|
-S skip submitting the Rockbox scrobbler log to Last.fm
|
||||||
-B skip rebuilding the Rockbox database
|
-B skip rebuilding the Rockbox database
|
||||||
@@ -68,7 +69,7 @@ USAGE
|
|||||||
}
|
}
|
||||||
|
|
||||||
dry_run=false
|
dry_run=false
|
||||||
quick=false
|
counting=false
|
||||||
force=false
|
force=false
|
||||||
unmount=true
|
unmount=true
|
||||||
scrobble=true
|
scrobble=true
|
||||||
@@ -76,10 +77,10 @@ database=true
|
|||||||
for argument in "$@"; do
|
for argument in "$@"; do
|
||||||
[ "$argument" = "--help" ] && usage help
|
[ "$argument" = "--help" ] && usage help
|
||||||
done
|
done
|
||||||
while getopts ":nQfSBUh" option; do
|
while getopts ":nPfSBUh" option; do
|
||||||
case "$option" in
|
case "$option" in
|
||||||
n) dry_run=true ;;
|
n) dry_run=true ;;
|
||||||
Q) quick=true ;;
|
P) counting=true ;;
|
||||||
f) force=true ;;
|
f) force=true ;;
|
||||||
S) scrobble=false ;;
|
S) scrobble=false ;;
|
||||||
B) database=false ;;
|
B) database=false ;;
|
||||||
@@ -198,11 +199,15 @@ fi
|
|||||||
# thousand files over USB is minutes of apparent hang. Counting first costs a
|
# thousand files over USB is minutes of apparent hang. Counting first costs a
|
||||||
# second pass over the tree but means the transfer can show a real percentage
|
# second pass over the tree but means the transfer can show a real percentage
|
||||||
# rather than a number that grows as rsync discovers more work.
|
# rather than a number that grows as rsync discovers more work.
|
||||||
|
# Counting is opt-in because it is not cheap. It walks and compares both trees
|
||||||
|
# in full, exactly as the transfer does, and on a FAT card holding fifty
|
||||||
|
# thousand files that traversal costs more than moving the data. Without it the
|
||||||
|
# progress line still shows the running count, the rate and the album in
|
||||||
|
# flight; only the percentage and the estimate are lost, and those were the
|
||||||
|
# least useful part of it.
|
||||||
total=0
|
total=0
|
||||||
total_bytes=0
|
total_bytes=0
|
||||||
if $quick; then
|
if $counting; then
|
||||||
printf 'sync-to-ipod: skipping the count; no percentage or estimate\n' >&2
|
|
||||||
else
|
|
||||||
printf 'sync-to-ipod: working out what needs copying...\n' >&2
|
printf 'sync-to-ipod: working out what needs copying...\n' >&2
|
||||||
# %l is the file's size, which is what makes an estimate possible.
|
# %l is the file's size, which is what makes an estimate possible.
|
||||||
# Directories are dropped: rsync reports those too, with an inode size that
|
# Directories are dropped: rsync reports those too, with an inode size that
|
||||||
|
|||||||
Reference in New Issue
Block a user