Make a large library affordable: flat idle CPU, and a third of the memory - #39
Merged
Conversation
`vuio-cli` ships the `vuio` binary. It had picked up a direct `rusqlite` with
`bundled`, and `vuio-core/unstable-internals` — a feature core's own docs say
"must never be enabled by a dependent crate" — purely to support a benchmark
generator that lived in `src/bin/`.
Neither was even needed for what the generator did: it imported nothing from
`vuio-core` at all, hand-copying `natural_cmp` and the whole DDL instead. The
feature could not have supplied either one — `natural_cmp` is `pub(crate)` at the
crate root, and `database::sqlite::schema` is a private module.
Move it to `crates/vuio-bench` (`publish = false`) and give `vuio-cli` back exactly
the dependencies it had before. `serde_json` and tokio's `io-std`/`io-util` stay;
those came with the MCP stdio bridge and `src/mcp.rs` uses them.
Being a separate crate is not sufficient on its own: cargo unifies features across
everything it builds at once, so a plain `cargo build` — which is what the release
pipeline runs — would still compile `vuio-core` with its internals open for the
benefit of a tool that never ships. So `vuio-bench` is excluded from
`default-members`, and `cargo tree -e normal` now reports the feature nowhere
outside core's own dev-dependency on itself.
Expose `sqlite::register_collations` under `unstable-internals` so the generator can
teach a direct connection the natural-order collation that two of the browse indexes
are declared with, rather than carrying a copy of `natural_cmp` that would drift.
Also delete two dead benchmark harnesses: `scripts/run-large-benchmarks.{sh,ps1}`
invoked a test target that does not exist and exported `SQLX_OFFLINE` for a
dependency this project has never had, and `tests/cmd.txt` named two more tests that
do not exist either.
A scan built a full `MediaFile` for every file it saw before asking whether the record was stale — canonicalizing the path, probing for a subtitle sidecar and, for audio, parsing the whole container with symphonia. Since the answer is almost always "unchanged", and a scan runs every five minutes, a library that nobody touched was completely re-read 288 times a day. Ask `stat` first. On a 100k library the startup scan drops from 15.7 CPU-seconds to 6.6, the five-minute tick from 13.9 to 5.2, and the scan now reports reading zero files. `tags_version` had to change meaning for that to be safe. It was set only when a tag read *succeeded*, so every video — and every audio file whose container will not parse — carried version 0 forever and would have been re-read on every scan regardless of the new check. It now records which reader last *examined* the record, which is what the "re-read when the reader improves" migration actually needs. Then parallelise what is left. Both passes are now concurrent: one `stat` per file to classify it, then the reads themselves. The reads already ended in `spawn_blocking`, but awaiting them one after another meant a single one was ever in flight, so a scan used one core no matter how many the machine had. Also replace `ScanResult::unchanged_files` with a count. Nothing ever wanted more than the number, and retaining a clone per unchanged file meant building and dropping a copy of most of the index on every scan. `files_read` joins it — the difference between the two is the work avoided, it appears in the scan log, and a test asserts it is zero when a rescan finds nothing changed.
Every 300 seconds the reconciliation loop re-walked every configured root and then asked the filesystem about every path in the index. It did that whether or not anything had happened — and it had the information to know better: the watcher records which roots lost events, the loop calls `take_dirty_roots()`, logs the count, and then sweeps all of them anyway. Sweep the dirty ones. Full sweeps move to `media.full_rescan_interval_hours` (default 24, `0` to disable), which is the right cadence for what they actually guard against — a network share that drops events, or a backend queue that overflowed. The missing-file pass moves to the same trigger for the same reason: a deletion inside a watched root already arrives as an event and is handled per-path, so walking the entire index every five minutes is a backstop running at the wrong frequency. `take_dirty_roots` now returns roots as the configuration writes them rather than as the normalized key they are watched under, since matching them against `media.directories` is the only thing any caller does with them. Two smaller things in the missing-file pass itself. It asked whether each configured root was mounted once per indexed file — a million syscalls for a question with one answer per root — so that is hoisted. And its per-thousand-file progress line drops to `debug`, having been ten thousand `info` lines on a large library.
…poll Two costs that scale with the library and are paid whether or not anything changed. `DirectoryDelta::apply` pruned on every transaction, which during a scan means once per thousand files. Neither prune statement is index-served — one scans the counters, the other scans `directories` with a probe per row — and an insert-only scan cannot drive a count to zero, so all of it was looking for deletions that could not have happened. Prune only when some delta was negative. `get_stats` aggregates the whole of `media_files`: it needs `size` and `mime_family` together and no index covers both. It is also on the path of `/readyz`, `/metrics`, `/metrics/json` and the dashboard's five-second poll, so an idle server with a large library re-read the entire table several times a minute to produce an answer that had not changed. Cache it against a write generation that `execute_write` bumps — every write in the process goes through there, so the answer is never stale, it is simply not recomputed for nothing. The generation is taken before the write and re-checked before the cache is filled, so a result read from the old state cannot be stored under the new number.
`perform_graceful_shutdown` ran an unconditional `VACUUM`, which rewrites the entire database file. On a large library that turned every stop into a multi-gigabyte copy, and it happened regardless of `database.vacuum_on_startup` — the setting whose whole purpose is to say whether compaction is wanted. Tie it to that setting, and say in the setting's own description that it now governs both ends. Write-ahead logging has already made the data durable by then; compaction only reclaims pages left behind by deletions.
SSDP and mDNS were started after the initial media scan, and the scan walks the whole library. On a large one that meant nothing on the network could discover the server until the walk finished — the HTTP socket was accepting, but no TV knew the server existed to connect to it. Start discovery first. A client that connects mid-scan browses whatever is indexed so far, which is strictly better than being unable to find the server at all, and the watcher is already running by then so nothing is missed.
…elete `idx_media_tags_version` and `idx_media_tags_key` are never used. `tags_version` appears in no `WHERE` or `ORDER BY` — it is compared in Rust against a value the scanner already holds — and `media_tags` is only ever read by `media_file_id`, which its primary key already serves. Both were paid on every row indexed: two more b-tree insertions each time. Schema v5 drops them. Dropping an index is not the destructive kind of migration the additive-only rule guards against; an index holds nothing that is not derivable from the table. `write_extra_tags` also cleared the side table before writing a row that had just been inserted, where by construction there is nothing to clear. It still clears unconditionally when updating an existing record, which is what keeps a file whose tags became unreadable from keeping its old ones. Measured at 20k files these did not move the wall clock — a cold scan is bound by the single SQLite writer, and at that size the difference is inside the noise. They are here because they are provably unnecessary work per row, which is the kind that matters at 500k and not before. Also make the benchmark generator write a real, silent MP3 rather than eighteen bytes shaped like a header. The tag reader has to actually parse these, or a cold scan measures the database write path and nothing else, and the cost that dominates a real library — opening and probing every file — never appears in the numbers.
`database.cache_mb` is applied to every SQLite connection separately — one writer and two to four readers — so a server configured with the 128 MB default settles at roughly 640 MB resident, which is what a 500k-file library shows in practice. Nothing said so, in the Admin tab or in the example config, and the number is the largest single contributor to the process's memory.
`visit_files` reports `matched`, the size of the entire result, because DLNA's `TotalMatches` needs it — and computing it means evaluating the query a second time. For a ranked search that is the expensive half: the engine has to find and rank every hit in order to count them, so a page of one costs the same as a page of two hundred and fifty. `/api/media` pages by cursor and never looks at the total, yet ran the count twice per request — once for the pass that collects ids for the metadata overlay, once for the pass that writes the rows. Add `visit_files_page`, which returns only how many rows it visited, and use it there. Defaulted on the trait so no backend has to implement it; SQLite overrides it to skip the count. On a 500k library: a search matching 62,000 files goes from 0.801s to 0.416s, and one matching a single file from 0.120s to 0.061s.
The debounced watcher pairs the two halves of a rename by file system id, because the `from` path is gone by the time the event arrives. Upstream's `FileIdMap` supplies those ids by walking every watched root at startup, `stat`ing each entry, and holding a `HashMap<PathBuf, FileId>` entry per file for the life of the process. On a 500,000-file library that measured as a twelve-second walk and ~90 MB of resident memory that never comes back — a 65 MB hash table plus a `PathBuf` per file — to keep a hint that is only ever read for the handful of paths in a rename. It also does worse than grow: `remove_path` scans the whole map per removal, and a dropped backend event triggers `rescan`, re-walking every root at exactly the moment the system is already struggling. What the ids buy is smaller than the price. A renamed directory is handled by removing the old subtree and rescanning the new one, which is what the unpaired delete-then-create pair produces anyway; a renamed file is the only real difference, worth one stable row id and one skipped tag read. Only macOS and Windows need them at all, since inotify carries a rename cookie — which is why upstream itself uses `NoCache` on Linux. `BoundedFileIdCache` keeps that behaviour while it is cheap and stops paying for it when it is not. The seed walk stops at 25,000 entries and later ids are remembered only for paths that actually saw an event, held in two generations so the older half is dropped wholesale rather than tracked per entry. Libraries under the cap behave exactly as before. Above it, renaming an untouched file degrades to delete-then-create, which the event handler already supports.
To notice edits to the config file, the manager watched the file's parent directory *recursively*, with the debouncer's default file-id cache. The handler then discards every event whose path is not equal to the config file — so the recursion could never match anything, while the cache walked and `stat`ed everything underneath and kept a map entry per file. Where the config lives decides how bad that is. Beside the media it configures (`vuio --config ./vuio.toml`, run from a library folder) it indexes the whole library; in a home directory it indexes the home directory. On the 500,000-file benchmark it was 122 MB resident and about six seconds of startup, all of it before the media scan could begin, for one file compared by equality. Watch one level, and with `NoCache`: the config file is a direct child of the directory, and renames are never stitched here, so the ids were built and never read. Startup on that library goes from 23.3s to 10.7s and steady memory from 469 MB to 355 MB.
An earlier commit said the budget applies per connection "so resident memory settles at several times this figure", inferring a multiple from the number of connections rather than measuring it. A sweep at 500,000 files says otherwise: a connection only allocates pages it reads, and one reader does the heavy queries, so going from 8 to 128 moved resident memory by ~160 MB, not by five times the difference. The sweep also found what the setting is actually for. Folder browsing and flat listing are 1 ms at every value from 8 to 256 — they are index-served and do not touch it. Full-text search is the only thing that responds, and as a step rather than a slope: 0.21s at everything up to 160, 0.12s from 192 up, where the search index finally fits. Advice to raise it "if browsing reads from disk" pointed at the wrong operation and implied a gradient that is not there.
A sweep of `database.cache_mb` at 500,000 files, in release, against every value from 8 to 256: | cache_mb | RSS | browse | list | search | |---|---|---|---|---| | 8 | 355 MB | 0.001s | 0.001s | 0.201s | | 32 | 385 MB | 0.001s | 0.001s | 0.209s | | 128 | 525 MB | 0.001s | 0.001s | 0.210s | | 160 | 565 MB | 0.001s | 0.001s | 0.207s | | 192 | 564 MB | 0.001s | 0.001s | 0.121s | | 256 | 565 MB | 0.001s | 0.001s | 0.122s | Folder browsing and flat listing are served from indexes and are flat at every value. Search is the only thing that responds, and as a step rather than a slope: it stays at 0.21s until the whole search index fits and then halves, which on that library happened between 160 and 192. That makes 128 the worst available point at scale — 170 MB more than 8, and still on the slow side of the step. A default that spends memory has to buy something, and below the step this one buys nothing measurable. Servers that want the faster search need to go past the step, not merely up, and the docs now say so instead of implying a gradient.
`ScanResult` retained a whole `MediaFile` for every file it added or updated — a `PathBuf`, three `String`s, six `Option<String>`, an `AudioTags` of fourteen more fields, a `StreamInfo`, and a `Vec` of every remaining tag. On a library with rich tags that is more than a kilobyte a file, and the records are already in the database by the time the scan returns. Nothing ever read them back. `summary()` and `total_changes()` called `.len()`, and the one production consumer took a `.len()`, a log line and an `.is_empty()`. So a cold scan built most of the index a second time, in memory, to produce three integers — the same mistake `unchanged_files` made before it became a counter. Measured on a 100,000-file library, release: a cold scan drops from 148 MB peak and 169 MB settled to 67 MB peak and 78 MB settled. The scan itself is unchanged in wall time. Tests that inspected a retained record now read it back from the database, which is where the property they assert about — a canonicalized path, a symlink indexed under its target but named for the link — actually has to hold.
`DiagnosticsCollector::refresh` called `sysinfo`'s `refresh_all()`, which walks the whole process table and stores a record per process, then read exactly one of them: our own pid. The heap profile showed a couple of megabytes rebuilt on every sample, and `/metrics/json` samples every five seconds for as long as a dashboard is open. Refresh what the snapshot actually reads — system memory, CPU usage, and our own pid — and take the CPU list once at construction, since `refresh_cpu_usage` does not build it and the count cannot change. `new_all()` goes with it, on every platform rather than just FreeBSD, since its only purpose was to prime that same table. The FreeBSD carve-out for it is no longer needed; the disk one stays, being a separate upstream bug. The cost is the one FreeBSD already accepted: the first sample reports no process CPU usage, having no earlier reading to difference against. The test now asserts the fields that a too-narrow refresh would silently empty — CPU count, system memory, and our own process's memory and runtime.
`validate_and_cleanup_deleted_files` loaded every fingerprint in the index as one `Vec` and then called `path.exists()` per row — a blocking syscall, once per file, on an async worker. It runs at startup and again on the periodic sweep, so a library's worth of records was materialised twice over on a schedule. Page it instead, by id, and hand each page's existence checks to the blocking pool in one hop rather than one per file. Every row is still considered — this is the pass that prunes records belonging to no configured library at all, so it cannot be scoped to a root the way a scan can. Measured at 100,000 files, release: a cold scan settles at 77 MB rather than 94, and a rescan of an unchanged library peaks at 87 MB rather than 92. Together with the streaming scan, the 100k figures are now 66 MB peak / 77 MB settled for a cold scan and 87 MB for a rescan, against 148 / 169 / 108 before this branch. The paging is what the new test covers: an index larger than one page is where a paging mistake hides, since rows past the first page would simply never be checked and deleted files would stay indexed forever.
A window of 4096 fed a writer batched at 1000, so a first scan wrote 4096 rows at a time instead of the batch size the rest of the code is tuned around. Match them: one window is one write, and the flush no longer needs a threshold argument to distinguish "batch is full" from "this is the last of them".
Three lints on code added by this branch: `ScanResult` lost its `Default` when the derive was dropped, the scan's progress check spelled out a modulo that `is_multiple_of` says better, and a cache test compared a length to zero. `Default` is written out rather than derived on purpose. Every count starts at zero, but `complete` starts *true* and is cleared by anything the scan could not enumerate — a derived one would start it false and mark every scan incomplete.
Narrowing the config watch from a recursive parent to a non-recursive one broke config reload on FreeBSD: three tests that had passed for years started failing, because an edit to the file was never noticed. kqueue reports per file descriptor. A watch on a directory says the directory changed and names the directory, so rewriting a file *inside* it is invisible unless that file's own descriptor is registered — and the handler identifies its config by comparing paths for equality, so a directory-named event never matches. The descriptor used to be registered by accident: notify's kqueue backend walks the tree for a recursive watch and registers every entry it finds, which is the very walk that made a config living beside a media library index the library. So watch both, neither recursively. The file, so a write into it is seen on every backend. The parent, because saving a config usually replaces it — write a temporary file, rename it over the old one — and that destroys the inode the file watch was holding, which only the directory can see. Verified by building against `notify/macos_kqueue`, which swaps the same backend in locally: the three tests fail without the file watch and pass with it.
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
1 participant
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.
A 500,000-object library was unusable: the server burned CPU forever re-checking
files that had not changed, and held hundreds of megabytes to do it. This branch
fixes both, and gives
vuio-cliback the dependency set it had before thebenchmark work borrowed it.
Measured
Release build, on generated libraries with real files on disk.
/api/browse,/api/mediaand/readyzstay under a millisecond at both sizes.Idle CPU is 0.36 s per 222 s at 500k and the same at 100k — the point of the
branch is that idling no longer scales with the library.