perf(scan): partial file hash + per-phase timing - #319
Conversation
Instrument scan_folder_inner with wall-clock markers (walk / metadata load / fast-path stat / extract+DB / post-processing) plus cumulative BLAKE3-hash vs lofty-tag CPU time summed across the parallel extraction tasks. Logged once on the existing "scan complete" line so a slow scan on a large library is diagnosable without a profiler — and so we can choose the right optimisation (hash-bound vs tag-bound vs DB-bound) from real data. Diagnostics only; no behaviour change.
Full-file BLAKE3 hashing was the dominant scan cost — instrumentation on a 902-track folder showed extract+DB = 97.7 s of which the hash read ~99% (tag/lofty was 1 s). The scan was disk-throughput bound, reading every byte of every file (~9 GB). hash_file now digests `size + first 1 MiB + last 1 MiB` for files over 2 MiB, instead of the whole file. Strong identity for music: - moved/renamed copies keep the same bytes → same hash (dedup holds), - tag rewrites shift bytes in the head/tail window → hash changes, so edited files still re-extract, - file length is folded in so same-head/tail/different-size can't collide. Migration-free: the rescan fast path keys on (mtime, size), not the hash, so existing rows are never force-rehashed; stored full-file hashes stay valid until the file changes. Tests cover the small/whole-file path, head + size sensitivity, and the documented middle-byte blind spot.
InstaZDLL
added
scope: backend
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesHash partiel, doublons et scan
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/crates/core/src/scanner/extract.rs`:
- Around line 40-55: The partial hash in extract.rs should not be used as the
sole deduplication key because Files larger than 2 * HASH_CHUNK_BYTES can
collide when only the middle bytes differ. Update the dedup flow that consumes
file_hash so it either computes/retains a full-content hash for grouping or
performs a stronger secondary check before forming the duplicate bucket and
showing the removal UI, and verify the logic around the file_hash generation and
the dedup grouping path that the test covering middle-byte changes exercises.
- Around line 682-700: Le test actuel de hash_file couvre la sensibilité à la
tête et à la taille, mais pas la fenêtre de fin, donc une régression sur le
seek/read_exact du tail pourrait passer inaperçue. Étendez
hash_file_large_detects_head_and_size_changes pour modifier un octet dans la
queue du fichier et vérifier que hash_file change aussi, en gardant les autres
vérifications existantes intactes; utilisez les symboles hash_file et
hash_file_large_detects_head_and_size_changes pour localiser la suite de tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a9a0dd21-1e94-4671-bb0b-f210fee4184f
📒 Files selected for processing (3)
docs/features/library.mdsrc-tauri/crates/app/src/commands/scan.rssrc-tauri/crates/core/src/scanner/extract.rs
The scan-time file_hash is now a partial digest (size + head + tail), so a duplicate group keyed on it is only a candidate — two distinct files could in theory share it but differ in unread middle bytes. Since the duplicates UI deletes from a group, find_duplicates now re-verifies each candidate with a full-content hash (scanner::hash_file_full, off-thread, only on the few candidate files), splitting any collision into separate groups and dropping singletons. Also extend the hash_file test to flip a tail-window byte (guards the seek-to-end + read_exact of the tail chunk).
|
@coderabbitai Les deux findings valides, fixés dans 51ee28f :
clippy workspace ✅ · |
|
Tip For best results, initiate chat on the files or code changes.
La revalidation full-content dans Je considère donc ces deux points comme réglés. 🐇✅ ✏️ Learnings added
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/crates/app/src/commands/duplicates.rs`:
- Around line 5-7: The duplicates command currently groups candidates only by
track.file_hash, so legacy full hashes and newer partial hashes for identical
files will never meet and will be missed. Update the logic around the duplicate
search and hash_file_full revalidation to support both hash formats by either
migrating/versioning existing hashes or broadening the SQL prefilter with a
stable field like file_size before grouping, and add a compatibility test that
covers legacy full-hash profiles versus newly scanned partial hashes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c294259c-0ba0-40a7-93c8-676cb5aff013
📒 Files selected for processing (4)
docs/features/library.mdsrc-tauri/crates/app/src/commands/duplicates.rssrc-tauri/crates/core/src/scanner/extract.rssrc-tauri/crates/core/src/scanner/mod.rs
A legacy full hash and a newer partial hash for the same file never match (both are 64-char blake3 hex, indistinguishable), so grouping candidates by file_hash missed duplicates that span the partial-hash rollout. Prefilter by byte size instead — a format-stable field every real duplicate shares (size is also folded into the partial hash) — then let the existing full-content verification form the exact groups. Simplifies the consuming code to a flat candidate list + one bucketing pass, with a deterministic output order. Add a compatibility test: byte-identical files group by content regardless of any stored hash, and unreadable files are excluded.
|
@coderabbitai Finding valide, fixé dans 7a367c2. Hash legacy full vs partiel indistinguables → doublons cross-migration manqués : exact, les deux sont des blake3 hex 64 chars, impossible de les distinguer pour filtrer sélectivement. Solution retenue (ta 2ᵉ option) : prefilter par
Test de compat ajouté : des fichiers byte-identiques groupent par contenu quel que soit le hash stocké ( Note coût : on full-hash les fichiers de même taille. Sur de l'audio les tailles sont quasi-uniques (VBR/FLAC au byte près), donc les groupes de taille ≈ les vrais doublons ; le dedup étant rare + user-initiated, c'est acceptable et désormais correct. clippy app |
|
Tip For best results, initiate chat on the files or code changes.
Le passage du préfiltre de Je considère ce point comme réglé. 🐇✅ ✏️ Learnings added
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/features/library.md`:
- Line 54: The opening sentence in the library docs is outdated:
`commands/duplicates.rs::find_duplicates` no longer groups directly by
`file_hash` alone. Update the wording so it says the command first prefilters
candidates by `file_size` and then verifies/group results using the full-content
hash from `scanner::hash_file_full`, matching the behavior described later in
the paragraph.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d4ca61f4-50ed-404f-8779-badb13ee97f3
📒 Files selected for processing (2)
docs/features/library.mdsrc-tauri/crates/app/src/commands/duplicates.rs
find_duplicates prefilters by file_size and groups with a full-content hash, not by the partial scan-time file_hash directly.
|
@coderabbitai Valide, fixé dans da492da — la phrase d'ouverture dit désormais que |
|
Tip For best results, initiate chat on the files or code changes.
La doc est maintenant cohérente avec l’implémentation : préfiltre par |
1 participant
Résumé
Item E du re-scope v1.6.0 (perf scanner sur grosses libs). Deux commits :
scan complete(walk / meta-load / stat / extract+DB / post + temps CPU hash vs tag cumulés sur les threads parallèles).Diagnostic (données réelles, lib 4000+)
Le hash BLAKE3 du fichier entier = ~99% du coût, borné par le débit disque (lecture de ~9 Go pour 902 titres). lofty/tags négligeable.
Fix
scanner::hash_filedigèretaille + 1er Mio + dernier Miopour les fichiers > 2 Mio, au lieu du fichier entier → lecture ~2 Mio/fichier au lieu de la taille complète (~5×+ moins d'I/O).Identité préservée :
Migration-free : le fast-path rescan clé sur
(mtime, size), pas le hash → aucun re-scan forcé, les hash full existants restent valides jusqu'à modif du fichier.Blind spot assumé : deux fichiers distincts de même taille + mêmes 1er/dernier Mio mais milieu différent collisionneraient — inexistant sur de la vraie musique (tête + taille diffèrent déjà). Documenté + testé.
Tests
3 tests core : chemin whole-file (petits fichiers), sensibilité tête + taille, blind-spot milieu documenté.
cargo test -p waveflow-core hash_file✅. clippy workspace ✅.Doc
À mesurer après merge
L'user relance un scan de la même lib → la ligne
scan complete(instrumentation gardée) confirmera le gain réel.Follow-up séparé (pas dans cette PR)
Les logs ont aussi révélé que l'analyse (BPM/loudness) tourne en concurrence du scan et se bat pour le lock SQLite (
database is locked, INSERT à 5.6 s). À traiter à part — contribue à la lenteur perçue end-to-end.Milestone v1.6.0.
Summary by CodeRabbit