fix(analysis): make library analysis cancellable + cooperative (closes #286)#288
Merged
Merged
Conversation
Issue #286 reports that the post-scan "Auto-analyse tracks" run freezes a mid-tier Windows laptop after ~1500 of 4002 tracks. The log confirms the analyzer was mid-run when the freeze hit (1445 `using xing header for duration` lines from Symphonia, each marking a full-file MP3 decode). The shape of the bug: `run_analyze_library` was a tight loop of `spawn_blocking(analyze_file)` + sqlx upsert, with no yield, no sleep, no cancellation gate. On a 4000-track library that pins at least one core at 100 % for 30+ minutes, leaves zero idle slices for the UI / Defender's per-file open scan / Windows scheduler. The user had no way to stop it short of force-killing the app. This commit adds three coupled primitives that solve all three sub- problems (no cancel, no breathing room, no double-start guard): ## Cancellation - `ANALYSIS_CANCEL: AtomicBool` mirrors the existing `PREFETCH_CANCEL` flag in `crate::commands::lyrics`. Reset to `false` at the start of every run so a stale `true` from a prior cancellation doesn't short-circuit the next call before track 0. - `ANALYSIS_RUNNING: AtomicBool` swap-on-entry double-start guard. Without it, a user click on "Analyze library" while the post-scan `maybe_auto_analyze` hook is still in flight races on the same `INSERT ... ON CONFLICT(track_id)` against `track_analysis` — both runs would redo every Symphonia decode for the same set of pending tracks (free 2x CPU saturation). The second caller now bails with `cancelled = true`. - `RunningGuard: Drop` clears `ANALYSIS_RUNNING` on every exit path — early return, sqlx Err propagation, future `?` insertions, even a decode panic — so the flag can't get stuck and brick auto-analyze for the rest of the session. - New `#[tauri::command] cancel_library_analysis() -> bool` flips the cancel flag and returns whether a run was actually in flight (so the UI can show a confirmation toast). Idempotent. ## Cooperative scheduling Every loop iteration now ends with: tokio::task::yield_now().await; tokio::time::sleep(ANALYSIS_PER_TRACK_PAUSE).await; // 25 ms The yield gives other tokio tasks (UI events, sync drain, playback commands) a turn on the reactor between decodes. The 25 ms sleep hands the OS scheduler back enough time to interleave Defender, Spotlight, mpd, etc. between two CPU-bound decodes. On a 4000-track library that adds ~100 s of idle to wall-clock — about a 2 % overhead vs the freeze risk it prevents. ## Cancellation gate placement The cancel check sits at the TOP of the loop, BEFORE the decode and the spawn_blocking. A user click that lands between two decodes is honoured without burning one extra ~8-second symphonia run. The post-loop summary carries `cancelled: bool` so the frontend can render "Cancelled at X / Y" instead of pretending the run finished. ## Frontend - `cancelLibraryAnalysis(): Promise<boolean>` in `src/lib/tauri/analysis.ts`. `LibraryAnalysisSummary` gains `cancelled: boolean`. - `SettingsView` swaps the Analyze button out for a Stop button while `isAnalyzingLib === true`. The Stop button stays clickable even though the worker loop only exits at the next track boundary — the click resolves instantly but the running flag stays set until `analyzeLibrary`'s awaited promise unblocks, so there's no tug-of-war between the optimistic UI flip and the actual worker state. - New `settings.analyze.cancel` i18n key ("Stop analysis" / "Arrêter l'analyse" / etc.) across the 16 supported locales + the legacy `kr` alias. Brand-style consistency with the existing `settings.analyze.action` ("Analyze") label. ## What this commit does NOT address - The slow first scan (3 min 25 s for 4002 tracks vs 19-42 s in other players) is a separate structural cost — WaveFlow hashes every file with BLAKE3 + extracts cover art + writes per-track rows, which other players don't all do. Defer to its own perf pass. - The auto-analyze flag defaulting to ON in onboarding is unchanged here — the user explicitly opted in via the wizard toggle in the reported case. Defer the onboarding UX rewording to a follow-up. 84 desktop unit tests + workspace `cargo check` clean. `bun run typecheck` clean.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
InstaZDLL
added
scope: frontend
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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.
Draft pour test manuel. Adresse le freeze laptop signalé dans l'issue #286 sur l'auto-analyse post-scan.
Closes #286
Le bug
L'utilisateur a 4002 MP3, lance l'auto-analyse depuis le wizard onboarding, et après ~1500 tracks le laptop freeze + ventilos à fond. Log montre 1445 lignes `using xing header for duration` de Symphonia — confirmation que l'analyzer était en plein full-file decode quand le freeze a touché.
Cause : `run_analyze_library` était une tight loop `spawn_blocking(analyze_file)` + sqlx upsert, sans `yield_now`, sans sleep, sans cancellation. Sur 4000 tracks ça épingle 1 core à 100% pendant 30+ minutes — zéro slot idle pour l'UI / Windows Defender / scheduler. L'utilisateur n'avait aucune sortie sauf force-killer l'app.
Le fix
Trois primitives couplées qui adressent les trois sous-problèmes :
1. Cancellation
2. Cooperative scheduling
Chaque itération de la loop finit avec :
```rust
tokio::task::yield_now().await;
tokio::time::sleep(ANALYSIS_PER_TRACK_PAUSE).await; // 25 ms
```
3. Cancellation gate placement
Le check `ANALYSIS_CANCEL` est au TOP de la loop, AVANT le decode et le spawn_blocking. Un click qui tombe entre deux decodes est honoré sans burner un decode symphonia de ~8 sec supplémentaire.
Frontend
Ce qui N'EST PAS dans cette PR
Tests
Summary by CodeRabbit
Notes de Version
Nouvelles Fonctionnalités
Localization