Remove --autotune - #153
Merged
Merged
Conversation
It is the one feature that can quietly make oans slower, and it does so
persistently.
Measured on a 32-core NVMe box. The storage heuristic picks 8, matching the
plateau documented in CLAUDE.md ("8 was the knee; 16/32 gave no wall gain
while sys exploded"). Autotune, run as a normal user, recommends 16 -- and
stores it:
threads time throughput
8 1s 942.6 MiB/s
12 1s 1.1 GiB/s
16 1s 1.2 GiB/s <- best
Recommended: --io-threads=16
Stored in the hashfile; future runs use --io-threads=16 automatically.
Without root it cannot drop caches, so it measures memory bandwidth, which
keeps scaling with threads well past the point where btrfs metadata
contention caps real cold throughput. It prints a warning and persists the
answer anyway -- into the hashfile the systemd timer then replays forever.
So the documented NAS setup path, followed without sudo, silently locks in a
thread count worse than the default it would have got for free.
That leaves nothing it is good for. On SSD the heuristic is the validated
path, so autotune either agrees (no value) or disagrees by measuring the
wrong thing (negative value). On HDD the constants are admittedly guesses,
but autotune needs root plus cold reads to say anything true, and CLAUDE.md
already records that we cannot measure it meaningfully here. For the "what
is fastest on my disks" question, scripts/bench.py --walk-threads does the
job properly -- cold, interleaved, medians -- as a dev tool that does not
write to the hashfile.
Removes src/autotune.{c,h}, its test, the --autotune mode and its
report-mode exclusivity, the autotune_io_threads config key and its reader,
and dbfile_set_config_int() (autotune was its only caller). Step 2 of the
NAS quick-start is gone and the remaining steps renumbered; the cache-dir
setup it carried moves into the new Step 2.
No schema bump: create_tables() is unchanged and a stale autotune_io_threads
key in an existing hashfile is simply never read. Verified an old hashfile
carrying the key still scans, falling back to the heuristic.
scripts/verify.sh passes (116 tests, valgrind smoke). Closes #150.
Co-Authored-By: Claude <[email protected]>
Review pass over the removal commit. Every doc layer was updated except
CLAUDE.md -- the repo's own guidance file, and the one most likely to send
the next contributor looking for a deleted file.
- CLAUDE.md: rewrite "## io-threads auto-tuning (--autotune)" as "## io-threads
default (storage heuristic)". Keep the fact that matters -- the HDD/pool
constants are unmeasured guesses -- but it can no longer say "treat
--autotune as authoritative", so record instead that there is now no in-tree
way to validate them and point at scripts/bench.py --walk-threads. Note why
--autotune was removed, so it does not get reinvented. Drop --autotune from
the report-modes list, and correct the schema-bump note: removing a config
key is as safe as adding one. Also fixes a stale `options.io_threads_set`
reference (the sentinel is io_threads == 0).
- oans.c/opt.h: rename auto_tune_io_threads() to apply_storage_defaults(). The
old name echoed a flag that no longer exists, and under-described a function
that also sets the scan-ETA rotational weight. Header comment now names both
jobs and why one storage_detect() serves them.
- oans.c: the report-mode comment called the group "read-only", but -R deletes
rows -- the man page this branch just corrected says so explicitly.
- storage.h: the rewritten comment had become circular ("conservative: picks a
safe default"). State the useful thing instead: the SSD path is measured, the
HDD constants are not.
- nas-quickstart.md: rewrap the intro to 80 columns like the rest of the file.
No behaviour change; scripts/verify.sh passes (116 tests, valgrind smoke).
Co-Authored-By: Claude <[email protected]>
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 #150.
--autotuneis the one feature that can quietly make oans slower, and it does so persistently.The measurement
On a 32-core NVMe box the storage heuristic picks 8, matching what
CLAUDE.mdalready records: "8 was the knee (7.3s), 16/32 gave no wall gain whilesysexploded (16→23→46s)".Autotune, run as a normal user, recommends and stores 16:
Without root it cannot drop caches, so it measures memory bandwidth — which keeps scaling with threads long past the point where btrfs metadata contention caps real cold throughput. It prints a warning and then persists the answer anyway, into the hashfile the systemd timer replays forever.
So the documented NAS setup path, followed without
sudo, silently locks in a thread count worse than the default the user would have got for free.Why remove rather than fix
min(nproc, 8)is the validated path, so autotune either agrees (no value) or disagrees by measuring the wrong thing (negative value).CLAUDE.mdalready records that it cannot be measured meaningfully on the dev box.scripts/bench.py --walk-threadsalready answers the question properly — cold, interleaved, medians — as a dev tool that does not write to the hashfile.Gating persistence behind a verified cold measurement was considered; it fixes the harmful path but leaves 532 lines and a documented setup step earning nothing on the hardware most people run.
What goes
src/autotune.{c,h}, its integration test, the--autotunemode and its report-mode exclusivity, theautotune_io_threadsconfig key and its reader, anddbfile_set_config_int()(autotune was its only caller). Step 2 of the NAS quick-start is removed and the remaining steps renumbered — the cache-directory setup it carried moves into the new Step 2. Man page, README, zsh completion, benchmarks and two stale code comments updated.Net −739/+41 across 15 files.
Compatibility
No schema bump.
create_tables()is unchanged and a staleautotune_io_threadskey in an existing hashfile is simply never read again. Verified directly: injected the key into a hashfile, re-scanned, and the run fell back to the heuristic with no error.The heuristic itself is untouched —
storage_recommend_io_threads()and its unit tests are unchanged, so the default sizing behaves exactly as before.scripts/verify.shpasses: build with warnings-as-failure, 116 tests, valgrind scan+dedupe+replay smoke.🤖 Generated with Claude Code