diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b2dd0a2f..47a048dd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -128,23 +128,27 @@ jobs: echo "code=${{ github.run_number }}" >> "$GITHUB_OUTPUT" - name: Fetch TTS runtime AAR - # sherpa-onnx neural-TTS + ASR runtime (Kokoro voice, Whisper voice search). It's a 57 MB + # sherpa-onnx neural-TTS + ASR runtime (Kokoro voice, Whisper voice search). It's a ~47 MB # prebuilt AAR (no Maven artifact) hosted on the fixed-tag `tts-runtime` release, gitignored # out of the repo - fetch it into app/libs/ so :app can package the arm64 AND armeabi-v7a # .so. Verify the size so a truncated download fails loud instead of producing a - # runtime-crashing APK. + # runtime-crashing APK. (The threshold is sized for 1.13.4's 48,847,529 bytes; the 1.13.3 + # AAR was 57 MB, and the old >50 MB check failed the UPGRADED artifact - resize this when + # the AAR is bumped, it is a truncation guard, not a version pin.) run: | mkdir -p app/libs # Hosted on this repo's fixed-tag `tts-runtime` release (a one-time manual upload). - curl -fSL -o app/libs/sherpa-onnx-1.13.3.aar \ - "https://github.com/${{ github.repository }}/releases/download/tts-runtime/sherpa-onnx-1.13.3.aar" - test "$(stat -c%s app/libs/sherpa-onnx-1.13.3.aar)" -gt 50000000 + # 1.13.4 (onnxruntime 1.27.0) is REQUIRED, not preferred: its bundled runtime fixes the + # armv7 unaligned-read SIGBUS that crashed every model load on 32-bit phones (issue #95). + curl -fSL -o app/libs/sherpa-onnx-1.13.4.aar \ + "https://github.com/${{ github.repository }}/releases/download/tts-runtime/sherpa-onnx-1.13.4.aar" + test "$(stat -c%s app/libs/sherpa-onnx-1.13.4.aar)" -gt 40000000 # The hosted AAR must actually CARRY 32-bit ARM, not just be big enough. If it is ever # replaced with an arm64-only build, the v7a strip fix (#81) silently reverts: the APK # still builds, still installs on a TCL Flip 2, and voice search + Vela voice are dead # again with an UnsatisfiedLinkError nobody sees until a tester reports it. Fail here # instead - this is the check whose absence let the original bug ship. - unzip -l app/libs/sherpa-onnx-1.13.3.aar | grep -q "jni/armeabi-v7a/libsherpa-onnx-jni.so" \ + unzip -l app/libs/sherpa-onnx-1.13.4.aar | grep -q "jni/armeabi-v7a/libsherpa-onnx-jni.so" \ || { echo "::error::tts-runtime AAR has no armeabi-v7a sherpa-onnx - 32-bit phones would lose voice search + neural TTS"; exit 1; } - name: App unit tests diff --git a/AGENTS.md b/AGENTS.md index f0ea14a9..59bcd6d4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -747,9 +747,438 @@ state - upstream's own 13ac02e8 already made the layers panel a VelaMenu): (raises the ceiling ~2x); don't remove it. (2) **Any Overpass / large-HTTP-body reader MUST stream-parse** - `Json.decodeFromStream(body.byteStream())` into a tiny `@Serializable` DTO, NEVER `resp.body.string()` + `parseToJsonElement` (that held ~5-10x the wire size in transient heap and - OOM'd mid-read - the Flock `out body` fetch per pan did this; fixed in `OverpassAlprCameras`, - `OverpassTrafficSignals`/`OverpassPois` are the same pattern + a pending follow-up). And NEVER lower a + OOM'd mid-read - the Flock `out body` fetch per pan did this). And NEVER lower a per-viewport Overpass fetch's min-zoom without shrinking the box. + **The Overpass follow-up is DONE (audited 2026-07-20):** `OverpassAlprCameras`, `OverpassTrafficSignals` + AND `OverpassPois` all `decodeFromStream` today. This line previously said the latter two were pending, + which sent a low-RAM investigation chasing already-fixed code. The remaining fully-buffered hot reader + is the GOOGLE ambient path, not Overpass: `GoogleMapsDataSource.get()` ends in `.string()`, and + `GoogleResponse.parse` then makes a `substring` copy plus a full `JsonElement` DOM - times a 15-term + fan-out per pan. That, not Overpass, is the ~180 MB/12 s. It cannot simply `decodeFromStream`: the + payload is a positional nameless array walked by `at(0,1,3)` paths, so there is no DTO to decode into. + The levers that DO move it are the term count and the `!7i` pool size. + +- **Low-RAM devices are a FIRST-CLASS target, and the app now adapts to them (issue #83, 2026-07-20).** + D-pad-first means feature-phone-first, and those phones are memory-poor as well as small. + - **`app/ui/MemoryPressure.kt` is the one seam.** `init()` from `VelaApp` classifies the device + (`ActivityManager.isLowRamDevice` OR heap class <= 127 MB) and `VelaApp.onTrimMemory` fans every + `TRIM_MEMORY_*` out to registered holders. **Anything that allocates something large or NATIVE + must register a release callback.** Registration, never a Hilt entry point: reaching a singleton + from a trim would CONSTRUCT it, so the trim would allocate the very thing it is freeing. + - `LowRamMode.enabled` (`:core`) is the `:core`-visible mirror, pushed in by `VelaApp` - same seam + as `CategoryFilter.enabled`, because `:core` must never read an `:app` holder. + - **The low-RAM predicate lives in `:core` as `LowRamMode.classify`, and it has TESTS.** It has + shipped wrong twice, both times in ways no dev device could reveal: `heapClassMb in 1..127` + excluded 128, the one heap class 1 GB phones and low-end 2 GB phones actually use - so the + device issue #83 was filed from could plausibly have received none of the work - and a 0 from a + failed probe fell out of that range and selected the memory-HUNGRY path for a device we knew + nothing about. It now takes three signals (`isLowRamDevice`, total RAM <= 2048 MB, heap class + <= 128 MB), **treats an unreadable probe as constrained**, and lives in `:core` purely so + `core/src/test/.../LowRamModeTest.kt` can pin the boundaries. Failing toward low-RAM costs a + roomy phone about a second on its first mic tap; failing the other way can OOM a phone with no + headroom. + - Total RAM is the signal that actually describes the device; heap class is a Dalvik knob an OEM + can set to anything. `MemoryInfo.totalMem` reports what the OS can hand out, so a nominal 2 GB + phone reads ~1900 MB and a 3 GB phone ~2800 MB. The M5 reads 2878 MB and stays on the normal + path, which is what keeps every measurement in this file comparable - there is a test asserting + exactly that, so if it ever flips you will be told. + - **`resetprop` exercises REAL low-RAM detection, and is the ONLY way to do it on a release + build.** `debug.vela.lowram` is `BuildConfig.DEBUG`-gated, so it is inert on `staging`/`release` + - the low-RAM branches had therefore never run on a minified build at all. + + adb shell su -c "resetprop dalvik.vm.heapgrowthlimit 96m" # then relaunch + adb shell su -c "resetprop dalvik.vm.heapgrowthlimit 256m" # restore + + `ActivityManager.staticGetMemoryClass()` reads that property per call, so the app sees the new + heap class immediately and `LowRamMode.classify` runs for real (`forced=no`). Magisk's + `resetprop` is what makes a `ro.`-style property writable. RESTORE IT - it changes the heap + growth limit for every app started afterwards. + - **Verify by BEHAVIOUR, not by log, on staging.** `Timber.plant(DebugTree)` is DEBUG-gated, so + `MemoryPressure init` never reaches logcat on a production build. Use the renderer count + instead: low-RAM skips the speculative WebView warm, so + `adb shell ps -A | grep -c sandboxed_process` is 0 after a search on the low-RAM path and 1 on + the normal one. That signal was 0/0/0 versus 1/1/1 across three pairs - perfectly separated, + unlike PSS. + - Measured this way on `staging`, the low-RAM path is worth about **148 MB**: 166/169/173 MB + against 287/372/295 MB. It also proves R8 did not break those branches (0 crashes). + - **Simulating memory pressure: the pages must stay HOT or you measure nothing.** A hog that + allocates once is simply compressed into this device's 1.6 GB of zram, and `MemAvailable` goes + UP - the first attempt at this "applied" 1500 MB and freed 148 MB. Re-touch every page in a loop + to deny them to the swapper. Then it bites: `MemAvailable` fell to ~130 MB and lmkd began + killing on "direct reclaim and thrashing". + - **What that revealed, and it matters for the whole design: trims are not a reliable defence.** + Under real pressure `staging` received **zero** `onTrimMemory` callbacks and was killed 20 s + in (`oom_score_adj 0`, reason "device is not responding"); the debug build at a gentler 1.6 GB + got **exactly one** `level=15`, released and purged in 1 ms, and was killed 8 s later. lmkd + kills on thrash-driven unresponsiveness before AMS gets round to asking anyone to release. + **Proactive reclaim - the idle reapers, not warming what will not be used, not holding a + document after a scrape - is what actually protects a constrained phone.** A release that only + happens on trim mostly does not happen. + - Do not push past ~1.6 GB on this device. At 1.9 GB the launcher enters a kill loop and the app + dies before `MemoryPressure.init` even runs, so the test stops discriminating between good and + bad memory behaviour and only says "the device is broken". A continuously-rewritten 1.9 GB is + a pathological workload, not a small phone. + - **Verify the low-RAM path or it ships unverified.** Every dev phone we own reports + `lowRam=false heapClassMb=256`, so those branches are dead code locally. Debug builds honour + `adb shell setprop debug.vela.lowram true` (then relaunch). NB `false` FORCES the normal path, + it does NOT clear the override - clearing needs an unparseable value, so use + `setprop debug.vela.lowram none`. The two only look equivalent because every dev phone we own + detects as normal anyway. `setprop ""` is a shell syntax error, not a reset. + - **Measuring: `am send-trim-memory` REFUSES background levels on a foreground process** + ("Unable to set a background trim level on a foreground process"). Press HOME first. A harness + that discards that stderr measures NOTHING and reports a clean baseline - this happened here and + produced a whole benchmark of void numbers before the error was noticed. Always check it. + - Measured on an M5 (2.9 GB, Android 13, standardDebug, median of 5 cold starts) main vs the fix, + low-RAM path: peak PSS 831 MB -> 581 MB (-30%), post-trim 397 MB -> 246 MB (-38%), native heap + 223 MB -> 95 MB (-57%), cold start 4811 ms -> 4333 ms. Idle PSS run-to-run variance is +-60 MB, + so single idle readings prove nothing; compare post-trim, which is paired within a run. + - **The ASR model is the single largest reclaimable allocation: ~267 MB PSS** (~101 MB of weights + in `scudo:secondary` plus ~146 MB of onnxruntime arena in `scudo:primary`). It releases on a + severe trim, is NOT warmed at startup on a low-RAM device, and - on EVERY device - is dropped + after `REAP_IDLE_MS` unused and rebuilt on next use. The window is RAM-SCALED (120 s low-RAM, + 600 s roomy): a reload is ~1 s of dead mic on the next tap, and on a roomy phone that latency + regression buys nothing - pre-reaper those devices held the model all session and were fine. + `WhisperRecognizer.release()` declines while a listen is in flight - freeing the native + recognizer under a running decode is a use-after-free that takes the process down rather than + throwing. + - **A model's file size says NOTHING about its resident cost - measure before adopting.** NeMo + Conformer CTC small is a 46 MB int8 file that ballooned to ~760 MB-1.2 GB PSS through + onnxruntime on the M5 (rejected); Moonshine's small weights still cost ~212 MB across its four + ORT sessions - no lighter resident than Whisper tiny's ~214 MB (both same-protocol launch + deltas, 32-bit M5). k2 Zipformer small (26 MB encoder) was built, wired and then REMOVED + before merge: resident size is not the only bar - it is a 2023 librispeech (audiobook-domain) + model, and a maps app lives on the proper nouns that domain mishears; no post-processing fixes + that. The 267 MB Whisper hold is TRANSIENT now (idle reap), which is what actually makes it + viable on small phones. "Encoder-only means small" and "fewer MB means less RAM" were both + device-refuted in one afternoon. The clean way to isolate one model's resident cost: let the + IDLE REAP fire (it releases ONLY the recognizer) and diff `Native Heap` pre/post inside one + settled process - launch-to-launch totals swing +-150 MB with map content and prove nothing at + engine granularity. + - **Do not reach for a device gate when an IDLE gate will do.** The warm-at-startup behaviour was + first made low-RAM-conditional, which protected the instant-first-mic-tap UX on roomier phones + but left them holding 267 MB all session. Reaping on idle keeps that UX AND reclaims the memory + everywhere: device-verified on the 2.9 GB M5, `scudo:secondary` 111 MB -> 9 MB at the 120 s mark + with the model rebuilding on next use. Idle PSS 421 MB -> 299 MB (-29%) on a NON-low-RAM device. + Ask "can this be released when unused?" before "which devices should get less?". + - **`MapView.onLowMemory()` must be called.** MapLibre's tile/glyph/sprite caches are native and + that is the only way to shrink them; nothing called it before. + - **Cutting the ambient TERM COUNT does not cut peak memory, and it silently deletes POIs.** The + low-RAM path briefly fetched 8 of the 15 category terms. Both halves of that were wrong. + - Peak is set by `ambientFanout`, a `Semaphore(4)`, and every buffer (response String, stripped + copy, `JsonElement` DOM) is allocated INSIDE `withPermit`. At most 4 exist at once however many + terms are queued behind them, so 15 -> 8 changes the number of WAVES, not the peak. The + semaphore's own KDoc already said this: "Bounding to 4 caps the peak transient heap with the + same final pool." The levers that DO move the peak are the permit count and the response size + (`!7i`), and only the latter is used. + - Its justification was false. It kept `school` and `park` on the grounds that only they lack a + second source while the ambient layer is up. NOTHING has one then: `VelaMapView` sets + `poi_r1/poi_r7/poi_r20` to `NONE` **wholesale** on `if (navMode || ambientPois.isNotEmpty())`, + not per category. The dropped terms lost their fallback identically. Parks at least keep a + landuse polygon so the green area survives without the pin; a gym, bar or pharmacy exists ONLY + as a pin, making those the worse things to drop, not the safer ones. + - The observation behind it was real (a 6-term subset did lose every park and school pin, caught + by an A/B screenshot). The GENERALISATION drawn from one observation was not. When a screenshot + shows category X vanishing, that is evidence about the fan-out, not about X being special. + - **A Kotlin `release()` does NOT give memory back to the KERNEL, only to scudo.** Freeing a model + or a WebView returns its pages to the allocator's free lists, where RSS/PSS still count them and + lmkd still sees a fat process. `mallopt()` is the only way to hand them on and it is reachable + only from C, which is why `app/src/main/cpp/velamem.cpp` exists - the app's ONLY native module, + ~4 KB per ABI, built for `arm64-v8a` + `armeabi-v7a` only. `MemoryPressure.dispatch` schedules + it 750 ms after every trim (the delay matters: the WebView reapers post `destroy()` to the main + looper and `VelaApp` clears Coil after `dispatch` returns, so an inline purge would run before + the memory it is meant to reclaim was actually freed). + - Measured on the M5, 3 alternating A/B pairs, ASR-model-release scenario: with the purge + suppressed `scudo:primary` moved 60/32/28 KB in the 7 s after a severe trim, i.e. NOTHING; + with it on, 3704/3008/2792 KB. No overlap. A second 8-pair A/B over map/POI churn showed the + same shape, 3345 KB -> 6931 KB mean reclaimed (Mann-Whitney U=7, n=8/8, p<0.05). + - Do NOT expect this to reclaim the onnxruntime arena. It is worth a consistent ~3 MB, not tens. + The ASR model's ~111 MB lives in `scudo:secondary`, which is mmap-backed and comes back on + `free()` with no purge needed - measured 111 MB -> 7 MB in BOTH arms. The purge only moves + `scudo:primary`, which stays around 75 MB either way. + - `M_PURGE_ALL` is API 34+. On the Android 13 dev phone it returns 0 and the code falls back to + `M_PURGE` (API 28+). The logged `mode=` says which actually took (2, 1, or 0 for neither); + do not assume `M_PURGE_ALL` ran just because `all=true` was passed. + - **Backgrounding delivers `TRIM_MEMORY_UI_HIDDEN` (20), NOT `TRIM_MEMORY_BACKGROUND` (40).** + Measured on the M5: pressing HOME logs `dispatch level=20` and nothing more; 40 arrives only + later, once the process sinks in the LRU list under real pressure. `isSevere` starts at 40, so + it is deliberately FALSE at the single most common moment the app is handed. That is right for + releasing (do not thrash a model on every HOME press) and wrong for purging, which is why the + purge triggers from `TRIM_MEMORY_RUNNING_LOW` (10) up. Anything that should happen "when the + user leaves the app" must key off 20, not 40. + - **A/B a memory change on ONE binary or the arms differ in more than the change.** Two builds + also differ in background settling, and idle PSS swings +-60 MB run to run, so a cross-build + comparison cannot attribute a few MB to anything. `adb shell setprop debug.vela.nopurge true` + suppresses the purge at runtime, which makes the delta a paired within-run measurement. Verify + the gate really gates before trusting either arm: one arm must log `native purge suppressed` + and the other `native purge all=... mode=...`, or the A/B is measuring one thing twice. + - **The hidden WebViews are the single largest thing Vela costs, and app PSS CANNOT SEE IT.** + Android runs the WebView renderer OUT OF PROCESS. Measured on the M5 after one search: + `sandboxed_process0` at 305-347 MB PSS, plus `webview_service` 21 MB and `webview_apk` 37 MB, + none of it in the app's own `dumpsys meminfo`. Every issue-#83 number was app-PSS only, so the + largest item in the app was invisible to the whole exercise. **When measuring memory here, + always `adb shell ps -A | grep sandboxed_process` and total the WebView processes too.** + - The app-side half is `GL mtrack`, and it is huge: 26 MB with no WebView alive, 396-461 MB once + the two scraper WebViews exist. That is the offscreen layouts (`WV_WIDTH`x`WV_HEIGHT`, e.g. + 1200x3200) allocating graphics buffers charged to OUR process. Chromium logs + `tile memory limits exceeded` at that size. Cutting the offscreen viewport is a real lead. + - **MEASURE THE `staging` VARIANT, NOT `debug`.** `staging` is `initWith(release)` - R8-minified, + resources shrunk, non-debuggable, installs side by side as `app.vela.staging` - so it is the + production memory profile without touching a real release install. Every number in issue #83 was + `standardDebug` and overstates the app substantially: + + | state | standardDebug | standardStaging (production) | + |---|---|---| + | after a search (warm) | ~460 MB | **~279 MB** | + | place open | 410-508 MB | **335-392 MB** | + | after a severe trim | - | **140-146 MB** | + | `Code` bucket | 101 MB | **30-45 MB** | + + The `Code` gap is extracted dex and JIT profiles that simply do not exist in a release build, so + roughly 55-70 MB of any debug reading is an artifact. Check a conclusion against `staging` before + spending effort on it. + - **`mallinfo` "free" is address space, NOT reclaimable resident memory. Do not chase it.** The + arena routinely reports something like 440 MB total against 41 MB live, which looks like ~400 MB + waiting to be reclaimed. It is not: scudo has already madvised those pages away, and + `scudo:primary` PSS at that same moment was only 67 MB. Measured directly by purging during + active use with no listener release (a `RUNNING_MODERATE` trim, which `isSevere` excludes): + `scudo:primary` moved 67.1 -> 64.5 MB, i.e. **2.6 MB**. A periodic idle purge is therefore not + worth building; the earlier framing of that gap as reclaimable was wrong. + - **What the `mallopt` purge is actually worth: ~10 MB, on production.** A/B on `staging`, 3 runs + per arm, comparing where `scudo:primary` SETTLES after a severe trim (the pre-trim value swings + 147-382 MB run to run and is useless): purge on 52.4/53.1/54.1 MB, purge off 54.4/58.6/76.7 MB. + A single trim reclaims 133-328 MB on production, but nearly all of that is the registered + listeners releasing plus what the platform already does on trim - only ~10 MB is the purge. Do + not credit the purge with the whole trim delta; run the control. + - **Throw the scraped DOCUMENT away when the scrape ends; the viewport is not the lever.** After + a scrape the photo fetcher used to hold a fully rasterized Google Maps page until the 120 s reap, + i.e. through the whole time the user reads the place sheet. `blankAfterScrape()` navigates to + `about:blank` in `fetch()`'s `finally`. Measured at place-open + 75 s: **`GL mtrack` ~497 MB -> + 64-70 MB and TOTAL PSS ~950 MB -> 410-508 MB**, photo counts unchanged. + - Resizing does NOT work, and this was measured before believing it: shrinking the view to 0x0 + after a scrape reclaimed nothing at all (494/496/497 MB against a 497/498 MB control). Chromium + keeps the tiles it has rasterized for a live document regardless of view size. **Document + lifetime is the only lever with leverage here** - the 1200x3200 viewport is 3.84 Mpx = 15 MB at + 4 B/px, yet GL mtrack was ~490 MB, so the number is a whole composited layer tree against a + tile budget, not one viewport buffer. Stop spending device time on geometry. + - **`about:blank` opens the next fetch's load gate early unless you guard for it.** Parking the + view at `about:blank` broke the NEXT scrape: its `onPageFinished` fires on the freshly installed + `webViewClient`, completes the `ready` gate before the real page commits, and the scraper injects + into an empty document. **Caught only by opening a SECOND place** - the same place scraped 33 + photos as the first place opened and 0 as the second. `onPageFinished` now ignores `about:` URLs. + - Test the second place, every time. A one-place test cannot see any bug in WebView REUSE, and + re-tapping the SAME place is served from the LRU cache without scraping at all, so it cannot + see one either. Confirm from the log that two DIFFERENT featureIds actually scraped. + - **Do not LAY OUT a scraper WebView until it is actually scraping.** `WebPhotoFetcher` sized its + view inside `ensureWebView`, i.e. at construction, and `warm()` goes through `ensureWebView` - so + a speculative warm built a full 1200x3200 composited surface over `maps?hl=en`, a page with zero + scrapeable content, and held it for the entire 300 s warm window on a 480x640 phone. Sizing moved + into `sizeForScrape(wv)`, called immediately before `loadUrl` in `fetch()`. Matched A/B, same + harness, 3 runs per arm, search-then-browse with no place opened: + **`GL mtrack` 448/427/441 MB -> 77/71/72 MB (-365 MB) and TOTAL PSS 866/854/852 MB -> + 485/460/459 MB (-390 MB)**, with the scrape byte-for-byte unaffected (28/28/28 photos on the + same place in both arms). + - This fetcher is the ONLY one that lays out during a warm. `WebPopularTimesFetcher.prewarm`, + `WebDirectionsFetcher` and `WebStopDeparturesFetcher` never call measure/layout at all, and + `WebReviewsFetcher` has no warm. That is why every GL number in this codebase tracks THIS view, + and why a reviews-side change looked like it helped when it could not have. + - The size itself is load-bearing at scrape time - the grids virtualize, so at 0x0 a category tab + renders about one tile and the scrape comes back nearly empty. Size BEFORE `loadUrl` so the + page's first layout is already at scrape geometry. Deferring the layout is safe precisely + because it leaves scrape-time geometry identical; SHRINKING it is not the same bet. + - Gate any viewport change on scrape COUNT, not memory. Both fetchers log + `scraped N photos/reviews for ` for exactly this. A change that halves memory and + quietly halves the gallery is a regression no memory metric shows. + - **A quality metric stuck at zero cannot fail, so it proves nothing.** A 720 px width was tried + and reverted: on the photo side it held (28 -> 28) but on the reviews side the scrape returns 0 + for every place tried at 1200 AND at 720 - a pre-existing failure - so that arm was + unfalsifiable. Check the control can produce a non-zero result before trusting an A/B. + - `GL mtrack` at place-open is BIMODAL (~490 MB laid out and alive, ~71 MB not), so single + readings there mean nothing. Measure the warm window, repeat, and report the spread. + - **BOTH speculative warms have to be bounded or neither helps.** The renderer is SHARED by every + WebView in the process, so one un-reaped view keeps it alive for all of them. `WebPhotoFetcher` + had no idle reaper at all, and `WebPopularTimesFetcher.prewarm()` created a view and never + called `scheduleReap()` (only `fetch()` did). `MapViewModel` warms both on every search, so one + search pinned the renderer for the session. Device-verified the hard way: reaping only the photo + view left the renderer alive at ~200 MB because the popular-times view still held it. Fixing + both, with no trim involved, took the renderer to zero and app PSS 744 MB -> 351 MB. + - A speculative warm gets a LONGER reap window than a real fetch (`WARM_REAP_IDLE_MS` 300 s vs + `REAP_IDLE_MS` 120 s). The warm exists so the first place tap skips the cold start; reaping it + at 120 s would expire during an ordinary browse and waste the warm entirely. Bounded, not + short, is the goal - the bug was session-long, not "not aggressive enough". + - **A reap must drain `pending`, exactly like `rendererGone` does.** Destroying the view kills the + injected scraper, so nothing will ever complete those deferreds; a reap landing mid-fetch parks + the fetch in `deferred.await()` for the full `TOTAL_TIMEOUT_MS` (40 s) while it HOLDS the + fetcher's `Mutex`, stalling everything queued behind it. An empty result is the documented + best-effort failure; a 40 s hang is not. + - **Verifying a reap needs a LOG, not a process check.** `WebView.destroy()` does not kill the + renderer promptly - measured 220 MB still resident 8 s after a destroy and the process gone only + minutes later - and an OS trim can kill it for unrelated reasons, so "the process went away" does + not mean your timer fired. The first attempt to verify this was unfalsifiable for exactly that + reason: the renderer vanished at t+60 s and the logs showed `dispatch level=15`/`40`, i.e. a real + trim, not the reaper. Log the reap, then assert the log AND assert no severe trim fired. + - **Freeing a native model needs a LEASE, not an atomic counter checked outside the lock.** + `WhisperRecognizer` guards the recognizer with `leases`, mutated ONLY under `loadLock`, and + `release()` checks the count and frees inside that same lock. The first version of this checked + an `AtomicInteger` before taking the lock while `ensureRecognizer()` handed the pointer out on a + lock-free fast path, which is a check-then-act: the reaper reads 0, a mic tap increments and + takes the pointer, the reaper frees it under the running decode. `runCatching` around the decode + CANNOT save you - `OfflineRecognizer.release()` frees C++ memory and the result is a SIGSEGV in + `libsherpa-onnx-jni` that takes the process down. **An atomic counter does not make a + check-then-act atomic.** Take the lease and the pointer under one lock, or do not take either. + - **`release()` must not block the main thread, so it `tryLock`s.** It is called from + `onTrimMemory` on the main thread, and `loadLock` is held across the ~1 s native model load, so + a blocking acquire stalls the UI thread for that whole load just to reclaim memory the idle + reaper would reclaim anyway. Skipping is safe; the next trim or the reaper retries. `Remove + model` passes `wait = true` because there the user asked for it - and `deleteAsrEngine` runs + that whole sequence (wait + native free + the up-to-154 MB recursive delete) on `Dispatchers.IO`, + never the UI thread. Device-verified that this window is REAL and not theoretical: hammering + `am send-trim-memory RUNNING_CRITICAL` across startup logged `release skipped, model load + in progress` 5 times in one run. + - `RUNNING_CRITICAL` is the level to use for this - it is `isSevere` AND the OS accepts it on a + FOREGROUND process, so it exercises the load window without needing HOME first. + - **The lease protects EVERY path that frees the recognizer, including the engine-switch rebuild.** + `ensureRecognizerLocked`'s key-mismatch branch must not `release()` the superseded model while + `leases > 0` - a listen can still be decoding inside it (mic tap, then Settings "Use" on another + engine). It parks the old recognizer in `retired` instead, drained under `loadLock` once the + lease count returns to zero. Holding two models briefly is the price of not freeing one under a + running decode. + - **Taking a lease across a suspension point must be CANCELLATION-SAFE.** `withContext { acquire() }` + runs the block to completion and then throws `CancellationException` INSTEAD of returning the + value when the caller was cancelled mid-load - so a `finally` keyed off the returned reference + leaks the lease forever (every release path then declines for the rest of the process, and the + 267 MB becomes unreclaimable). `listen()` sets a `leased` flag INSIDE the block via `.also {}` + and keys the `finally` off the flag, not the reference. + - **A trim-triggered WebView reap DECLINES while a fetch is in flight, except at CRITICAL.** All + five fetchers' `reapNow(force)` skip teardown when `pending` is non-empty and the trim is merely + severe - destroying the view kills the injected scraper and turns a live gallery/reviews/ + directions fetch into an empty panel, and the fetch's own `finally` re-arms the reap moments + later anyway. A CRITICAL trim forces the teardown and then DRAINS `pending` (complete-empty), so + the stranded fetch fails fast instead of parking in `deferred.await()` for the full timeout + while holding the serializing mutex. All reap bookkeeping is main-thread-confined via `onMain` + in every fetcher - `reap` scheduling is a read-modify-write and @Volatile does not fix one. + - **The sherpa-onnx AAR is pinned at >= 1.13.4 because of 32-bit ARM.** Its bundled onnxruntime + (1.27.0) fixes an unaligned-read SIGBUS (`BUS_ADRALN`) that crashed every MODEL LOAD on + armeabi-v7a - TTS and ASR both, uncatchably, at startup (issue #95). Device-verified both ways + on an M5 forced to `--abi armeabi-v7a`: 1.13.3/ORT 1.24.3 SIGBUSes in `libonnxruntime.so` on + the loading thread; 1.13.4/ORT 1.27.0 loads and releases clean. The feature phones this fork + exists for have NO system speech engine - the downloaded models are their only voice - so "gate + voice off on 32-bit" is not an acceptable fallback and the runtime must keep working there. + Test any AAR bump on a 32-bit install before shipping it. + - **A quarantined model makes `warmUp()` a silent no-op.** The corrupt-model quarantine keys + (`asr_model_bad_`, `piper_model_bad_` in `vela_settings`) are only ever + lifted by the installer paths' `clearQuarantine()`. Side-loading model files by hand leaves a + latched flag set, so the model never loads, `scudo:secondary` sits at ~11 MB instead of + ~111 MB, and a memory benchmark silently measures the model-absent case. Check `secondary` is + actually model-sized before believing any ASR/TTS memory number - and clear BOTH the bad flag + and the strike counter when resetting a device by hand. + +## Performance: what has been measured + +- **The slowness users feel is CONTENT LATENCY, not frames. Measure that axis first.** + Device-measured on staging: **tapping a place to a complete gallery is 28.8 s** (35 photos). The + scrapers are built around 40 s and 45 s timeouts with a 7 s page-load allowance, and the photo + scraper's own JS polls up to 58 ticks at 500 ms, so ~30 s is the designed shape, not a stall. + Frame time on the same device is 9.0 ms against a 16.7 ms budget - there is nothing to win there. + A user reporting "the whole app is a bit slow" is far more likely describing this than jank. + - So performance work on Vela should start with: how long until the user sees the thing they asked + for? Search results, ambient POI pins, the gallery, reviews. Not `gfxinfo`, not cold start. + - A concrete lead nobody has pulled: `GoogleMapsDataSource.nearbyPlaces` fans out 15 category + terms 4-at-a-time and finishes with `awaitAll().flatten()`, so **every ambient pin appears at + once after the slowest term**, roughly four network waves in. Streaming each term's results as + they land would put first pins on screen ~4x sooner for the same total work. + - **But it is NOT a drive-by edit, and here is the trap.** `nearbyPlaces` post-processes the whole + fan-out with the SLIM-FLAVOR HEAL: for the first ~3 s of a session Google serves per-place blocks + with the review count ABSENT, which zeroes `ambientProminence` and, in the code's own words, + "silently broke everything keyed on it: prominence ranking, dot sizing, label tiers - all flat". + The heal detects that flavour across the merged pool and refetches. Painting each term as it + lands would put pins on screen BEFORE the heal can run, i.e. exactly the flat-ranking bug that + was already fixed once. There is no `onPartial` on `MapDataSource.nearbyPlaces` today (photos and + reviews have one; ambient does not), so the interface, the ViewModel call site at + MapViewModel.kt:3659, the heal, `rankAmbientPlaces` and the take-N cap all have to be worked out + together. Collision priority is at least already stable across uploads (prominence, not list + index - upstream c35eea33), so repeated uploads will not reshuffle placement. +- **The UI is NOT the bottleneck - measure before optimising it.** An atrace across cold start plus + a D-pad drive on the staging build: `Choreographer#doFrame` totals 3,860 ms over **430 frames = + 9.0 ms per frame**, against a 16.7 ms budget at 60 Hz. measure/layout/draw is 3.5 ms/frame and + inflate is 16 ms in total. **There is no frame-budget problem on this device.** GC, at 2,165 ms, + is the largest remaining cost, which is why allocation work (see the `FlockCameras` index) is the + productive direction and composable restructuring is not. + - This measurement should have come FIRST. Two `MapScreen` extractions were attempted and reverted + before anyone checked whether the uncompiled composable was actually costing frames. It is not. + A method being uncompiled only matters if it runs hot, and at 9 ms/frame this one does not hurt. +- **`MapScreen` is too big for ART to COMPILE, so the main screen runs interpreted.** On the + shipping build ART logs `Method exceeds compiler instruction limit: 19621 in void + i2.r1.f(i2.E3, z3.a, Z.p, int)`, which the R8 mapping resolves to + `MapScreenKt.MapScreen(MapViewModel, Function0, Composer, int)` (`MapScreenKt -> i2.r1`, + `MapScreen -> f`). ART's optimizing compiler skips methods over ~10,000 dex instructions, so the + composable that runs on every recomposition of the main screen is never compiled. Verify with: + + adb logcat -d | grep -i "exceeds compiler instruction limit" + + The body spans roughly lines 192-2172 of a 3,479-line file. It is the largest known code-size + anomaly, but per the frame measurement above it is **not** a demonstrated performance problem - + do not spend effort here without first showing it costs frames. + - The fix is to extract the big `if` blocks under the root `Box` into private composables. A + trial extraction of the largest (lines 1828-2048, the idle-map overlay block, 221 lines) was + done and reverted, and it establishes the recipe: the function needs a **`BoxScope` receiver** + (the block uses `Modifier.align`), and it captures **23** names - `chromeLift, context, + darkTheme, driveFollowing, followMe, layersOpen, metersPerPixel, parkedCarLabel, + parkingClearedMsg, parkingMovedMsg, parkingNoFixMsg, parkingSet, parkingTapAction, resultsShown, + searchOpen, show, showParkingHistory, showParkingMenu, softkeyBarShown, speedOverlayArmed, + state, vm`. + - Six of those are `var ... by remember { mutableStateOf(...) }` (`followMe`, `layersOpen`, + `metersPerPixel`, `showParkingHistory`, `showParkingMenu`, `speedOverlayArmed`) and the block + WRITES them. Pass the `MutableState` and re-delegate at the top of the extracted function + (`var showParkingMenu by showParkingMenuState`) so the 221-line body stays byte-identical. + Passing them by value is a compile error, not a silent break - the compiler is the safety net + here, which is what makes this refactor tractable. + - Do it ONE block at a time, rebuilding and re-checking the logcat number after each, and stop + when the message disappears. Screenshot the map after each step. + - **EXTRACTION DOES NOT WORK. Two controlled experiments, both worse. Do not try a third.** + Extracting a block into a private composable consistently INCREASES MapScreen's instruction + count, and the effect is not explained by how many values the block captures: + + | block extracted | lines | captures | lines/capture | MapScreen after | + |---|---|---|---|---| + | (baseline) | - | - | - | **19,621** | + | 1828-2048 idle overlays | 221 | 23 | 9.6 | 20,351 (+730) | + | 991-1104 dpad overlay | 114 | 9 | 12.7 | **21,638 (+2,017)** | + + The second was chosen precisely because it had a far better lines-per-capture ratio, on the + theory that Compose's per-parameter `$changed` plumbing was the cost. It came out WORSE than the + first. Both compiled, installed and ran without crashing, so this is a code-size result, not a + correctness one; both were reverted. Whatever dominates the count, adding a composable call + layer costs more than the body it removes. **Shrinking MapScreen under the ~10,000 limit is not + reachable by pulling blocks out of it**, and anyone trying should have a different hypothesis + and measure it in one build before doing the work. + - The stale earlier advice below is kept only for the mechanics it records (BoxScope receiver, + passing `MutableState` and re-delegating with `by` so the body stays byte-identical). Those + techniques are correct; the strategy they serve is not. + - **A naive extraction makes it WORSE, and this was measured, not guessed.** The 221-line block + above was fully extracted into `BoxScope.MapIdleOverlays` with all 21 captures passed and the + six `MutableState`s re-delegated. It compiled, ran and did not crash - and MapScreen went + **19,621 -> 20,351 instructions**. Compose emits `$changed`/`$changed1` bitmask plumbing per + parameter at the call site, and for a capture set that size it costs more than the body removes. + The change was reverted. + - So the rule is: **extract blocks with FEW captures, not the biggest blocks.** Before extracting, + count the captures (comment out the block, compile, read the unresolved references - that is + the exact list, and it takes one build). A 100-line block taking 4 parameters will beat a + 220-line block taking 21. Recomputing composable-local values inside the extracted function + (`LocalContext.current`, `stringResource`, `isAppInDarkTheme()`, `VelaSoftkeys.isActive()`) + rather than passing them also drops the count without changing behaviour. +- **Startup is GC-bound, not compilation-bound. Do not reach for a baseline profile.** Forcing full + AOT (`cmd package compile -m speed -f`) made cold start WORSE - 828 ms against 775 ms - which is + an upper bound on anything a profile could buy. An atrace of a cold start instead attributes + seconds to GC (`CopyingPhase`, `NativeAlloc concurrent copying GC`, `MarkingPhase`), so allocation + count at startup is the thing worth cutting. That is what motivated the `FlockCameras` CSR index. +- **Measuring startup: control the dexopt state or measure nothing.** `adb install -r` resets it, so + runs straight after an install are unprofiled and slower. Comparing a fresh-install arm against a + warmed baseline once "showed" that REMOVING work made startup slower. Cold start on this device + swings 739-1552 ms even matched, so prefer a lower-variance metric (atrace GC slices) for anything + smaller than a few hundred ms. +- **`dumpsys gfxinfo` does not measure this app's map.** MapLibre renders through its own GL context, + so HWUI frame stats cover only the Compose chrome - a D-pad drive produced 60 frames at 0% jank + and a swipe drive 11 frames, neither of which could have detected a regression. ## Layout @@ -1297,7 +1726,13 @@ state - upstream's own 13ac02e8 already made the layers panel a VelaMenu): 11 languages; URL = `…/tts-models/vits-piper-.tar.bz2`). `PiperSynth.ensureLoaded` reloads when the selected voice changes; `PiperSynth.reloadVoice()` is the SINGLE switch trigger - it bumps the generation counter (aborting any in-flight utterance) then tears down + rebuilds on the same serial - worker, so `tts` is never freed mid-`generate()`. `MapViewModel.migrateFlatLayoutIfNeeded` (first + worker, so `tts` is never freed mid-`generate()`. The MEMORY-PRESSURE release deliberately does + NOT bump the generation (`release(interrupt = false)`): a trim must reclaim the model, not cut + off the nav prompt being spoken mid-word - the serial worker orders the free after the current + utterance either way. `ensureLoaded` carries the same TWO-STRIKE per-voice crash sentinel as the + ASR loads (`piper_load_strikes_`/`piper_model_bad_`): a voice whose native load + dies (SIGBUS/segfault - uncatchable) crash-looped the app at EVERY launch (issue #95, TCL Flip 2) + because `warmUp()` runs at startup and `catch (Throwable)` never saw the abort. `MapViewModel.migrateFlatLayoutIfNeeded` (first thing in `init`) relocates the old flat single-voice install in place (rename, copy-fallback, verify-gated, re-runnable) - never re-downloads. - **Voice search (speak a query into the search bar), two tiers.** `ui/VoiceSearch` (process-wide @@ -1310,9 +1745,14 @@ state - upstream's own 13ac02e8 already made the layers panel a VelaMenu): D-pad-focusable `Dialog`, Done auto-focuses); wiring + the RECORD_AUDIO launcher + the download-offer are in `MapScreen`; the Settings -> Search per-engine picker is in `SettingsScreen`. Needs `RECORD_AUDIO` (manifest; asked at the mic tap). - - **PICKABLE ENGINES via `voice/AsrEngine` (an enum catalog; ported upstream 5d2a6636 / 118e7e8c / - 137beea9).** Three: `WHISPER_TINY` (multilingual, ~58 MB, the `DEFAULT`), `SENSE_VOICE` - (en/zh/ja/ko/yue, ~154 MB), `MOONSHINE` (English-only, ~101 MB). Each is an OPTIONAL download to + - **PICKABLE ENGINES via `voice/AsrEngine` (an enum catalog; ported upstream 5d2a6636 / + 118e7e8c / 137beea9).** Three: `WHISPER_TINY` (multilingual, ~58 MB, the `DEFAULT`), + `SENSE_VOICE` (en/zh/ja/ko/yue, ~154 MB), `MOONSHINE` (English-only, ~101 MB). Transcripts + pass through `SpeechText.cleanSearchTranscript`, which also lowercases ALL-CAPS output and + runs `spokenNumbersToDigits` - unit-tested inverse text normalization ("ONE TWENTY THREE + MAIN STREET" -> "123 main street") kept as insurance for any future word-form engine; the + current three write digits themselves and pass through unchanged. Each is an OPTIONAL + download to `filesDir/asr//`; `active()` is the picked engine (pref `asr_engine`), `forRecognition(lang)` falls back to Whisper when the pick can't do the app language. **Whisper stays the default and the ONLY thing onboarding / the map mic offer install** (`downloadAsrModel()` = diff --git a/app/build.gradle.kts b/app/build.gradle.kts index d152765d..7b256c64 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -59,6 +59,12 @@ android { // unaffected - it is host-side only. ndk { abiFilters += listOf("arm64-v8a", "armeabi-v7a") } + // libvelamem: the mallopt() purge shim (app/src/main/cpp). Built only for the two ABIs the + // ndk filter above ships. + externalNativeBuild { + cmake { abiFilters += listOf("arm64-v8a", "armeabi-v7a") } + } + // MapTiler key injected from the CI secret (-PmaptilerKey); empty for // local builds, in which case the app falls back to the keyless // OpenFreeMap basemap. Never stored in the repo. @@ -203,6 +209,16 @@ android { } } + // The app's only native code: the mallopt() purge shim. Pinned NDK/CMake versions so a + // developer with a different NDK installed gets the same libvelamem.so as CI. + ndkVersion = "27.0.12077973" + externalNativeBuild { + cmake { + path = file("src/main/cpp/CMakeLists.txt") + version = "3.22.1" + } + } + compileOptions { sourceCompatibility = JavaVersion.VERSION_17 targetCompatibility = JavaVersion.VERSION_17 @@ -260,11 +276,13 @@ dependencies { implementation(project(":core")) implementation(project(":yapchik")) // vendored softkey engine (LGPL-3.0) - keypad/D-pad softkeys - // sherpa-onnx: in-process neural TTS runtime (runs the downloaded Kokoro model). Vendored AAR - // (no official Maven artifact; the JitPack coordinate doesn't resolve). Lives in :app because a - // library module can't consume a local .aar - KokoroSynth sits in :app and bridges into :core's - // VoiceGuide via an interface. Native .so are arm64-only in the package (see packaging{}). - implementation(files("libs/sherpa-onnx-1.13.3.aar")) + // sherpa-onnx: in-process neural TTS + ASR runtime. Vendored AAR (no official Maven artifact; + // the JitPack coordinate doesn't resolve). Lives in :app because a library module can't consume + // a local .aar. 1.13.4 is a LOAD-BEARING upgrade, not routine: its bundled onnxruntime (1.27.0, + // up from 1.24.3) fixes the armv7 unaligned-read SIGBUS that crashed every model LOAD on 32-bit + // ARM phones (issue #95; device-verified both broken-before and fixed-after on an M5 forced to + // `--abi armeabi-v7a`). Do not downgrade past it while the fork ships v7a. + implementation(files("libs/sherpa-onnx-1.13.4.aar")) // Extracts the Kokoro model's .tar.bz2 at download time (Android has no built-in bzip2/tar). implementation("org.apache.commons:commons-compress:1.27.1") diff --git a/app/src/main/cpp/CMakeLists.txt b/app/src/main/cpp/CMakeLists.txt new file mode 100644 index 00000000..5f9808c4 --- /dev/null +++ b/app/src/main/cpp/CMakeLists.txt @@ -0,0 +1,12 @@ +# The app's only native module: a mallopt() purge shim (see velamem.cpp for why). +# Deliberately tiny and STL-free, so it adds a few KB per ABI rather than a runtime. +cmake_minimum_required(VERSION 3.22.1) +project(velamem LANGUAGES CXX) + +add_library(velamem SHARED velamem.cpp) + +# -Os and no exceptions/RTTI: this is three lines of code calling libc, none of which needs them. +target_compile_options(velamem PRIVATE -Os -fno-exceptions -fno-rtti -fvisibility=hidden) + +# Only libc is needed; mallopt lives there. No log lib, the Kotlin side does the logging. +target_link_libraries(velamem) diff --git a/app/src/main/cpp/velamem.cpp b/app/src/main/cpp/velamem.cpp new file mode 100644 index 00000000..6db7428c --- /dev/null +++ b/app/src/main/cpp/velamem.cpp @@ -0,0 +1,38 @@ +// Native-allocator purge. The ONLY reason this module exists. +// +// Issue #83 gave every big holder a release() and fanned OS trims out to them (MemoryPressure), but +// a Kotlin release only hands pages back to the ALLOCATOR, not to the kernel. Scudo keeps them on +// its free lists, so RSS/PSS barely moves and the OOM killer still sees a fat process. +// +// Measured on the M5 (2.9 GB, Android 13, app.vela.debug, PR #85 build, all 8 listeners firing): +// a full TRIM_MEMORY_COMPLETE moved scudo:primary only 56,578 -> 54,978 KB while mallinfo reported +// a 442 MB arena holding just 46 MB live. That gap is what mallopt() reclaims and nothing on the +// Java side can touch. +// +// bionic exposes exactly one lever for it, and only through libc: +// M_PURGE (API 28+) release free memory in the calling thread's arena +// M_PURGE_ALL (API 34+) walk every arena; documented as able to take 2x+ a plain M_PURGE +// +// The values are ABI-stable, so they are spelled out rather than taken from , which +// keeps the build independent of NDK header vintage. An unsupported option makes mallopt() return +// 0, so calling M_PURGE_ALL on an API 33 device is a harmless no-op that falls through to M_PURGE. + +#include +#include + +#ifndef M_PURGE +#define M_PURGE (-101) +#endif +#ifndef M_PURGE_ALL +#define M_PURGE_ALL (-104) +#endif + +// Returns which lever actually took, so the Kotlin side can log it and a device that supports +// neither is visible in logcat instead of silently doing nothing: +// 2 = M_PURGE_ALL, 1 = M_PURGE, 0 = neither supported +extern "C" JNIEXPORT jint JNICALL +Java_app_vela_ui_MemoryPressure_nativePurge(JNIEnv*, jobject, jboolean all) { + if (all && mallopt(M_PURGE_ALL, 0) != 0) return 2; + if (mallopt(M_PURGE, 0) != 0) return 1; + return 0; +} diff --git a/app/src/main/java/app/vela/VelaApp.kt b/app/src/main/java/app/vela/VelaApp.kt index 8b5fc595..db0a2c9c 100644 --- a/app/src/main/java/app/vela/VelaApp.kt +++ b/app/src/main/java/app/vela/VelaApp.kt @@ -39,15 +39,34 @@ class VelaApp : Application(), coil.ImageLoaderFactory { * ~128 MB of decoded gallery bitmaps by design, which is most of the "rapid place churn * runs into the ceiling" OOM (issue #182; measured: 3 gallery-bearing places grew the live * Dalvik heap 14 -> 94 MB). 48 MB still holds a couple of screens of thumbnails + a hero - * or two; everything else re-decodes from Coil's disk cache, which is untouched. */ + * or two; everything else re-decodes from Coil's disk cache, which is untouched. + * + * The cap is now a function of the device instead of one constant: a 48 MB bitmap cache is + * reasonable on a 2-3 GB phone and absurd on a keypad phone whose whole heap class is 96 MB + * (issue #83). Low-RAM devices get 16 MB, which still covers a screen of result thumbnails. + * [MemoryPressure.init] must run before this, and does - onCreate inits it first. */ override fun newImageLoader(): coil.ImageLoader = coil.ImageLoader.Builder(this) .memoryCache { coil.memory.MemoryCache.Builder(this) - .maxSizeBytes(48 * 1024 * 1024) + .maxSizeBytes(if (app.vela.ui.MemoryPressure.lowRam) 16 * 1024 * 1024 else 48 * 1024 * 1024) .build() } .build() + /** + * Hand OS memory pressure to every holder that owns a large or native allocation (issue #83). + * Before this existed nothing in the app implemented ComponentCallbacks2, so a + * TRIM_MEMORY_COMPLETE released nothing at all and the OS had no option but to kill us. + * Coil's own cache is trimmed here; everything else releases through [MemoryPressure]. + */ + override fun onTrimMemory(level: Int) { + super.onTrimMemory(level) + app.vela.ui.MemoryPressure.dispatch(level) + if (app.vela.ui.MemoryPressure.isSevere(level)) { + runCatching { coil.Coil.imageLoader(this).memoryCache?.clear() } + } + } + /** Apply the persisted in-app language to the Application context too (no-op when following the * system), so `getString` from the ViewModel/nav-notification also localizes - resolved at launch * from the saved pref (an in-session change re-reads it on next launch). */ @@ -64,6 +83,11 @@ class VelaApp : Application(), coil.ImageLoaderFactory { Timber.plant(DiagTree(diag)) if (BuildConfig.DEBUG) Timber.plant(Timber.DebugTree()) + // Device memory class first: the Coil cap and the eager-warm decisions below both read it. + app.vela.ui.MemoryPressure.init(this) + // Push the device class down to :core, which cannot read an :app holder (same seam as + // CategoryFilter.enabled). Gates the ambient POI fan-out in GoogleMapsDataSource. + app.vela.core.data.LowRamMode.enabled = app.vela.ui.MemoryPressure.lowRam Units.init(this) AppTheme.init(this) AppLocale.init(this) // resolve the app language (system default) → drives the nav-text locale diff --git a/app/src/main/java/app/vela/data/FlockCameras.kt b/app/src/main/java/app/vela/data/FlockCameras.kt index 1b081343..56f8b021 100644 --- a/app/src/main/java/app/vela/data/FlockCameras.kt +++ b/app/src/main/java/app/vela/data/FlockCameras.kt @@ -34,14 +34,56 @@ object FlockCameras { private const val BUNDLED_VER = "flock_cameras_version.txt" private const val CELL = 0.1 // grid cell size in degrees (~11 km) for the bucket index - @Volatile private var loaded = false - private var lat = DoubleArray(0) - private var lng = DoubleArray(0) - private var op = arrayOf() - private val grid = HashMap>() + /** + * One GENERATION of the dataset: coordinates, operators, and the 0.1 deg bucket index as a flat + * CSR (compressed sparse row) triple rather than a `HashMap>`. + * + * [cellKeys] is sorted and unique; the rows in cell `k` are + * `cellRows[cellStart[k] until cellStart[k + 1]]`. Lookup is a binary search on [cellKeys]. + * + * Why CSR: the map version cost roughly 166,000 objects to build - one boxed `Integer` per camera + * (124,406 of them), plus an `ArrayList` + its `Object[]` + a boxed `Long` key + a `HashMap.Node` + * per occupied cell (13,965 of those) - and every one of them was garbage the moment the parse + * finished. This is five arrays. Startup on this app is GC-bound, not compilation-bound (forcing + * full AOT made cold start WORSE, 828 ms against 775 ms), so allocation count at startup is the + * thing worth cutting. Lookups are unchanged in behaviour and no slower in practice: the viewport + * scan touches on the order of 100 cells, and a binary search over ~14,000 keys is ~14 compares. + * + * Why one class and not six fields on the object: the six arrays are one INVARIANT - a lookup's + * key array and offset array must come from the same parse. `refresh()`'s hot-swap runs on + * Dispatchers.IO while a viewport scan may be walking the index, and separately-published fields + * can tear (new, larger `cellKeys` against old `cellStart` = `cellStart[k + 1]` past the end - + * an ArrayIndexOutOfBoundsException out of `inBox`, which nothing catches). A single @Volatile + * reference snapshotted once per query cannot: a reader sees the whole old generation or the + * whole new one. + */ + private class Data( + val lat: DoubleArray, + val lng: DoubleArray, + val op: Array, + val cellKeys: LongArray, + val cellStart: IntArray, + val cellRows: IntArray, + ) { + /** + * Run [body] for every camera row in the cell at ([row], [col]). No-op when the cell is + * empty. Replaces `grid[key(r, c)]?.let { ... }`, and allocates nothing - notably no + * iterator, which the old `for (i in bucket)` over a `MutableList` created (and + * unboxed on every step). + */ + inline fun forEachInCell(row: Long, col: Long, body: (Int) -> Unit) { + val k = cellKeys.binarySearch(key(row, col)) + if (k < 0) return + var p = cellStart[k] + val end = cellStart[k + 1] + while (p < end) { body(cellRows[p]); p++ } + } + } + + @Volatile private var data: Data? = null - val isLoaded: Boolean get() = loaded - val size: Int get() = lat.size + val isLoaded: Boolean get() = data != null + val size: Int get() = data?.lat?.size ?: 0 private fun key(row: Long, col: Long): Long = (row shl 32) xor (col and 0xffffffffL) private fun rowOf(v: Double): Long = Math.floor(v / CELL).toLong() @@ -60,9 +102,9 @@ object FlockCameras { /** Parse the newest available file once, off the main thread. Safe to call repeatedly (a loaded call no-ops). */ suspend fun ensureLoaded(context: Context) { - if (loaded) return + if (data != null) return withContext(Dispatchers.IO) { - if (loaded) return@withContext + if (data != null) return@withContext val dl = downloadedBin(context) val stream = if (dl.exists()) runCatching { dl.inputStream() }.getOrNull() else runCatching { context.assets.open(BUNDLED) }.getOrNull() @@ -72,9 +114,13 @@ object FlockCameras { /** Build the arrays + index from a gzipped-TSV stream and publish them (never leaves `loaded` false once set). */ private fun loadFrom(raw: InputStream) { - val las = ArrayList(130_000) - val los = ArrayList(130_000) - val ops = ArrayList(130_000) + // Primitive growable arrays, not ArrayList: the list version boxed a java.lang.Double + // per coordinate, 248,812 of them for the bundled 124,406-row dataset, all garbage the moment + // toDoubleArray() copied them out. + var las = DoubleArray(130_000) + var los = DoubleArray(130_000) + var ops = arrayOfNulls(130_000) + var n = 0 val intern = HashMap() // operator column is highly repetitive - intern it raw.use { r -> GZIPInputStream(r).bufferedReader().useLines { lines -> @@ -84,18 +130,49 @@ object FlockCameras { val la = line.substring(0, t1).toDoubleOrNull() ?: continue val lo = line.substring(t1 + 1, t2).toDoubleOrNull() ?: continue val o = line.substring(t2 + 1) - las.add(la); los.add(lo); ops.add(intern.getOrPut(o) { o }) + if (n == las.size) { // dataset outgrew the guess - double, same as ArrayList did + las = las.copyOf(n * 2); los = los.copyOf(n * 2); ops = ops.copyOf(n * 2) + } + las[n] = la; los[n] = lo; ops[n] = intern.getOrPut(o) { o } + n++ } } } - val g = HashMap>() - for (i in las.indices) g.getOrPut(key(rowOf(las[i]), rowOf(los[i]))) { ArrayList() }.add(i) - // Publish (a bad/partial parse threw before here, so we never swap in a half-built set). - lat = las.toDoubleArray(); lng = los.toDoubleArray(); op = ops.toTypedArray() - grid.clear(); grid.putAll(g) - loaded = true + + // CSR index, built with sorts and counting rather than a map of lists. Three passes over + // primitives and no per-row object at all; see [cellKeys]. + val keys = LongArray(n) { key(rowOf(las[it]), rowOf(los[it])) } + val sorted = keys.copyOf() + sorted.sort() + var uniq = 0 + for (i in 0 until n) if (i == 0 || sorted[i] != sorted[i - 1]) uniq++ + val ck = LongArray(uniq) + var u = 0 + for (i in 0 until n) if (i == 0 || sorted[i] != sorted[i - 1]) { ck[u] = sorted[i]; u++ } + // Count per cell, then prefix-sum into start offsets. + val start = IntArray(uniq + 1) + for (i in 0 until n) start[ck.binarySearch(keys[i]) + 1]++ + for (k in 1..uniq) start[k] += start[k - 1] + // Scatter row indices into their cell's slot. `fill` walks a copy of the offsets so + // `start` stays the published boundary array. + val fill = start.copyOf() + val rows = IntArray(n) + for (i in 0 until n) { val k = ck.binarySearch(keys[i]); rows[fill[k]] = i; fill[k]++ } + + // Publish the whole generation in ONE volatile write (a bad/partial parse threw before + // here, so we never swap in a half-built set - and a concurrent reader holding the old + // generation's snapshot keeps using it, consistently, until its query ends). + @Suppress("UNCHECKED_CAST") + data = Data(las.copyOf(n), los.copyOf(n), ops.copyOf(n) as Array, ck, start, rows) } + /** Test seam: build a generation from an in-memory stream (unit tests have no Context/assets). */ + @androidx.annotation.VisibleForTesting + internal fun loadFromForTest(raw: InputStream) = loadFrom(raw) + + @androidx.annotation.VisibleForTesting + internal fun resetForTest() { data = null } + private val downloadHttp: OkHttpClient by lazy { OkHttpClient.Builder().callTimeout(0, TimeUnit.SECONDS).readTimeout(60, TimeUnit.SECONDS).build() } @@ -132,7 +209,7 @@ object FlockCameras { /** Cameras inside the bbox, for DRAWING. Empty if not loaded yet (caller falls back to Overpass). */ fun inBox(south: Double, west: Double, north: Double, east: Double): List { - if (!loaded) return emptyList() + val d = data ?: return emptyList() // ONE snapshot per query - see [Data] val out = ArrayList() val r0 = rowOf(south); val r1 = rowOf(north) val c0 = rowOf(west); val c1 = rowOf(east) @@ -140,9 +217,9 @@ object FlockCameras { while (r <= r1) { var c = c0 while (c <= c1) { - grid[key(r, c)]?.let { bucket -> - for (i in bucket) { - if (lat[i] in south..north && lng[i] in west..east) out.add(AlprCamera(LatLng(lat[i], lng[i]), op[i])) + d.forEachInCell(r, c) { i -> + if (d.lat[i] in south..north && d.lng[i] in west..east) { + out.add(AlprCamera(LatLng(d.lat[i], d.lng[i]), d.op[i])) } } c++ @@ -154,7 +231,8 @@ object FlockCameras { /** Cameras within [meters] of any SEGMENT of [polyline], for the route count. Empty if not loaded. */ fun along(polyline: List, meters: Double = 120.0): List { - if (!loaded || polyline.size < 2) return emptyList() + val d = data ?: return emptyList() // ONE snapshot per query - see [Data] + if (polyline.size < 2) return emptyList() val pad = 0.01 val r0 = rowOf(polyline.minOf { it.lat } - pad); val r1 = rowOf(polyline.maxOf { it.lat } + pad) val c0 = rowOf(polyline.minOf { it.lng } - pad); val c1 = rowOf(polyline.maxOf { it.lng } + pad) @@ -163,11 +241,9 @@ object FlockCameras { while (r <= r1) { var c = c0 while (c <= c1) { - grid[key(r, c)]?.let { bucket -> - for (i in bucket) { - val p = LatLng(lat[i], lng[i]) - if (nearPolyline(p, polyline, meters)) out.add(AlprCamera(p, op[i])) - } + d.forEachInCell(r, c) { i -> + val p = LatLng(d.lat[i], d.lng[i]) + if (nearPolyline(p, polyline, meters)) out.add(AlprCamera(p, d.op[i])) } c++ } diff --git a/app/src/main/java/app/vela/ui/MemoryPressure.kt b/app/src/main/java/app/vela/ui/MemoryPressure.kt new file mode 100644 index 00000000..88f41751 --- /dev/null +++ b/app/src/main/java/app/vela/ui/MemoryPressure.kt @@ -0,0 +1,233 @@ +package app.vela.ui + +import android.app.ActivityManager +import android.content.ComponentCallbacks2 +import android.content.Context +import java.util.concurrent.CopyOnWriteArrayList +import timber.log.Timber + +/** + * Process-wide memory-pressure fan-out, in the same shape as the other app-level holders + * (`TransitLayer`, `AppTheme`): `init()` from `VelaApp`, then anything holding a large or native + * allocation registers a release callback. + * + * Why registration and not a Hilt entry point: reaching `WhisperRecognizer`/`PiperSynth` from + * `onTrimMemory` through an EntryPoint would CONSTRUCT them if they had never been used, so a + * trim would allocate the very models it is trying to free. A holder registers only once it + * actually owns something worth releasing, so a trim can never create work. + * + * Measured on the M5 (2.9 GB, Android 13, standardDebug) before this existed: TRIM_MEMORY_COMPLETE + * released 0 KB, because nothing in the app implemented ComponentCallbacks2 at all. + */ +object MemoryPressure { + + /** A registered releaser. [level] is a `ComponentCallbacks2.TRIM_MEMORY_*` constant. */ + fun interface Listener { + fun release(level: Int) + } + + private val listeners = CopyOnWriteArrayList() + + /** + * True when this device cannot comfortably carry our normal budgets. Three independent signals, + * any of which is enough, because no single one catches the phones this work is for: + * + * - `ActivityManager.isLowRamDevice`, the canonical flag, but only Go-configured builds set it. + * - Total system RAM ([totalRamMb]) at or under `LowRamMode.LOW_TOTAL_RAM_MB`. This is the + * signal that actually describes the device; heap class is a Dalvik tuning knob an OEM can set + * to anything. + * - Heap class ([heapClassMb]) at or under `LowRamMode.LOW_HEAP_CLASS_MB`, which catches an OEM + * that ships plenty of RAM but hands apps a small heap. + * + * The predicate itself is `LowRamMode.classify`, in `:core` so it can be unit-tested. + * + * **When we cannot tell, we assume constrained.** Failing to the low-RAM path costs a roomy + * phone about a second on its first mic tap and its first place open; failing the other way can + * OOM a phone that had no headroom. The old predicate did the opposite: it read + * `heapClassMb in 1..127`, so a failed `ActivityManager` lookup produced 0, fell out of the + * range, and silently selected the memory-hungry path on a device we knew nothing about. + */ + @Volatile var lowRam: Boolean = false + private set + + /** The device's normal (non-large) heap class in MB. 0 when it could not be read. */ + @Volatile var heapClassMb: Int = 0 + private set + + /** Total system RAM in MB, as the OS reports it. 0 when it could not be read. */ + @Volatile var totalRamMb: Int = 0 + private set + + fun init(context: Context) { + val am = context.getSystemService(Context.ACTIVITY_SERVICE) as? ActivityManager + heapClassMb = am?.memoryClass ?: 0 + totalRamMb = am?.let { m -> + runCatching { + val mi = ActivityManager.MemoryInfo() + m.getMemoryInfo(mi) + (mi.totalMem / (1024L * 1024L)).toInt() + }.getOrDefault(0) + } ?: 0 + val forced = forcedLowRam() + // The decision itself lives in :core so it can be unit-tested; this side only probes. + // am == null means we could not ask at all, which the 0/0 probes already classify as + // constrained, but say it explicitly rather than leaning on that coincidence. + lowRam = forced ?: ( + am == null || + app.vela.core.data.LowRamMode.classify(am.isLowRamDevice, heapClassMb, totalRamMb) + ) + Timber.i( + "MemoryPressure init lowRam=%b heapClassMb=%d totalRamMb=%d forced=%s", + lowRam, heapClassMb, totalRamMb, forced?.toString() ?: "no", + ) + } + + /** + * Debug-only override so the low-RAM path can be exercised on a normal dev phone: + * + * adb shell setprop debug.vela.lowram true # force the low-RAM path, then relaunch + * adb shell setprop debug.vela.lowram false # force the normal path, then relaunch + * adb shell setprop debug.vela.lowram none # clear the override, back to real detection + * + * All three need a relaunch; this is read once from [init]. Note that `false` FORCES the normal + * path rather than clearing the override - the two only look alike because every dev phone we + * own detects as normal anyway. Clearing needs an unparseable value ([debugFlag] returns null + * for anything that is not true/1/false/0), hence `none`; `setprop ""` is a shell syntax + * error, not a reset. + * + * Without this the low-RAM branches are dead code on every device we actually own (the M5 dev + * phone reports heapClassMb=256, lowRam=false), which means they would ship unverified. + */ + private fun forcedLowRam(): Boolean? = debugFlag("debug.vela.lowram") + + /** + * Read a debug-only tri-state system property. Returns null when unset, unparseable, or on a + * non-debug build, so release behaviour is never affected by one of these. + * + * NB clearing one is `setprop false`, NOT `setprop ""` - an empty value is a + * syntax error at the shell, not a reset. + */ + private fun debugFlag(name: String): Boolean? { + if (!app.vela.BuildConfig.DEBUG) return null + val v = runCatching { + @Suppress("PrivateApi") + val sp = Class.forName("android.os.SystemProperties") + sp.getMethod("get", String::class.java).invoke(null, name) as? String + }.getOrNull() + return when (v?.lowercase()) { + "true", "1" -> true + "false", "0" -> false + else -> null + } + } + + /** Register [listener]; returns a handle whose `close()` unregisters. Safe to call any time. */ + fun register(listener: Listener): AutoCloseable { + listeners.add(listener) + return AutoCloseable { listeners.remove(listener) } + } + + /** + * Fan a trim out to every registered holder. Each listener is isolated: one throwing must not + * stop the rest from releasing, since under real pressure we want every byte we can get. + * + * The fan-out alone is only half the job: a listener's `release()` returns pages to SCUDO, not + * to the kernel, so RSS barely moves. [schedulePurge] finishes it. See [nativePurge]. + */ + fun dispatch(level: Int) { + Timber.i("MemoryPressure dispatch level=%d listeners=%d", level, listeners.size) + for (l in listeners) { + runCatching { l.release(level) } + .onFailure { Timber.w(it, "MemoryPressure listener failed") } + } + // Anything except the gentlest level is worth a purge. Deliberately WIDER than isSevere: + // measured on the M5, backgrounding the app delivers only TRIM_MEMORY_UI_HIDDEN (20), never + // TRIM_MEMORY_BACKGROUND (40), so gating the purge on isSevere would skip the single most + // common moment we are handed - the app is off-screen, nothing can jank, and the allocator + // is holding pages nobody will touch again for minutes. + if (level >= ComponentCallbacks2.TRIM_MEMORY_RUNNING_LOW) schedulePurge(isSevere(level)) + } + + // ---------------------------------------------------------------- native allocator purge + + /** + * Scudo hands freed pages back only when asked, and `mallopt` is the only way to ask. Measured + * on the M5 (Android 13, app.vela.debug, all 8 listeners releasing): a full TRIM_MEMORY_COMPLETE + * moved `scudo:primary` just 56,578 -> 54,978 KB while mallinfo showed a 442 MB arena holding + * 46 MB live. Everything in that gap is reclaimable and unreachable from Kotlin. + * + * Returns which lever took: 2 = M_PURGE_ALL, 1 = M_PURGE, 0 = neither (pre-API-28). + */ + private external fun nativePurge(all: Boolean): Int + + /** False when libvelamem is missing (an ABI we do not ship, a stripped install). The purge is + * then skipped rather than taking the process down over an optimization. */ + private val nativeReady: Boolean = + runCatching { System.loadLibrary("velamem") } + .onFailure { Timber.w(it, "libvelamem unavailable, native purge disabled") } + .isSuccess + + /** One daemon thread, created lazily. Never the main thread: a purge walks the allocator's free + * lists behind its global lock, and that is not something to do on the UI thread. */ + private val purgeExec by lazy { + java.util.concurrent.Executors.newSingleThreadScheduledExecutor { r -> + Thread(r, "mem-purge").apply { isDaemon = true } + } + } + + /** Coalesces a burst of trims into one purge. The OS routinely sends several levels in a row. */ + private val purgePending = java.util.concurrent.atomic.AtomicBoolean(false) + + /** + * Purge after [PURGE_DELAY_MS], not immediately: the WebView reapers post their `destroy()` to + * the main looper and `VelaApp` clears Coil right after [dispatch] returns, so an inline purge + * would run BEFORE the memory it is meant to reclaim has actually been freed and reclaim close + * to nothing. + * + * [all] picks the lever. M_PURGE_ALL walks every arena and is documented as able to take over + * twice as long as a plain M_PURGE, so it is spent only on levels [isSevere] already treats as + * "drop it"; a routine UI_HIDDEN gets the cheap one. + */ + private fun schedulePurge(all: Boolean) { + if (!nativeReady) return + // Debug-only kill switch, so the purge's contribution can be A/B measured on ONE binary: + // adb shell setprop debug.vela.nopurge true # then relaunch, this is read per-trim + // Without it the only way to attribute a delta is to compare two different builds, which + // also differ in background settling and cannot be paired inside a single run. + if (debugFlag("debug.vela.nopurge") == true) { + Timber.i("MemoryPressure native purge suppressed by debug.vela.nopurge") + return + } + if (!purgePending.compareAndSet(false, true)) return + val scheduled = runCatching { + purgeExec.schedule({ + purgePending.set(false) + val t0 = android.os.SystemClock.uptimeMillis() + val mode = runCatching { nativePurge(all) }.getOrDefault(0) + Timber.i( + "MemoryPressure native purge all=%b mode=%d took=%dms", + all, mode, android.os.SystemClock.uptimeMillis() - t0, + ) + }, PURGE_DELAY_MS, java.util.concurrent.TimeUnit.MILLISECONDS) + }.getOrNull() + if (scheduled == null) purgePending.set(false) // executor rejected; let the next trim retry + } + + /** Long enough for the main-looper-posted WebView destroys and the Coil clear to have landed. */ + private const val PURGE_DELAY_MS = 750L + + + /** + * The app is backgrounded or the OS is genuinely short of memory, so caches that only speed + * things up should go. Everything at or above this level is a "drop it" signal. + */ + fun isSevere(level: Int): Boolean = + level >= ComponentCallbacks2.TRIM_MEMORY_BACKGROUND || + level == ComponentCallbacks2.TRIM_MEMORY_RUNNING_CRITICAL || + level == ComponentCallbacks2.TRIM_MEMORY_RUNNING_LOW + + /** Only the harshest levels, where we drop things that cost real time to rebuild. */ + fun isCritical(level: Int): Boolean = + level >= ComponentCallbacks2.TRIM_MEMORY_COMPLETE || + level == ComponentCallbacks2.TRIM_MEMORY_RUNNING_CRITICAL +} diff --git a/app/src/main/java/app/vela/ui/map/MapViewModel.kt b/app/src/main/java/app/vela/ui/map/MapViewModel.kt index f7d1d243..affecdf5 100644 --- a/app/src/main/java/app/vela/ui/map/MapViewModel.kt +++ b/app/src/main/java/app/vela/ui/map/MapViewModel.kt @@ -1233,8 +1233,14 @@ class MapViewModel @Inject constructor( // popular times AND the photo gallery land faster when the user taps a result // (both idempotent; the photo warm primes the renderer + HTTP/2 sockets + cache // so the first place page skips the cold start). - viewModelScope.launch { runCatching { webPopularTimes.prewarm() } } - runCatching { webPhotos.warm() } + // Skipped on low-RAM devices: each warm spins up a Chromium renderer SPECULATIVELY, on the + // guess that a search predicts a place tap. When memory is the scarce resource that trade is + // backwards - the user pays two renderers on every search whether or not they open anything + // (issue #83). Those phones build the WebView on first real use instead. + if (!app.vela.ui.MemoryPressure.lowRam) { + viewModelScope.launch { runCatching { webPopularTimes.prewarm() } } + runCatching { webPhotos.warm() } + } searchJob?.cancel() searchJob = viewModelScope.launch { // A fresh typed search leaves any along-route browse: picks open places normally again. @@ -3466,6 +3472,7 @@ class MapViewModel @Inject constructor( ) } if (ok && VelaPiper.isVoiceReady(appContext, id)) { + piperSynth.clearQuarantine(id) // a fresh download replaces whatever was quarantined if (firstEver) selectVoice(id) else flashStatus(appContext.getString(R.string.mapvm_voice_downloaded, v.displayName)) } else { showStatus(appContext.getString(R.string.mapvm_voice_download_failed, v.displayName)) @@ -4468,8 +4475,20 @@ class MapViewModel @Inject constructor( /** Remove one engine's model (Settings "Remove"); the active pick degrades to another installed * engine automatically (AsrEngine.active). */ fun deleteAsrEngine(engine: app.vela.voice.AsrEngine) { - engine.dir(appContext).deleteRecursively() - refreshAsr() + // Drop it from the UI immediately (optimistic, same idiom as deleteVoice); the real work is + // OFF the main thread: release(wait = true) can park behind a multi-second in-flight native + // load on loadLock, then frees ~267 MB of native memory, and the recursive delete unlinks up + // to 154 MB of files - all three are ANR material on a slow keypad phone's UI thread. + _state.update { it.copy(asrInstalledIds = it.asrInstalledIds - engine.id) } + viewModelScope.launch(kotlinx.coroutines.Dispatchers.IO) { + // Free the loaded model BEFORE removing its files. Deleting the directory alone left the + // native recognizer resident for the rest of the process (~267 MB measured, issue #83), + // so "Remove" reclaimed disk but no memory at all. Released unconditionally: the loaded + // engine may not be [engine], but the worst case is a ~1 s reload on the next listen. + whisperRecognizer.release(wait = true) // deliberate user action: worth waiting out a load + engine.dir(appContext).deleteRecursively() + refreshAsr() + } } /** Onboarding offers BOTH on-device speech models on one screen, so a user can pick both at once. diff --git a/app/src/main/java/app/vela/ui/map/VelaMapView.kt b/app/src/main/java/app/vela/ui/map/VelaMapView.kt index f7ed559d..e93dabe0 100644 --- a/app/src/main/java/app/vela/ui/map/VelaMapView.kt +++ b/app/src/main/java/app/vela/ui/map/VelaMapView.kt @@ -1084,7 +1084,17 @@ fun VelaMapView( } } lifecycleOwner.lifecycle.addObserver(observer) + // MapLibre keeps its tile, glyph and sprite caches in NATIVE memory, and the only way to ask + // it to shrink them is onLowMemory(). Nothing called it before (issue #83), so the map held + // its full cache through every trim the OS sent. Registered with the map's own lifecycle so + // the listener can never outlive the MapView it points at. + val trim = app.vela.ui.MemoryPressure.register { level -> + if (app.vela.ui.MemoryPressure.isSevere(level)) { + runCatching { mapView.onLowMemory() } + } + } onDispose { + trim.close() lifecycleOwner.lifecycle.removeObserver(observer) mapView.onPause() mapView.onStop() diff --git a/app/src/main/java/app/vela/voice/AsrEngine.kt b/app/src/main/java/app/vela/voice/AsrEngine.kt index 77f301ea..d1afff44 100644 --- a/app/src/main/java/app/vela/voice/AsrEngine.kt +++ b/app/src/main/java/app/vela/voice/AsrEngine.kt @@ -20,12 +20,20 @@ private const val VAD_FILE = "silero_vad.onnx" * Three engines, because they trade off differently and the user picks (ported from upstream * PimpinPumpkin/Vela 5d2a6636 + 118e7e8c sizes + 137beea9 language fallback): * - [WHISPER_TINY] - the multilingual default. 99-language Whisper tiny (int8); covers every - * language Vela's UI supports (incl. Hebrew, Russian, Spanish). The safe all-rounder, and the - * smallest - so it stays the default and the ONLY thing the one-tap onboarding/map offer installs, - * which matters on the RAM/storage-constrained feature phones this fork targets. + * language Vela's UI supports (incl. Hebrew, Russian, Spanish). The safe all-rounder and the + * smallest download - so it stays the default and the ONLY thing the one-tap onboarding/map + * offer installs. NOT the smallest loaded: ~214 MB PSS resident (measured, 32-bit M5) - but + * the idle reaper makes that transient, not session-long. * - [SENSE_VOICE] - FunAudioLLM SenseVoice. More accurate + faster than Whisper tiny, but only for * English, Chinese, Cantonese, Japanese, Korean. Bigger (opt-in). - * - [MOONSHINE] - Useful Sensors Moonshine tiny. Lowest latency, ENGLISH ONLY. Bigger (opt-in). + * - [MOONSHINE] - Useful Sensors Moonshine tiny. Lowest latency, ENGLISH ONLY. Bigger (opt-in), + * and despite the small weights its four ORT sessions cost ~212 MB PSS loaded - no lighter + * resident than Whisper (measured, 32-bit M5). + * + * Before adding a "low-memory" fourth engine, read the measurement notes in AGENTS.md: NeMo + * Conformer CTC small (46 MB file) ballooned to ~760 MB-1.2 GB resident through onnxruntime, and + * k2 Zipformer small was built, measured and then REMOVED - librispeech-domain models mishear the + * proper nouns a maps app lives on, and no amount of post-processing fixes that. * * Whisper stays the default so no language silently regresses; the other two are opt-in via the * voice-search engine picker in Settings. This holds only metadata + a cheap install check + the diff --git a/app/src/main/java/app/vela/voice/PiperSynth.kt b/app/src/main/java/app/vela/voice/PiperSynth.kt index 665bc37a..fa1dcf36 100644 --- a/app/src/main/java/app/vela/voice/PiperSynth.kt +++ b/app/src/main/java/app/vela/voice/PiperSynth.kt @@ -38,6 +38,23 @@ class PiperSynth @Inject constructor( @Volatile private var loadFailed = false @Volatile private var generation = 0 + init { + // The Piper VITS model is the app's second-largest native holding after the ASR model. + // CRITICAL only, deliberately narrower than the recognizer's severe trigger: dropping the + // synth costs a reload on the next prompt, and a prompt arriving late during navigation is + // a missed turn. TRIM_MEMORY_COMPLETE only reaches background processes, and + // RUNNING_CRITICAL means the device is about to start killing things regardless. + // The teardown posts to the piper-tts worker, so it is already serialized against an + // in-flight synthesis and cannot free the model out from under one (issue #83). + // interrupt = false: a trim must reclaim memory, not SILENCE the prompt being spoken - + // release()'s default generation bump aborts an in-flight utterance within ~200 ms, which + // during navigation is a missed turn (the very regression scoping to CRITICAL was meant to + // avoid). Un-bumped, the serial worker frees the model right AFTER the current utterance. + app.vela.ui.MemoryPressure.register { level -> + if (app.vela.ui.MemoryPressure.isCritical(level)) release(interrupt = false) + } + } + /** Which voice id `tts` currently holds - lets [ensureLoaded] detect a voice switch and rebuild. */ @Volatile private var loadedVoiceId: String? = null @@ -79,6 +96,16 @@ class PiperSynth @Inject constructor( worker.execute { ensureLoaded() } } + private fun prefs() = context.getSharedPreferences("vela_settings", Context.MODE_PRIVATE) + + /** Lift a voice's crash quarantine after a fresh download - the bad files are gone, so the next + * load may try again. Called by the installer path, never automatically. */ + fun clearQuarantine(voiceId: String) { + prefs().edit() + .putBoolean(KEY_MODEL_BAD + voiceId, false) + .putInt(KEY_LOAD_STRIKES + voiceId, 0).apply() + } + private fun ensureLoaded(): OfflineTts? { val r = VelaPiper.resolved(context) ?: return null // nothing usable installed val cur = tts @@ -88,10 +115,32 @@ class PiperSynth @Inject constructor( // use-after-free). loadFailed resets so a previously-bad voice doesn't block a new one. runCatching { cur?.release() } tts = null; loadedVoiceId = null; numSpeakers = 0; loadFailed = false + // CRASH SENTINEL around the native load, PER VOICE - the same two-strike idiom as + // WhisperRecognizer's ASR loads, closing the same hole for TTS (issue #95: a voice whose + // load dies natively - SIGBUS/segfault, which no `catch (Throwable)` can see - crash-looped + // the app at EVERY launch, because warmUp() runs at startup and nothing remembered the + // previous attempt never returned). Bump a strike before the load, zero it once it returns; + // two stranded loads in a row quarantine THAT voice only and delete its dir, so the app + // boots (system TTS takes over) and a fresh download starts clean. Two, not one: a process + // killed mid-load (swipe-away, memory reclaim) strands a strike exactly like a crash, and + // must not delete a healthy 80 MB voice. + val prefs = prefs() + val strikesKey = KEY_LOAD_STRIKES + r.voiceId + val badKey = KEY_MODEL_BAD + r.voiceId + val strikes = prefs.getInt(strikesKey, 0) + if (strikes >= 2) { + Timber.tag(TAG).e("two voice loads never returned (native crash) - quarantining ${r.voiceId}") + prefs.edit().putInt(strikesKey, 0).putBoolean(badKey, true).apply() + runCatching { VelaPiper.modelDirFor(context, r.voiceId).deleteRecursively() } + loadFailed = true + return null + } + if (prefs.getBoolean(badKey, false)) { loadFailed = true; return null } // Two attempts: a voice loaded the instant its download/extract finishes can lose the race with // the filesystem flush on some devices - the first OfflineTts load throws, and (without a retry) // loadFailed sticks so the voice stays SILENT until an app restart. A brief retry heals it. repeat(2) { attempt -> + prefs.edit().putInt(strikesKey, prefs.getInt(strikesKey, 0) + 1).apply() try { // Lower the VITS noise scales below the library defaults (noiseScale 0.667, noiseScaleW 0.8). // Those defaults make synthesis STOCHASTIC - the same phrase varies run to run, which is why @@ -110,11 +159,15 @@ class PiperSynth @Inject constructor( val engine = OfflineTts(assetManager = null, config = cfg) numSpeakers = engine.numSpeakers() runCatching { engine.generate(text = " ", sid = 0, speed = SPEED) } + // The load (and warm synth) RETURNED - the process survived it, so zero the strikes. + // Only a native abort mid-load leaves one standing. + prefs.edit().putInt(strikesKey, 0).apply() tts = engine loadedVoiceId = r.voiceId Timber.tag(TAG).i("loaded ${r.voiceId}: sampleRate=${engine.sampleRate()} speakers=$numSpeakers") return engine } catch (t: Throwable) { + prefs.edit().putInt(strikesKey, 0).apply() // a CATCHABLE failure is not a native crash Timber.tag(TAG).e(t, "model load failed (attempt ${attempt + 1}): ${t.message}") if (attempt == 0) runCatching { Thread.sleep(200) } // let a just-written model settle, then retry } @@ -283,8 +336,13 @@ class PiperSynth @Inject constructor( worker.execute { runCatching { track?.pause(); track?.flush() } } } - override fun release() { - generation++ + override fun release() = release(interrupt = true) + + /** Free the engine + track. [interrupt] aborts any in-flight utterance first (the right thing + * when the voice is being switched off or replaced); the memory-pressure path passes false so + * the current prompt finishes - the serial worker orders the free after it either way. */ + fun release(interrupt: Boolean) { + if (interrupt) generation++ worker.execute { runCatching { track?.release() }; track = null runCatching { tts?.release() }; tts = null @@ -294,6 +352,10 @@ class PiperSynth @Inject constructor( private companion object { const val TAG = "PiperSynth" const val SPEED = 1.0f + // Crash-sentinel keys, PER VOICE (suffixed with the voice id) - same idiom and reasoning as + // WhisperRecognizer's KEY_LOAD_STRIKES/KEY_MODEL_BAD, see the sentinel in [ensureLoaded]. + const val KEY_LOAD_STRIKES = "piper_load_strikes_" + const val KEY_MODEL_BAD = "piper_model_bad_" // Silence spliced between sentences (seconds) - a natural period beat for nav prompts. const val PAUSE_SEC = 0.32f // Shorter beat spliced at commas/semicolons so clauses don't run together ("In a quarter mile, …"). diff --git a/app/src/main/java/app/vela/voice/WhisperRecognizer.kt b/app/src/main/java/app/vela/voice/WhisperRecognizer.kt index 29722b09..1ac5440f 100644 --- a/app/src/main/java/app/vela/voice/WhisperRecognizer.kt +++ b/app/src/main/java/app/vela/voice/WhisperRecognizer.kt @@ -36,18 +36,166 @@ import kotlin.math.sqrt * the end of speech, and returns the transcript. Nothing leaves the phone and no third-party voice * app is needed (that's tier-2 - the RECOGNIZE_SPEECH intent handoff in MapScreen). * - * The Whisper recognizer loads lazily and is kept for the process lifetime (~1 s to load); the VAD is - * created per listen (it's tiny and holds streaming state). R8 must keep `com.k2fsa.sherpa.onnx.**` - * (JNI resolves classes by name) - already in `consumer-rules`/`proguard` for Piper. + * The Whisper recognizer loads lazily (~1 s) and is NO LONGER kept for the process lifetime: it costs + * ~267 MB PSS, so it is dropped after [REAP_IDLE_MS] of quiet and on any severe memory trim, and + * rebuilt on the next use (issue #83). The VAD is created per listen (it's tiny and holds streaming + * state). R8 must keep `com.k2fsa.sherpa.onnx.**` (JNI resolves classes by name) - already in + * `consumer-rules`/`proguard` for Piper. */ @Singleton class WhisperRecognizer @Inject constructor( @ApplicationContext private val context: Context, ) { - private val loadLock = Any() + /** Guards [recognizer]/[loadedKey] AND [leases]. A ReentrantLock rather than `synchronized` + * so [release] can `tryLock` instead of blocking - see there. */ + private val loadLock = java.util.concurrent.locks.ReentrantLock() @Volatile private var recognizer: OfflineRecognizer? = null @Volatile private var loadedKey: String? = null + /** + * Outstanding leases on the loaded recognizer: non-zero while a [listen] holds a pointer to it. + * [release] refuses to free the model while this is set, because `OfflineRecognizer.release()` + * frees C++ memory an in-flight decode is still reading - a use-after-free that takes the + * process down rather than throwing, so `runCatching` around the decode cannot save it. + * + * **Only ever mutated while holding [loadLock]**, in [acquireRecognizer]/[releaseLease]. That is + * the whole point. It used to be incremented in [listen] with no lock while [release] read it + * with no lock, which is a check-then-act with a real window: the reaper could evaluate the + * count as 0, a mic tap could then increment it and take the pointer off `ensureRecognizer`'s + * lock-free fast path, and the reaper would go on to free the model under the running decode. + * The window was not small either - any thread holding [loadLock] for a ~1 s model load parks + * `release()` between its check and the free for that whole time. + */ + private val leases = java.util.concurrent.atomic.AtomicInteger(0) + + /** Recognizers superseded by an engine/language switch WHILE a lease was outstanding. The + * switch path must not free the old engine then - the lease-holding decode is still inside + * it, and `OfflineRecognizer.release()` frees C++ memory (the same use-after-free [leases] + * exists to stop). Parked here instead, freed by [drainRetiredLocked] once every lease is + * back. Guarded by [loadLock]. Briefly costs two resident models; a switch mid-listen is + * rare enough that correctness wins. */ + private val retired = ArrayList() + + /** Idle-reap timer, same idea as the web fetchers' `REAP_IDLE_MS` (issue #182). One daemon + * thread, shared, created lazily so a device that never loads the model never starts it. */ + private val reaper by lazy { + java.util.concurrent.Executors.newSingleThreadScheduledExecutor { r -> + Thread(r, "asr-reaper").apply { isDaemon = true } + } + } + @Volatile private var reapTask: java.util.concurrent.ScheduledFuture<*>? = null + + init { + // Measured on an M5 (2.9 GB, standardDebug, issue #83): the loaded Whisper tiny int8 model + // costs ~267 MB PSS - ~101 MB of weights in scudo:secondary plus ~146 MB of onnxruntime + // arena in scudo:primary. That was resident for the whole process with no way to reclaim it, + // and it survived deleteAsrEngine(). It is by far the largest single reclaimable allocation + // in the app, so it releases on any severe trim and reloads (~1 s) on the next listen. + app.vela.ui.MemoryPressure.register { level -> + if (app.vela.ui.MemoryPressure.isSevere(level)) release() + } + } + + /** + * Drop the model after a quiet period, on EVERY device, not just low-RAM ones. + * + * Warming at startup buys an instant first mic tap (a user asked for it, 2026-07-10) but the app + * was then holding ~267 MB for the whole session on the CHANCE of a tap that many users never + * make. Reaping after idle keeps the instant first tap and stops the model outliving the user's + * interest in it; a later tap pays the same ~1 s load the very first one used to. Every load and + * every listen re-arms the timer, so an active dictation session never reaps mid-use. + */ + private fun armIdleReap() { + reapTask?.cancel(false) + reapTask = runCatching { + reaper.schedule({ release() }, REAP_IDLE_MS, java.util.concurrent.TimeUnit.MILLISECONDS) + }.getOrNull() + } + + /** + * Free the native recognizer. Safe to call any time: no-op when nothing is loaded, and declines + * while a listen holds a lease (see [leases]). The next [listen]/[warmUp] rebuilds it. + * + * The lease check happens INSIDE [loadLock], together with the free, so a listen cannot start + * between the two. That is what makes this not a use-after-free. + * + * [wait] controls what happens when the lock is already held, which means a ~1 s native model + * load is in progress. The default does NOT block: the trim path runs on the MAIN thread from + * `Application.onTrimMemory`, and stalling the UI thread for a whole model load to reclaim + * memory is a bad trade when the idle reaper or the next trim will retry anyway. `Remove model` + * passes true, because there the user asked for it and a brief wait is correct. + */ + fun release(wait: Boolean = false) { + if (wait) loadLock.lock() else if (!loadLock.tryLock()) { + Timber.tag(TAG).i("release skipped, model load in progress") + return + } + try { + if (leases.get() > 0) { + Timber.tag(TAG).i("release skipped, listen in flight") + return + } + drainRetiredLocked() + val r = recognizer ?: return + recognizer = null + loadedKey = null + runCatching { r.release() } + .onFailure { Timber.tag(TAG).w(it, "recognizer release failed") } + Timber.tag(TAG).i("recognizer released") + } finally { + loadLock.unlock() + } + } + + /** + * Load if needed and take a LEASE, both under [loadLock]. Pair with [releaseLease] in a + * `finally`. Returns null when the model is absent or the native load failed, in which case no + * lease is taken. + * + * Callers must not hold the returned pointer past [releaseLease]: the lease is the only thing + * stopping [release] from freeing it. + */ + private fun acquireRecognizer(): OfflineRecognizer? { + loadLock.lock() + try { + val r = ensureRecognizerLocked() ?: return null + leases.incrementAndGet() + return r + } finally { + loadLock.unlock() + } + } + + /** + * Give back a lease taken by [acquireRecognizer]. Deliberately does NOT block on [loadLock]: the + * decrement happens only once the decode is finished with the pointer, so the worst a racing + * [release] can do is read the pre-decrement value and conservatively decline. Taking the lock + * here would instead park the end of every utterance behind an unrelated model load - so the + * retired-model drain runs only when the lock is free, and every other lock-holder (acquire, + * release, the next load) drains as well, so a skipped drain is picked up at the next one. + */ + private fun releaseLease() { + if (leases.decrementAndGet() == 0 && loadLock.tryLock()) { + try { + drainRetiredLocked() + } finally { + loadLock.unlock() + } + } + } + + /** Free every recognizer parked by an engine switch, once no lease can still be inside one. + * **Caller must hold [loadLock].** */ + private fun drainRetiredLocked() { + if (leases.get() > 0 || retired.isEmpty()) return + for (r in retired) { + runCatching { r.release() } + .onFailure { Timber.tag(TAG).w(it, "retired recognizer release failed") } + } + retired.clear() + Timber.tag(TAG).i("retired recognizer(s) released") + } + private val audioManager by lazy { context.getSystemService(Context.AUDIO_SERVICE) as? AudioManager } @Volatile private var focusRequest: AudioFocusRequest? = null @@ -90,6 +238,13 @@ class WhisperRecognizer @Inject constructor( const val SAMPLE_RATE = 16000 const val VAD_WINDOW = 512 // Silero v4/v5 window at 16 kHz const val MAX_SECONDS = 15 // hard cap on one utterance + // Drop the loaded model after this quiet period (issue #83): long enough that a dictation + // session never reaps between utterances, short enough that a session-long 267 MB hold + // cannot happen. RAM-SCALED, not flat: a reload costs ~1 s of dead mic on the next tap, + // and on a roomy phone that latency regression buys nothing the phone needed - pre-#83 + // those devices held the model all session and were fine. 2 min where the 267 MB actually + // hurts, 10 min where it is merely tidy. + val REAP_IDLE_MS: Long get() = if (app.vela.ui.MemoryPressure.lowRam) 120_000L else 600_000L } private fun prefs() = context.getSharedPreferences("vela_settings", Context.MODE_PRIVATE) @@ -139,6 +294,14 @@ class WhisperRecognizer @Inject constructor( * built recognizer for the current engine+language. */ fun warmUp() { if (!AsrEngine.anyInstalled(context)) return + // On a low-RAM device the warm-up is a bad trade: it spends ~267 MB (measured, issue #83) at + // EVERY launch to save ~1 s on a mic tap the user may never make, and refreshAsr() calls this + // from VM init plus two LaunchedEffects. Those phones load on first listen instead. Roomier + // devices keep the instant-mic behaviour they have always had. + if (app.vela.ui.MemoryPressure.lowRam) { + Timber.tag(TAG).i("skipping ASR warm-up on a low-RAM device, will load on first listen") + return + } Thread({ runCatching { ensureRecognizer() } }, "asr-warmup").start() } @@ -147,103 +310,126 @@ class WhisperRecognizer @Inject constructor( * installed/usable or the native load fails - callers fall back to the provider intent or hide * the mic. */ private fun ensureRecognizer(): OfflineRecognizer? { + loadLock.lock() + try { + return ensureRecognizerLocked() + } finally { + loadLock.unlock() + } + } + + /** + * The body of [ensureRecognizer]. **Caller must hold [loadLock].** + * + * There is deliberately NO lock-free fast path here any more. The old one + * (`recognizer?.let { if (loadedKey == key) return it }` before the lock) is what let a decode + * obtain the native pointer while [release] was between its lease check and its free. Taking the + * lock on every acquire costs an uncontended lock per listen, which is nothing next to a 15 s + * utterance, and it is what makes the lease in [acquireRecognizer] atomic. + */ + private fun ensureRecognizerLocked(): OfflineRecognizer? { + drainRetiredLocked() val engine = engineForNow() val lang = pinnedLang(engine) val key = "${engine.id}|$lang" - recognizer?.let { if (loadedKey == key) return it } - synchronized(loadLock) { - recognizer?.let { if (loadedKey == key) return it else runCatching { it.release() } } - recognizer = null - if (!engine.isInstalled(context)) return null - - // CRASH SENTINEL around the native load, PER ENGINE. sherpa-onnx parses the .onnx files in - // C++, and a TRUNCATED-but-non-empty model segfaults inside libsherpa-onnx-jni rather than - // throwing - `runCatching` cannot catch a native abort, it takes the whole process down. - // Reachable in the real world: a copy that stops partway (storage full, process killed) - // leaves a short file that isInstalled()'s present-and-non-empty test happily accepts, and - // warmUp() runs at STARTUP, so the result was an unrecoverable crash loop. So: bump a - // strike counter before the load, zero it after; a counter that reaches TWO stranded - // loads means the process died inside the load twice in a row - quarantine THAT engine - // only and delete THAT engine's dir (a bad SenseVoice must never take out Whisper), - // report not-installed, let the app start. A fresh download clears the quarantine. - // TWO strikes, not one (the map sentinel's idiom, and device-measured necessity): the - // load takes seconds, and a process killed DURING it - the user swiping the app away, the - // system reclaiming memory, a test harness force-stop - strands the counter exactly like - // a native crash. One stranded load used to delete a healthy 154 MB download; a genuinely - // bad model crashes EVERY load, so it still self-heals one launch later. - val prefs = prefs() - val strikesKey = KEY_LOAD_STRIKES + engine.id - val badKey = KEY_MODEL_BAD + engine.id - val strikes = prefs.getInt(strikesKey, 0) - if (strikes >= 2) { - Timber.tag(TAG).e("two ASR loads never returned (native crash) - quarantining ${engine.id}") - prefs.edit().putInt(strikesKey, 0).putBoolean(badKey, true).apply() - runCatching { engine.dir(context).deleteRecursively() } - return null - } - if (prefs.getBoolean(badKey, false)) return null - prefs.edit().putInt(strikesKey, strikes + 1).apply() - - val dir = engine.dir(context) - fun p(name: String) = File(dir, name).absolutePath - val modelConfig = when (engine) { - AsrEngine.WHISPER_TINY -> OfflineModelConfig( - whisper = OfflineWhisperModelConfig( - encoder = p("tiny-encoder.int8.onnx"), - decoder = p("tiny-decoder.int8.onnx"), - language = lang, // pinned to the app language ("" = auto) - task = "transcribe", - tailPaddings = -1, - ), - tokens = p("tiny-tokens.txt"), - numThreads = 2, - modelType = engine.modelType, - ) - AsrEngine.SENSE_VOICE -> OfflineModelConfig( - senseVoice = OfflineSenseVoiceModelConfig( - model = p("model.int8.onnx"), - language = lang, // "auto" or one of zh/en/ja/ko/yue - useInverseTextNormalization = true, // "5 pm" not "five p m" - ), - tokens = p("tokens.txt"), - numThreads = 2, - modelType = engine.modelType, - ) - AsrEngine.MOONSHINE -> OfflineModelConfig( - moonshine = OfflineMoonshineModelConfig( - preprocessor = p("preprocess.onnx"), - encoder = p("encode.int8.onnx"), - uncachedDecoder = p("uncached_decode.int8.onnx"), - cachedDecoder = p("cached_decode.int8.onnx"), - ), - tokens = p("tokens.txt"), - numThreads = 2, - modelType = engine.modelType, - ) - } - val r = runCatching { - OfflineRecognizer( - config = OfflineRecognizerConfig( - featConfig = FeatureConfig(sampleRate = SAMPLE_RATE, featureDim = 80), - modelConfig = modelConfig, - ), - ) - }.onFailure { - // NAME the throwable. #84 fixed "the reason was discarded at the point of failure" - // for listen(), but left it here: getOrNull() ate the one fact that separates a - // missing .so (UnsatisfiedLinkError - the v7a strip, see app/build.gradle.kts) from - // an OOM on a small phone, and both surfaced as "re-download the model". A tester - // re-downloaded 47 MB twice on that advice. The class name alone decides it. - Timber.tag(TAG).e(it, "native ASR load failed (${engine.id}): ${it::class.java.simpleName}") - }.getOrNull() - // The load RETURNED (success or a catchable failure), so the process survived it: zero - // the strikes. Only a native abort (or a mid-load kill) leaves a strike standing, and - // only two in a row quarantine. - prefs.edit().putInt(strikesKey, 0).apply() - recognizer = r - loadedKey = key - return r + recognizer?.let { + if (loadedKey == key) return it + // Engine/language switch. Free the superseded model only if no listen can still be + // decoding inside it; otherwise park it in [retired] - releasing native memory under + // an outstanding lease is the use-after-free the lease exists to prevent. + if (leases.get() > 0) retired.add(it) else runCatching { it.release() } + } + recognizer = null + if (!engine.isInstalled(context)) return null + + // CRASH SENTINEL around the native load, PER ENGINE. sherpa-onnx parses the .onnx files in + // C++, and a TRUNCATED-but-non-empty model segfaults inside libsherpa-onnx-jni rather than + // throwing - `runCatching` cannot catch a native abort, it takes the whole process down. + // Reachable in the real world: a copy that stops partway (storage full, process killed) + // leaves a short file that isInstalled()'s present-and-non-empty test happily accepts, and + // warmUp() runs at STARTUP, so the result was an unrecoverable crash loop. So: bump a + // strike counter before the load, zero it after; a counter that reaches TWO stranded + // loads means the process died inside the load twice in a row - quarantine THAT engine + // only and delete THAT engine's dir (a bad SenseVoice must never take out Whisper), + // report not-installed, let the app start. A fresh download clears the quarantine. + // TWO strikes, not one (the map sentinel's idiom, and device-measured necessity): the + // load takes seconds, and a process killed DURING it - the user swiping the app away, the + // system reclaiming memory, a test harness force-stop - strands the counter exactly like + // a native crash. One stranded load used to delete a healthy 154 MB download; a genuinely + // bad model crashes EVERY load, so it still self-heals one launch later. + val prefs = prefs() + val strikesKey = KEY_LOAD_STRIKES + engine.id + val badKey = KEY_MODEL_BAD + engine.id + val strikes = prefs.getInt(strikesKey, 0) + if (strikes >= 2) { + Timber.tag(TAG).e("two ASR loads never returned (native crash) - quarantining ${engine.id}") + prefs.edit().putInt(strikesKey, 0).putBoolean(badKey, true).apply() + runCatching { engine.dir(context).deleteRecursively() } + return null } + if (prefs.getBoolean(badKey, false)) return null + prefs.edit().putInt(strikesKey, strikes + 1).apply() + + val dir = engine.dir(context) + fun p(name: String) = File(dir, name).absolutePath + val modelConfig = when (engine) { + AsrEngine.WHISPER_TINY -> OfflineModelConfig( + whisper = OfflineWhisperModelConfig( + encoder = p("tiny-encoder.int8.onnx"), + decoder = p("tiny-decoder.int8.onnx"), + language = lang, // pinned to the app language ("" = auto) + task = "transcribe", + tailPaddings = -1, + ), + tokens = p("tiny-tokens.txt"), + numThreads = 2, + modelType = engine.modelType, + ) + AsrEngine.SENSE_VOICE -> OfflineModelConfig( + senseVoice = OfflineSenseVoiceModelConfig( + model = p("model.int8.onnx"), + language = lang, // "auto" or one of zh/en/ja/ko/yue + useInverseTextNormalization = true, // "5 pm" not "five p m" + ), + tokens = p("tokens.txt"), + numThreads = 2, + modelType = engine.modelType, + ) + AsrEngine.MOONSHINE -> OfflineModelConfig( + moonshine = OfflineMoonshineModelConfig( + preprocessor = p("preprocess.onnx"), + encoder = p("encode.int8.onnx"), + uncachedDecoder = p("uncached_decode.int8.onnx"), + cachedDecoder = p("cached_decode.int8.onnx"), + ), + tokens = p("tokens.txt"), + numThreads = 2, + modelType = engine.modelType, + ) + } + val r = runCatching { + OfflineRecognizer( + config = OfflineRecognizerConfig( + featConfig = FeatureConfig(sampleRate = SAMPLE_RATE, featureDim = 80), + modelConfig = modelConfig, + ), + ) + }.onFailure { + // NAME the throwable. #84 fixed "the reason was discarded at the point of failure" + // for listen(), but left it here: getOrNull() ate the one fact that separates a + // missing .so (UnsatisfiedLinkError - the v7a strip, see app/build.gradle.kts) from + // an OOM on a small phone, and both surfaced as "re-download the model". A tester + // re-downloaded 47 MB twice on that advice. The class name alone decides it. + Timber.tag(TAG).e(it, "native ASR load failed (${engine.id}): ${it::class.java.simpleName}") + }.getOrNull() + // The load RETURNED (success or a catchable failure), so the process survived it: zero + // the strikes. Only a native abort (or a mid-load kill) leaves a strike standing, and + // only two in a row quarantine. + prefs.edit().putInt(strikesKey, 0).apply() + recognizer = r + loadedKey = key + if (r != null) armIdleReap() // start the quiet-period countdown from the load + return r } /** @@ -253,18 +439,53 @@ class WhisperRecognizer @Inject constructor( * (the user tapped done/close). Runs off the main thread; safe to cancel via coroutine too. */ /** Listen, transcribe, and say WHY when it does not work - see [VoiceResult]. Every failure exit - * logs under `VELAASR` so a tester's logcat names the cause without another round-trip. */ + * logs under `VELAASR` so a tester's logcat names the cause without another round-trip. + * + * Thin wrapper over [listenInner] that holds a LEASE on the recognizer for the whole utterance, + * so a memory trim or the idle reaper arriving mid-utterance cannot free the native model out + * from under the decode (see [leases] and [acquireRecognizer]). Taking the lease out here, not + * inside the inner function, is what makes it exception-safe against that function's many + * early returns. */ suspend fun listen( onLevel: (Float) -> Unit, onListening: () -> Unit, cancelled: () -> Boolean, + ): VoiceResult { + // Acquire (and load) OFF the main thread: this can be a ~1 s native load, and callers reach + // listen() from a UI coroutine. The lease is taken here rather than inside listenInner so + // that the many early returns in there cannot leak it - the finally below always gives it + // back, and it covers recording as well as decoding. + // + // `leased` is set INSIDE the withContext block, not inferred from `rec`: a coroutine + // cancelled during the ~1 s load makes withContext run the block to completion (taking the + // lease) and then throw CancellationException INSTEAD of returning the value - `rec` would + // never be assigned, and a rec-based finally would leak the lease forever, permanently + // disabling every release path (reaper, trims, Remove model). + reapTask?.cancel(false) // never reap mid-utterance + var leased = false + try { + val rec = withContext(Dispatchers.Default) { acquireRecognizer()?.also { leased = true } } + ?: run { + Timber.tag(TAG).e("listen failed: MODEL (model absent or native load failed)") + return VoiceResult.Failed(VoiceResult.Reason.MODEL, "model absent or native load failed") + } + return listenInner(rec, onLevel, onListening, cancelled) + } finally { + if (leased) releaseLease() + armIdleReap() // restart the quiet period from the END of this utterance + } + } + + private suspend fun listenInner( + rec: OfflineRecognizer, + onLevel: (Float) -> Unit, + onListening: () -> Unit, + cancelled: () -> Boolean, ): VoiceResult = withContext(Dispatchers.Default) { fun fail(reason: VoiceResult.Reason, detail: String? = null): VoiceResult.Failed { Timber.tag(TAG).e("listen failed: $reason${detail?.let { " ($it)" } ?: ""}") return VoiceResult.Failed(reason, detail) } - val rec = ensureRecognizer() - ?: return@withContext fail(VoiceResult.Reason.MODEL, "model absent or native load failed") if (!hasMicPermission()) return@withContext fail(VoiceResult.Reason.PERMISSION) val vad = runCatching { diff --git a/app/src/main/java/app/vela/web/WebDirectionsFetcher.kt b/app/src/main/java/app/vela/web/WebDirectionsFetcher.kt index 98be24ff..e3d7883c 100644 --- a/app/src/main/java/app/vela/web/WebDirectionsFetcher.kt +++ b/app/src/main/java/app/vela/web/WebDirectionsFetcher.kt @@ -59,20 +59,54 @@ class WebDirectionsFetcher @Inject constructor( @Volatile private var webView: WebView? = null private var reap: Runnable? = null + /** Run [block] on the main thread, inline when already there. All reap bookkeeping goes through + * here: [reap] is touched by the trim listener (main) and by the fetch path (the caller's + * dispatcher), and scheduling is a read-modify-write - main-confinement removes the race by + * construction (WebPhotoFetcher's fix, issue #83 follow-up). */ + private fun onMain(block: () -> Unit) { + if (Looper.myLooper() == Looper.getMainLooper()) block() else main.post(block) + } + /** Free the WebView after a quiet period (issue #182): a warm fetcher pins a full * maps.google.com page for the rest of the session, and several warm fetchers at once is * real memory. The next fetch after a reap just re-creates it. */ - private fun scheduleReap() { + private fun scheduleReap() = onMain { reap?.let(main::removeCallbacks) - val r = Runnable { - webView?.let { runCatching { it.loadUrl("about:blank"); it.destroy() } } - webView = null - } + val r = Runnable { reap = null; reapNow() } reap = r main.postDelayed(r, REAP_IDLE_MS) } - private fun cancelReap() { + /** Destroy the WebView. Main thread only (WebView requirement). In-flight aware, both ways: + * with a fetch in flight ([pending] non-empty) a non-forced reap DECLINES - destroying the + * view kills the injected scraper and turns a live fetch into an empty result, and the fetch's + * own finally re-arms the reap moments later anyway. A FORCED reap (critical trim - the OS is + * about to start killing) destroys regardless, then drains [pending] so the stranded fetch + * fails fast as empty instead of parking in `deferred.await()` for the full [TOTAL_TIMEOUT_MS] + * while holding [mutex]. */ + private fun reapNow(force: Boolean = false) { + if (!force && pending.isNotEmpty()) { + android.util.Log.i("WebDirectionsFetcher", "reap declined, fetch in flight") + return + } + webView?.let { runCatching { it.loadUrl("about:blank"); it.destroy() } } + webView = null + pending.keys.toList().forEach { id -> pending.remove(id)?.complete("") } + } + + init { + // Under real memory pressure the 120 s idle timer is far too slow - the OS is asking for + // memory NOW and a Chromium renderer is one of the largest things we hold (issue #83). + // Reap on the main thread, since WebView.destroy() requires it. Only a CRITICAL trim tears + // down mid-fetch; a merely-severe one declines while a fetch is in flight (see reapNow). + app.vela.ui.MemoryPressure.register { level -> + if (app.vela.ui.MemoryPressure.isSevere(level)) { + main.post { cancelReap(); reapNow(force = app.vela.ui.MemoryPressure.isCritical(level)) } + } + } + } + + private fun cancelReap() = onMain { reap?.let(main::removeCallbacks) reap = null } diff --git a/app/src/main/java/app/vela/web/WebPhotoFetcher.kt b/app/src/main/java/app/vela/web/WebPhotoFetcher.kt index 3e2b5669..24a0856b 100644 --- a/app/src/main/java/app/vela/web/WebPhotoFetcher.kt +++ b/app/src/main/java/app/vela/web/WebPhotoFetcher.kt @@ -54,6 +54,86 @@ class WebPhotoFetcher @Inject constructor( @Volatile private var webView: WebView? = null @Volatile private var warmed = false + /** Pending idle-reap callback. Only ever read or written on the main thread - see [onMain]. */ + private var reap: Runnable? = null + + init { + // A trim is the OS asking for memory NOW, far sooner than any idle timer (issue #83). + // Only a CRITICAL trim tears down mid-scrape; a merely-severe one declines while a fetch + // is in flight (see reapNow) so a moderate-pressure moment doesn't cost a whole gallery. + app.vela.ui.MemoryPressure.register { level -> + if (app.vela.ui.MemoryPressure.isSevere(level)) { + main.post { cancelReap(); reapNow(force = app.vela.ui.MemoryPressure.isCritical(level)) } + } + } + } + + /** + * Run [block] on the main thread, inline when already there. + * + * All reap bookkeeping goes through here because [reap] is touched from two places on + * different threads - the trim listener (main) and [fetch]/[warm] (the caller's dispatcher). + * Making every mutation main-thread-only removes the race by construction; @Volatile would not, + * since scheduling is a read-modify-write. Running inline when already on main keeps the + * cancel-then-navigate ordering inside [fetch]'s `Dispatchers.Main` block intact. + */ + private fun onMain(block: () -> Unit) { + if (Looper.myLooper() == Looper.getMainLooper()) block() else main.post(block) + } + + /** + * Free the WebView after a quiet period, like the other four fetchers (issue #182) - this one + * never had it, so a single search pinned a Chromium renderer for the WHOLE session and only + * renderer death or a trim could clear it. Measured on the M5: after a search the shared + * sandboxed renderer sat at 327 MB PSS, in a SEPARATE process, so it never showed up in the + * app's own PSS and every issue-#83 measurement missed it. + * + * [delayMs] differs by caller on purpose. A real fetch uses the siblings' [REAP_IDLE_MS]; a + * speculative [warm] uses the longer [WARM_REAP_IDLE_MS], because the warm exists precisely so + * a later place tap skips the cold start, and reaping it at 120 s would undo it for the ordinary + * search-then-browse-then-tap flow. Longer, but still bounded: the point is that it cannot be + * session-long. + */ + private fun scheduleReap(delayMs: Long) = onMain { + reap?.let(main::removeCallbacks) + val r = Runnable { reap = null; reapNow() } + reap = r + main.postDelayed(r, delayMs) + } + + private fun cancelReap() = onMain { + reap?.let(main::removeCallbacks) + reap = null + } + + /** Destroy the WebView immediately. Main thread only (WebView requirement). The next + * [warm]/fetch rebuilds it via `ensureWebView`, exactly as after a renderer death. + * + * A NON-FORCED reap declines while a scrape is in flight ([pending] non-empty): destroying the + * view mid-scrape turns a live gallery into 0 photos, and the fetch's finally re-arms the reap + * moments later anyway. A forced reap (critical trim) proceeds and drains [pending] for the + * same reason [rendererGone] does: the injected scraper dies with the view, so nothing will + * ever complete those deferreds. Without the drain a teardown mid-fetch leaves the fetch + * parked in `deferred.await()` for the full [TOTAL_TIMEOUT_MS] while it HOLDS [mutex], + * stalling every queued gallery behind it. An empty result is the documented best-effort + * failure mode; a 40 s hang is not. */ + private fun reapNow(force: Boolean = false) { + if (!force && pending.isNotEmpty()) { + android.util.Log.i("WebPhotoFetcher", "reap declined, fetch in flight") + return + } + val wv = webView ?: return + webView = null + warmed = false + runCatching { wv.loadUrl("about:blank"); wv.destroy() } + val stranded = pending.keys.toList() + stranded.forEach { id -> pending.remove(id)?.complete("") } + // The renderer is shared and lives in ANOTHER process, so its cost is invisible in this + // app's PSS - without a log there is no way to tell an idle reap from an OS trim killing + // the renderer, which made the first attempt to verify this unfalsifiable. + android.util.Log.i("WebPhotoFetcher", "photo WebView reaped (stranded fetches: ${stranded.size})") + } + // featureId → its scraped gallery. Re-tapping a place (or bouncing back from directions) then // shows photos INSTANTLY instead of re-running the ~20 s scrape. Access-order LRU, small cap. private val cache = object : LinkedHashMap>(16, 0.75f, true) { @@ -99,6 +179,10 @@ class WebPhotoFetcher @Inject constructor( } } wv.loadUrl("https://www.google.com/maps?hl=en") + // Bound the speculative warm. Reached only when THIS call created the view (the + // re-check above returns early when a fetch already owns it, and that fetch arms its + // own reap in `finally`), so this never shortens a real fetch's window. + scheduleReap(WARM_REAP_IDLE_MS) } } } @@ -125,6 +209,7 @@ class WebPhotoFetcher @Inject constructor( val cid = cidOf(featureId) ?: return emptyList() synchronized(cache) { cache[featureId] }?.let { return it } // instant on revisit - skip the scrape return mutex.withLock { + cancelReap() // a reap mid-scrape would destroy the view this fetch is about to drive val id = "p" + seq.incrementAndGet() val deferred = CompletableDeferred() pending[id] = deferred @@ -146,6 +231,13 @@ class WebPhotoFetcher @Inject constructor( return !(host == "google.com" || host.endsWith(".google.com")) } override fun onPageFinished(view: WebView?, url: String?) { + // IGNORE about:blank. [blankAfterScrape] parks the view there after the + // previous scrape, and that navigation can still be settling when this + // client is installed - its onPageFinished then opens the load gate + // early, the scraper injects into an empty document, and the fetch + // returns 0 photos. Device-caught: the same place scraped 33 photos as + // the first place opened and 0 as the second, until this guard. + if (url == null || url.startsWith("about:")) return main.postDelayed({ if (!ready.isCompleted) ready.complete(Unit) }, SETTLE_MS) } override fun onRenderProcessGone(view: WebView?, detail: RenderProcessGoneDetail?): Boolean { @@ -161,6 +253,9 @@ class WebPhotoFetcher @Inject constructor( // DOM that yields an empty result (safe) instead of the previous place's photos // being returned for THIS featureId (cross-place data). wv.evaluateJavascript("try{document.documentElement.innerHTML=''}catch(e){}", null) + // Size BEFORE navigating, so the ?cid= page's first layout is already at + // scrape geometry and the virtualized grids materialize exactly as before. + sizeForScrape(wv) wv.loadUrl("https://www.google.com/maps?cid=$cid&hl=en&gl=us") main.postDelayed({ if (!ready.isCompleted) ready.complete(Unit) }, MAX_LOAD_MS) ready.await() @@ -173,8 +268,13 @@ class WebPhotoFetcher @Inject constructor( } finally { pending.remove(id) partials.remove(id) + blankAfterScrape() // the scraped page is dead weight from here until the next scrape + scheduleReap(REAP_IDLE_MS) // start the quiet period from the END of the scrape } val out = raw?.let { parseLines(it) } ?: emptyList() + // Result count, so a change to the offscreen viewport (WV_WIDTH/WV_HEIGHT drive how much + // of the virtualized grid renders) can be A/B'd against scrape QUALITY, not just memory. + android.util.Log.i("WebPhotoFetcher", "scraped ${out.size} photos for $featureId") if (out.isNotEmpty()) synchronized(cache) { cache[featureId] = out } // cache only real results out } @@ -207,15 +307,60 @@ class WebPhotoFetcher @Inject constructor( wv.settings.domStorageEnabled = true wv.settings.userAgentString = VelaConfig.USER_AGENT wv.addJavascriptInterface(Bridge(), "VelaBridge") - // Real offscreen viewport - the category grids are VIRTUALIZED (like the reviews list); at 0×0 a - // category tab renders only ~1 tile, so a tall viewport is what makes each category populate fully. + // NOT laid out here on purpose - see [sizeForScrape]. The WebView stays 0x0 until a real + // fetch needs the grids to materialize. + webView = wv + return wv + } + + /** + * Give the WebView its real offscreen viewport, immediately before a scrape navigates. + * + * The size itself is load-bearing: the category grids are VIRTUALIZED (like the reviews list), + * so at 0x0 a category tab renders only about one tile and the scrape comes back nearly empty. + * That is why the viewport exists at all. + * + * But it only has to exist for a SCRAPE. This used to run in `ensureWebView`, i.e. at + * construction, and [warm] goes through `ensureWebView` - so a speculative warm created a full + * WV_WIDTH x WV_HEIGHT composited surface over `maps?hl=en`, a page with zero scrapeable content, + * and held it for the whole WARM_REAP_IDLE_MS window. Measured on the M5: `GL mtrack` is bimodal, + * ~490 MB with this view laid out and alive versus ~71 MB without, on a 480x640 phone screen. + * This fetcher is the only one that lays out during a warm at all (the other four either never + * call layout or have no warm), which is why every GL number tracked THIS view. + * + * Idempotent, and called before `loadUrl` so the `?cid=` page's FIRST layout is already at scrape + * geometry - that ordering is what keeps the scrape identical. + */ + private fun sizeForScrape(wv: WebView) { + if (wv.width == WV_WIDTH && wv.height == WV_HEIGHT) return wv.measure( android.view.View.MeasureSpec.makeMeasureSpec(WV_WIDTH, android.view.View.MeasureSpec.EXACTLY), android.view.View.MeasureSpec.makeMeasureSpec(WV_HEIGHT, android.view.View.MeasureSpec.EXACTLY), ) wv.layout(0, 0, WV_WIDTH, WV_HEIGHT) - webView = wv - return wv + } + + /** + * Throw the scraped page away the moment the scrape returns, rather than carrying it until the + * reap 120 s later. + * + * The scrape is the only thing that needed the page and it is over - the result is already + * parsed out of the bridge payload. What follows is the user reading the place sheet, which is + * minutes of a fully rasterized Google Maps document serving nobody. + * + * It has to be a NAVIGATION, not a resize. Shrinking the view back to 0x0 was tried first and + * measured to reclaim nothing at all (GL mtrack 494/496/497 MB against a 497/498 MB control): + * Chromium keeps the tiles it has already rasterized for a live document regardless of the + * view's size. Discarding the document is what frees them. + * + * Costs nothing functionally: the next `fetch` blanks the DOM and navigates to its own `?cid=` + * page anyway, so this page was never going to be read again. The WebView itself stays alive, so + * the renderer, HTTP/2 sockets, cookies and JS cache that make the next place fast are all kept - + * which is exactly what destroying it early would have thrown away. + */ + private fun blankAfterScrape() = onMain { + val wv = webView ?: return@onMain + runCatching { wv.loadUrl("about:blank") } } /** Self-polling DOM scraper: open the gallery, then VISIT EACH CATEGORY TAB (Menu / Food & drink / @@ -292,7 +437,21 @@ class WebPhotoFetcher @Inject constructor( const val SETTLE_MS = 1_200L const val MAX_LOAD_MS = 7_000L // Offscreen viewport so the virtualized category grids render a full batch (not ~1 tile). + // + // Applied by [sizeForScrape] immediately before a scrape navigates, NOT at construction. + // + // These are deliberately UNCHANGED from stock. Shrinking the width to 720 was tried and did + // hold the photo count on the one place it was A/B'd (28 -> 28), but scrape geometry governs + // how much of a virtualized grid materializes, and one place is not enough evidence to risk a + // quieter gallery in a locale or layout nobody sampled. Deferring the layout wins the same + // memory back without changing anything the scraper sees, so the size stays stock. const val WV_WIDTH = 1200 const val WV_HEIGHT = 3200 + // Destroy the idle WebView after this quiet period, same value as the other four fetchers. + const val REAP_IDLE_MS = 120_000L + // The speculative warm gets a longer leash: it is spent so the first place tap is instant, + // and 120 s would expire during an ordinary browse and waste the warm entirely. Still + // bounded, which is the whole point - before this the warm was held for the session. + const val WARM_REAP_IDLE_MS = 300_000L } } diff --git a/app/src/main/java/app/vela/web/WebPopularTimesFetcher.kt b/app/src/main/java/app/vela/web/WebPopularTimesFetcher.kt index dff125a7..480653d5 100644 --- a/app/src/main/java/app/vela/web/WebPopularTimesFetcher.kt +++ b/app/src/main/java/app/vela/web/WebPopularTimesFetcher.kt @@ -54,21 +54,57 @@ class WebPopularTimesFetcher @Inject constructor( @Volatile private var warm: CompletableDeferred? = null private var reap: Runnable? = null + /** Run [block] on the main thread, inline when already there. [reap] is touched from the trim + * listener (main) and from [fetch]/[prewarm] (the caller's dispatcher), and scheduling is a + * read-modify-write that @Volatile would not make safe. */ + private fun onMain(block: () -> Unit) { + if (Looper.myLooper() == Looper.getMainLooper()) block() else main.post(block) + } + /** Free the WebView after a quiet period (issue #182): the warm session pins a full * maps.google.com page for the rest of the session otherwise. The next fetch after a - * reap re-warms (google.com -> maps), a one-off few-second cost after minutes idle. */ - private fun scheduleReap() { + * reap re-warms (google.com -> maps), a one-off few-second cost after minutes idle. + * + * [delayMs] is longer for a speculative [prewarm] than for a real fetch - see + * [WARM_REAP_IDLE_MS]. */ + private fun scheduleReap(delayMs: Long = REAP_IDLE_MS) = onMain { reap?.let(main::removeCallbacks) - val r = Runnable { - webView?.let { runCatching { it.loadUrl("about:blank"); it.destroy() } } - webView = null - warm = null // ensureWarm re-runs the warm sequence on the next fetch - } + val r = Runnable { reap = null; reapNow() } reap = r - main.postDelayed(r, REAP_IDLE_MS) + main.postDelayed(r, delayMs) + } + + /** Destroy the WebView. Main thread only (WebView requirement). In-flight aware, both ways: + * with a fetch in flight ([pending] non-empty) a non-forced reap DECLINES - destroying the + * view kills the injected scraper and turns a live fetch into an empty result, and the fetch's + * own finally re-arms the reap moments later anyway. A FORCED reap (critical trim - the OS is + * about to start killing) destroys regardless, then drains [pending] so the stranded fetch + * fails fast as empty instead of parking in `deferred.await()` for the full + * [TOTAL_TIMEOUT_MS]. */ + private fun reapNow(force: Boolean = false) { + if (!force && pending.isNotEmpty()) { + android.util.Log.i("WebPopularTimesFetcher", "reap declined, fetch in flight") + return + } + webView?.let { runCatching { it.loadUrl("about:blank"); it.destroy() } } + webView = null + warm = null // ensureWarm re-runs the warm sequence on the next fetch + pending.keys.toList().forEach { id -> pending.remove(id)?.complete("") } + } + + init { + // Under real memory pressure the 120 s idle timer is far too slow - the OS is asking for + // memory NOW and a Chromium renderer is one of the largest things we hold (issue #83). + // Reap on the main thread, since WebView.destroy() requires it. Only a CRITICAL trim tears + // down mid-fetch; a merely-severe one declines while a fetch is in flight (see reapNow). + app.vela.ui.MemoryPressure.register { level -> + if (app.vela.ui.MemoryPressure.isSevere(level)) { + main.post { cancelReap(); reapNow(force = app.vela.ui.MemoryPressure.isCritical(level)) } + } + } } - private fun cancelReap() { + private fun cancelReap() = onMain { reap?.let(main::removeCallbacks) reap = null } @@ -124,6 +160,12 @@ class WebPopularTimesFetcher @Inject constructor( * idempotent (a warm already in progress is awaited, not restarted). */ suspend fun prewarm() { runCatching { withTimeoutOrNull(MAX_WARM_MS + 2_000L) { ensureWarm() } } + // Bound the speculative warm. Without this a prewarm-only WebView was never reaped at all - + // scheduleReap() is called only from fetch() - so one search pinned the SHARED Chromium + // renderer for the whole session. Device-verified: reaping WebPhotoFetcher's view alone left + // the renderer alive at ~200 MB PSS because this fetcher still held one. A real fetch + // cancels this and re-arms the shorter window in its own finally. + scheduleReap(WARM_REAP_IDLE_MS) } private suspend fun ensureWarm() = withContext(Dispatchers.Main) { @@ -207,6 +249,10 @@ class WebPopularTimesFetcher @Inject constructor( private companion object { const val TOTAL_TIMEOUT_MS = 22_000L const val REAP_IDLE_MS = 120_000L // destroy the idle WebView after this quiet period (issue #182) + // A speculative prewarm gets a longer leash than a real fetch: it is spent so the first + // place tap is fast, and 120 s would expire during an ordinary browse and waste it. Bounded + // is the point - before this a prewarm-only view was never reaped at all. + const val WARM_REAP_IDLE_MS = 300_000L const val SETTLE_MS = 1_200L const val MAX_WARM_MS = 9_000L } diff --git a/app/src/main/java/app/vela/web/WebReviewsFetcher.kt b/app/src/main/java/app/vela/web/WebReviewsFetcher.kt index 6d1fa9fd..1ccf9b6e 100644 --- a/app/src/main/java/app/vela/web/WebReviewsFetcher.kt +++ b/app/src/main/java/app/vela/web/WebReviewsFetcher.kt @@ -54,20 +54,54 @@ class WebReviewsFetcher @Inject constructor( @Volatile private var webView: WebView? = null private var reap: Runnable? = null + /** Run [block] on the main thread, inline when already there. All reap bookkeeping goes through + * here: [reap] is touched by the trim listener (main) and by the fetch path (the caller's + * dispatcher), and scheduling is a read-modify-write - main-confinement removes the race by + * construction (WebPhotoFetcher's fix, issue #83 follow-up). */ + private fun onMain(block: () -> Unit) { + if (Looper.myLooper() == Looper.getMainLooper()) block() else main.post(block) + } + /** Free the WebView after a quiet period (issue #182): a warm fetcher pins a full * maps.google.com page for the rest of the session, and several warm fetchers at once is * real memory. The next fetch after a reap just re-creates it. */ - private fun scheduleReap() { + private fun scheduleReap() = onMain { reap?.let(main::removeCallbacks) - val r = Runnable { - webView?.let { runCatching { it.loadUrl("about:blank"); it.destroy() } } - webView = null - } + val r = Runnable { reap = null; reapNow() } reap = r main.postDelayed(r, REAP_IDLE_MS) } - private fun cancelReap() { + /** Destroy the WebView. Main thread only (WebView requirement). In-flight aware, both ways: + * with a fetch in flight ([pending] non-empty) a non-forced reap DECLINES - destroying the + * view kills the injected scraper and turns a live fetch into an empty panel, and the fetch's + * own finally re-arms the reap moments later anyway. A FORCED reap (critical trim - the OS is + * about to start killing) destroys regardless, then drains [pending] so the stranded fetch + * fails fast as empty instead of parking in `deferred.await()` for the full [TOTAL_TIMEOUT_MS] + * while holding [mutex] (which would stall every queued fetch behind a 45 s hang). */ + private fun reapNow(force: Boolean = false) { + if (!force && pending.isNotEmpty()) { + android.util.Log.i("WebReviewsFetcher", "reap declined, fetch in flight") + return + } + webView?.let { runCatching { it.loadUrl("about:blank"); it.destroy() } } + webView = null + pending.keys.toList().forEach { id -> pending.remove(id)?.complete("") } + } + + init { + // Under real memory pressure the 120 s idle timer is far too slow - the OS is asking for + // memory NOW and a Chromium renderer is one of the largest things we hold (issue #83). + // Reap on the main thread, since WebView.destroy() requires it. Only a CRITICAL trim tears + // down mid-fetch; a merely-severe one declines while a fetch is in flight (see reapNow). + app.vela.ui.MemoryPressure.register { level -> + if (app.vela.ui.MemoryPressure.isSevere(level)) { + main.post { cancelReap(); reapNow(force = app.vela.ui.MemoryPressure.isCritical(level)) } + } + } + } + + private fun cancelReap() = onMain { reap?.let(main::removeCallbacks) reap = null } @@ -109,7 +143,11 @@ class WebReviewsFetcher @Inject constructor( return mutex.withLock { cancelReap() try { + // Result count, so a change to the offscreen viewport (WV_WIDTH/WV_HEIGHT drive how + // much of the virtualized list renders) can be A/B'd against scrape QUALITY, not + // just memory. fetchLocked(cid, onProgress, onPartial) + .also { android.util.Log.i("WebReviewsFetcher", "scraped ${it.size} reviews for $featureId") } } finally { scheduleReap() } @@ -413,6 +451,16 @@ class WebReviewsFetcher @Inject constructor( const val MAX_LOAD_MS = 7_000L // Offscreen viewport for the headless WebView - tall so the virtualized review list renders a // healthy batch per scroll position. + // + // Left at STOCK. A 720 px width was tried and reverted: the reviews scrape returns 0 on the + // test device for every place tried, at 1200 AND at 720 - a pre-existing failure, not caused + // by the width - so the quality metric was pinned at zero and the A/B could not fail. An + // unfalsifiable check is not evidence, and scrape geometry governs how much of the + // virtualized list materializes. Do not narrow this until the scrape works again and + // `scraped N reviews` can be compared on a place with hundreds of them. + // + // Unlike WebPhotoFetcher this fetcher lays out only inside a real fetch (it has no warm), so + // it never contributed to the idle/warm-window cost that the deferred layout there fixes. const val WV_WIDTH = 1200 const val WV_HEIGHT = 6000 } diff --git a/app/src/main/java/app/vela/web/WebStopDeparturesFetcher.kt b/app/src/main/java/app/vela/web/WebStopDeparturesFetcher.kt index dc3a4edd..af133a33 100644 --- a/app/src/main/java/app/vela/web/WebStopDeparturesFetcher.kt +++ b/app/src/main/java/app/vela/web/WebStopDeparturesFetcher.kt @@ -48,21 +48,55 @@ class WebStopDeparturesFetcher @Inject constructor( @Volatile private var webView: WebView? = null private var reap: Runnable? = null + /** Run [block] on the main thread, inline when already there. All reap bookkeeping goes through + * here: [reap] is touched by the trim listener (main) and by the fetch path (the caller's + * dispatcher), and scheduling is a read-modify-write - main-confinement removes the race by + * construction (WebPhotoFetcher's fix, issue #83 follow-up). */ + private fun onMain(block: () -> Unit) { + if (Looper.myLooper() == Looper.getMainLooper()) block() else main.post(block) + } + /** Free the WebView after a quiet period. A warm fetcher otherwise pins a full * maps.google.com page (DOM + renderer) for the rest of the session, and several warm * fetchers at once is real memory pressure (issue #182). The next fetch after a reap * just re-creates it - a one-off warm-up, only after minutes of not using the feature. */ - private fun scheduleReap() { + private fun scheduleReap() = onMain { reap?.let(main::removeCallbacks) - val r = Runnable { - webView?.let { runCatching { it.loadUrl("about:blank"); it.destroy() } } - webView = null - } + val r = Runnable { reap = null; reapNow() } reap = r main.postDelayed(r, REAP_IDLE_MS) } - private fun cancelReap() { + /** Destroy the WebView. Main thread only (WebView requirement). In-flight aware, both ways: + * with a fetch in flight ([pending] non-empty) a non-forced reap DECLINES - destroying the + * view kills the injected scraper and turns a live fetch into an empty board, and the fetch's + * own finally re-arms the reap moments later anyway. A FORCED reap (critical trim - the OS is + * about to start killing) destroys regardless, then drains [pending] so the stranded fetch + * fails fast as empty instead of parking in `deferred.await()` for the full [TOTAL_TIMEOUT_MS] + * while holding [mutex]. */ + private fun reapNow(force: Boolean = false) { + if (!force && pending.isNotEmpty()) { + android.util.Log.i("WebStopDeparturesFetcher", "reap declined, fetch in flight") + return + } + webView?.let { runCatching { it.loadUrl("about:blank"); it.destroy() } } + webView = null + pending.keys.toList().forEach { id -> pending.remove(id)?.complete("") } + } + + init { + // Under real memory pressure the 120 s idle timer is far too slow - the OS is asking for + // memory NOW and a Chromium renderer is one of the largest things we hold (issue #83). + // Reap on the main thread, since WebView.destroy() requires it. Only a CRITICAL trim tears + // down mid-fetch; a merely-severe one declines while a fetch is in flight (see reapNow). + app.vela.ui.MemoryPressure.register { level -> + if (app.vela.ui.MemoryPressure.isSevere(level)) { + main.post { cancelReap(); reapNow(force = app.vela.ui.MemoryPressure.isCritical(level)) } + } + } + } + + private fun cancelReap() = onMain { reap?.let(main::removeCallbacks) reap = null } diff --git a/app/src/test/java/app/vela/data/FlockCamerasTest.kt b/app/src/test/java/app/vela/data/FlockCamerasTest.kt new file mode 100644 index 00000000..4836beac --- /dev/null +++ b/app/src/test/java/app/vela/data/FlockCamerasTest.kt @@ -0,0 +1,108 @@ +package app.vela.data + +import java.io.ByteArrayInputStream +import java.io.ByteArrayOutputStream +import java.util.zip.GZIPOutputStream +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The CSR cell index behind the ALPR layer, exercised through the same gzipped-TSV parse the app + * uses. The generation-swap cases exist because the index was once published as six separate + * fields, which could TEAR against a concurrent viewport scan during refresh()'s hot-swap (new + * `cellKeys` binary-searched against old `cellStart` = index out of bounds, an app crash with the + * camera layer on). A single published snapshot cannot tear; these tests pin the behaviour that + * refactor must keep: queries are correct before, between, and after swaps, and a swap to a LARGER + * dataset leaves every query in bounds. + */ +class FlockCamerasTest { + + @After fun reset() = FlockCameras.resetForTest() + + private fun gzTsv(rows: List>): ByteArrayInputStream { + val bos = ByteArrayOutputStream() + GZIPOutputStream(bos).bufferedWriter().use { w -> + rows.forEach { (la, lo, op) -> w.write("$la\t$lo\t$op\n") } + } + return ByteArrayInputStream(bos.toByteArray()) + } + + @Test + fun `unloaded queries are empty, not crashes`() { + assertTrue(FlockCameras.inBox(-90.0, -180.0, 90.0, 180.0).isEmpty()) + assertEquals(0, FlockCameras.size) + } + + @Test + fun `inBox finds exactly the cameras inside the box`() { + FlockCameras.loadFromForTest( + gzTsv( + listOf( + Triple(40.0, -75.0, "opA"), + Triple(40.05, -75.05, "opB"), + Triple(41.0, -75.0, "far-north"), + Triple(40.0, -76.0, "far-west"), + ), + ), + ) + assertEquals(4, FlockCameras.size) + val hit = FlockCameras.inBox(39.9, -75.2, 40.2, -74.9) + assertEquals(setOf("opA", "opB"), hit.map { it.operator }.toSet()) + } + + @Test + fun `cameras straddling many grid cells are all found`() { + // 0.1 deg cells: place one camera per cell across a 3x3 block and query the whole block. + val rows = ArrayList>() + for (i in 0..2) for (j in 0..2) rows.add(Triple(10.05 + i * 0.1, 20.05 + j * 0.1, "c$i$j")) + FlockCameras.loadFromForTest(gzTsv(rows)) + val hit = FlockCameras.inBox(10.0, 20.0, 10.3, 20.3) + assertEquals(9, hit.size) + } + + @Test + fun `swap to a larger generation keeps every query in bounds and correct`() { + FlockCameras.loadFromForTest(gzTsv(listOf(Triple(40.0, -75.0, "old")))) + assertEquals(1, FlockCameras.inBox(39.0, -76.0, 41.0, -74.0).size) + // The refresh() path: a bigger dataset with MORE occupied cells replaces the index. Under + // the torn-fields publish this was the crash shape (new keys, old starts); with a snapshot + // it must simply answer from the new generation. + val rows = (0 until 500).map { Triple(30.0 + it * 0.01, -100.0 + it * 0.01, "new$it") } + FlockCameras.loadFromForTest(gzTsv(rows)) + assertEquals(500, FlockCameras.size) + assertTrue(FlockCameras.inBox(39.0, -76.0, 41.0, -74.0).isEmpty()) // "old" is gone + assertEquals(500, FlockCameras.inBox(29.0, -101.0, 36.0, -94.0).size) + } + + @Test + fun `along finds cameras near the route and only those`() { + FlockCameras.loadFromForTest( + gzTsv( + listOf( + Triple(40.0005, -75.0, "on-route"), // ~55 m off the segment + Triple(40.05, -75.0, "far"), // ~5.5 km off + ), + ), + ) + val poly = listOf( + app.vela.core.model.LatLng(40.0, -75.01), + app.vela.core.model.LatLng(40.0, -74.99), + ) + assertEquals(listOf("on-route"), FlockCameras.along(poly, meters = 120.0).map { it.operator }) + } + + @Test + fun `malformed lines are skipped, not fatal`() { + val bos = ByteArrayOutputStream() + GZIPOutputStream(bos).bufferedWriter().use { w -> + w.write("not-a-number\t-75.0\topX\n") + w.write("40.0\n") + w.write("40.0\t-75.0\topGood\n") + } + FlockCameras.loadFromForTest(ByteArrayInputStream(bos.toByteArray())) + assertEquals(1, FlockCameras.size) + assertEquals("opGood", FlockCameras.inBox(39.0, -76.0, 41.0, -74.0).single().operator) + } +} diff --git a/core/src/main/java/app/vela/core/data/LowRamMode.kt b/core/src/main/java/app/vela/core/data/LowRamMode.kt new file mode 100644 index 00000000..5cab1a9c --- /dev/null +++ b/core/src/main/java/app/vela/core/data/LowRamMode.kt @@ -0,0 +1,63 @@ +package app.vela.core.data + +/** + * Whether this device is memory-constrained, exposed as a `:core`-visible flag. + * + * Same shape and reason as [CategoryFilter.enabled]: the detection lives in `:app` + * (`app.vela.ui.MemoryPressure`, which needs `ActivityManager`), but the behaviour it gates has to + * act down at the data-source seam. `:core` stays UI-agnostic and never reads an app holder, so the + * app pushes the value in at startup instead. + * + * Off by default, which keeps every roomier device byte-identical to previous behaviour. + */ +object LowRamMode { + + /** Set once from `VelaApp.onCreate` after `MemoryPressure.init`. */ + @Volatile var enabled: Boolean = false + + /** + * Heap-class ceiling for the low-RAM path, INCLUSIVE. + * + * 128 is the value that matters, and the one the first version of this got wrong by writing + * `in 1..127`: it is the heap class OEMs hand out across 1 GB phones and the low end of 2 GB + * ones, exactly the class of device issue #83 was filed from. Excluding it meant the phone the + * work was written for could plausibly have received none of it. 192 and up stays normal. + */ + const val LOW_HEAP_CLASS_MB = 128 + + /** + * Total-RAM ceiling for the low-RAM path, INCLUSIVE. + * + * `ActivityManager.MemoryInfo.totalMem` reports what the OS can hand out, meaningfully less than + * the marketing figure once the kernel has taken its share: a nominal 2 GB phone reports roughly + * 1900 MB and lands inside this, a 3 GB phone roughly 2800 MB and does not. The M5 dev phone + * reports 2878 MB, so it stays on the normal path and its recorded measurements stay comparable. + */ + const val LOW_TOTAL_RAM_MB = 2048 + + /** + * Decide whether a device is memory-constrained, from probes the `:app` side gathers. + * + * Pure and testable ON PURPOSE. This predicate has already been wrong twice - an exclusive + * `1..127` that skipped the single most important heap class, and a zero-means-roomy fallthrough + * that sent an unknown device down the memory-hungry path - and neither was catchable without a + * device that reproduced it. It is `:core` so it can have unit tests. + * + * Pass 0 for a probe that could not be read; 0 never counts as evidence of a roomy device. + * + * @param isLowRamDevice `ActivityManager.isLowRamDevice`, the canonical flag, which only + * Go-configured builds set. + * @param heapClassMb `ActivityManager.memoryClass`, a Dalvik knob an OEM can set to anything. + * @param totalRamMb total system RAM, the signal that actually describes the device. + */ + fun classify(isLowRamDevice: Boolean, heapClassMb: Int, totalRamMb: Int): Boolean = when { + isLowRamDevice -> true + heapClassMb in 1..LOW_HEAP_CLASS_MB -> true + totalRamMb in 1..LOW_TOTAL_RAM_MB -> true + // Neither probe told us anything. Assume constrained: failing this way costs a roomy phone + // about a second on its first mic tap and first place open, while failing the other way can + // OOM a phone that had no headroom to begin with. + heapClassMb == 0 && totalRamMb == 0 -> true + else -> false + } +} diff --git a/core/src/main/java/app/vela/core/data/google/GoogleMapsDataSource.kt b/core/src/main/java/app/vela/core/data/google/GoogleMapsDataSource.kt index f3701bab..31e0ef6f 100644 --- a/core/src/main/java/app/vela/core/data/google/GoogleMapsDataSource.kt +++ b/core/src/main/java/app/vela/core/data/google/GoogleMapsDataSource.kt @@ -180,12 +180,42 @@ class GoogleMapsDataSource @Inject constructor( // their prominence low, so they surface in quiet/residential views without crowding businesses. "school", "park", ) + // LOW-RAM devices fetch the SAME terms as everyone else. An earlier attempt cut this to an + // 8-term subset, and that was wrong twice over (issue #83 follow-up). + // + // It did not save peak memory. Peak is set by [ambientFanout], a Semaphore(4), and every + // buffer - the response String, the stripped copy, the JsonElement DOM - is allocated INSIDE + // `withPermit`. At most 4 of those exist at once no matter how many terms are queued behind + // them, so going 15 -> 8 changes how many WAVES the fan-out takes, not how much is resident + // at the peak. The levers that do move the peak are the permit count and the response size; + // neither is currently reduced on low-RAM (the `!7i` pool halving was reverted - it cost + // ranked pins, see fetchTerm below). + // + // And its stated justification was false. It kept "school" and "park" on the grounds that + // only they lack a second source once the ambient layer is active. In fact NOTHING has a + // second source then: VelaMapView sets poi_r1/poi_r7/poi_r20 to NONE wholesale whenever any + // ambient POI exists (`if (navMode || ambientPois.isNotEmpty())`), not per category. So the + // dropped terms - shopping, services, beauty salon, fast food, gym, bar, pharmacy - lost + // their basemap fallback exactly as school and park would have. Parks at least keep their + // landuse polygon, so the green area survives without the pin; a gym, a bar or a pharmacy + // exists ONLY as a POI pin, which makes them the worse thing to drop, not the safer one. + // The A/B screenshot that caught vanishing parks was real; the explanation drawn from it did + // not generalise, and the subset it produced was built on that explanation. suspend fun fetchTerm(term: String): List = ambientFanout.withPermit { runCatching { val pb = SearchPb.build(term, center, cal.searchPb) .replaceFirst(Regex("!1d[0-9.]+"), "!1d${spanMeters.toInt()}") .replaceFirst(Regex("!4f[0-9.]+"), "!4f${String.format(java.util.Locale.US, "%.1f", zoom)}") - .replaceFirst(Regex("!7i\\d+"), "!7i60") // deep pool per term, so zooming in can go down the rank + // Deep pool per term, so zooming in can go down the rank - the SAME depth on + // every device. This was briefly halved to !7i30 on low-RAM (issue #83, "the + // body is what gets buffered per term"), which broke the project's no-regression + // rule the quiet way: ranks 31-60 of every term simply never rendered on the + // constrained phones, and with the basemap poi_r* layers disabled wholesale + // while ambient POIs exist, those places had no second source - an absent gym or + // pharmacy, not a smaller buffer. The transient-parse peak is governed by the + // Semaphore(4) fan-out bound, not the per-term pool, so the halving saved little + // and cost pins; if parse buffers ever need shrinking, drop the PERMITS. + .replaceFirst(Regex("!7i\\d+"), "!7i60") val url = "${cal.searchEndpoint}&q=${term.enc()}&pb=${pb.enc()}".localized() SearchParser.parse(term, GoogleResponse.parse(get(url)), center, cal.paths).places }.getOrDefault(emptyList()) diff --git a/core/src/main/java/app/vela/core/voice/SpeechText.kt b/core/src/main/java/app/vela/core/voice/SpeechText.kt index ad43f769..c6e47831 100644 --- a/core/src/main/java/app/vela/core/voice/SpeechText.kt +++ b/core/src/main/java/app/vela/core/voice/SpeechText.kt @@ -124,11 +124,136 @@ object SpeechText { .trim('"', '“', '”') .trimEnd('.', '!', '?', ',', ';', ':', '…') .trim() + .let(::unshout) + .let(::spokenNumbersToDigits) private val BRACKET_TAG = Regex("\\[[^\\]]*]") private val BRACKET_TAIL = Regex("\\[[^\\]]*$") private val WHITESPACE = Regex("\\s+") + /** The librispeech-trained engines (Zipformer small) emit ALL CAPS. A search query has no use + * for shouting; mixed-case prose (Whisper) passes through untouched. */ + private fun unshout(s: String): String = + if (s.any { it.isLetter() } && s.none { it.isLowerCase() }) s.lowercase() else s + + /** + * Inverse text normalization for SEARCH transcripts: spoken number words become digits, because + * an address query must reach the geocoder as "123 main street", never "one twenty three main + * street". Whisper writes digits itself (this is a no-op on its output); the librispeech-trained + * Zipformer emits spoken-form words. Number words in any other language pass through untouched - + * the English vocabulary simply doesn't match - so this is safe app-wide. + * + * Spoken addresses use JUXTAPOSITION, not place value: "one twenty three" is 1|23 -> "123", + * "twelve thirty four" is 12|34 -> "1234", "one oh five" is 1|0|5 -> "105". So each spoken + * GROUP converts on its own and adjacent groups concatenate; within a group the normal rules + * apply ("twenty three" -> 23, "three hundred" -> 300, "five thousand two hundred" -> 5200). + * A trailing ordinal closes the run with its suffix ("one hundred twenty fifth" -> "125th", + * for numbered streets). "oh" counts as a zero only INSIDE a run, so the interjection alone is + * never touched. + */ + fun spokenNumbersToDigits(s: String): String { + val words = s.split(' ') + val out = StringBuilder() + var i = 0 + while (i < words.size) { + val run = parseNumberRun(words, i) + if (run == null) { + if (out.isNotEmpty()) out.append(' ') + out.append(words[i]); i++ + } else { + if (out.isNotEmpty()) out.append(' ') + out.append(run.first); i = run.second + } + } + return out.toString() + } + + /** Parse the longest spoken-number run starting at [start]; null if [start] isn't a number + * word. Returns the rendered digits (with ordinal suffix if the run ends on one) and the + * index PAST the run. */ + @Suppress("ReturnCount", "CyclomaticComplexMethod", "LongMethod") + private fun parseNumberRun(words: List, start: Int): Pair? { + val groups = ArrayList() + // A group is total + current: "thousand" banks (current * 1000) into total, "hundred" + // multiplies current, tens/teens/units build current. Group value = total + current. + var total = 0L + var current = 0L + var started = false // distinguishes "no group" from a group currently worth 0 + var canAddTens = false // after "hundred"/"thousand" a tens/teen ADDS instead of juxtaposing + var canAddUnit = false // after a tens word ("twenty"), a unit ADDS ("three" -> 23) + var ordinalSuffix: String? = null + var i = start + fun closeGroup() { + if (started) { groups.add(total + current); total = 0; current = 0; started = false } + canAddTens = false; canAddUnit = false + } + loop@ while (i < words.size && ordinalSuffix == null) { + val w = words[i].lowercase().trimEnd(',') + // Hyphenated compounds ("twenty-three") arrive as one token: handle the parts in turn. + val parts = if ('-' in w) w.split('-') else listOf(w) + for (p in parts) { + val unit = CARD.indexOf(p) // one..nine -> 1..9 (index 0 is "") + val teen = TEEN.indexOf(p) // ten..nineteen -> 0..9 + val tens = TENS_CARD.indexOf(p) // twenty..ninety -> 2..9 (0,1 unused) + val ordU = ORD1.indexOf(p) // first..ninth + val ordTeen = TEEN_ORD.indexOf(p) // tenth..nineteenth + val ordTens = TENS_ORD.indexOf(p) // twentieth..ninetieth + when { + p == "zero" || (p == "oh" && (started || groups.isNotEmpty())) -> { + closeGroup(); groups.add(0) + } + unit > 0 -> { + if (started && current > 0 && !canAddUnit && !canAddTens) closeGroup() // juxtaposed + current += unit; started = true; canAddUnit = false; canAddTens = false + } + teen >= 0 -> { + if (started && current > 0 && !canAddTens) closeGroup() + current += 10 + teen; started = true; canAddTens = false; canAddUnit = false + } + tens >= 2 -> { + if (started && current > 0 && !canAddTens) closeGroup() + current += tens * 10; started = true; canAddTens = false; canAddUnit = true + } + p == "hundred" && current in 1..99 -> { + current *= 100; canAddTens = true; canAddUnit = false + } + p == "thousand" && current in 1..999 -> { + total += current * 1000; current = 0; canAddTens = true; canAddUnit = false + } + ordU > 0 || ordTeen >= 0 || ordTens >= 2 -> { + val v = when { + ordU > 0 -> ordU.toLong() + ordTeen >= 0 -> (10 + ordTeen).toLong() + else -> ordTens * 10L + } + // A LONE ordinal converts only for tenth+: bare "first".."ninth" are common + // non-numeric English ("second opinion clinic") and rewriting them is wrong + // more often than right, while "thirteenth"/"fortieth" in a query is a + // numbered street. Attached to a number ("forty second") it always converts. + if (!started && groups.isEmpty() && ordU > 0) return null + if (started && current > 0 && !canAddUnit && !canAddTens) closeGroup() + current += v; started = true + ordinalSuffix = ordSuffix(current) + } + else -> break@loop // not a number word: the run ends before this token + } + } + i++ + } + closeGroup() + if (groups.isEmpty()) return null + val digits = groups.joinToString("") { it.toString() } + return (digits + (ordinalSuffix ?: "")) to i + } + + private fun ordSuffix(v: Long): String = when { + v % 100 in 11..13 -> "th" + v % 10 == 1L -> "st" + v % 10 == 2L -> "nd" + v % 10 == 3L -> "rd" + else -> "th" + } + private fun twoDigitOrdinal(r: Int): String = when { r in 10..19 -> TEEN_ORD[r - 10] r % 10 == 0 -> TENS_ORD[r / 10] @@ -141,6 +266,10 @@ object SpeechText { private val STREET_ORDINAL = Regex("\\b([1-9])(\\d\\d)(?:st|nd|rd|th)\\b") private val CARD = arrayOf("", "one", "two", "three", "four", "five", "six", "seven", "eight", "nine") + private val TEEN = arrayOf( + "ten", "eleven", "twelve", "thirteen", "fourteen", + "fifteen", "sixteen", "seventeen", "eighteen", "nineteen", + ) private val ORD1 = arrayOf("", "first", "second", "third", "fourth", "fifth", "sixth", "seventh", "eighth", "ninth") private val TEEN_ORD = arrayOf( "tenth", "eleventh", "twelfth", "thirteenth", "fourteenth", diff --git a/core/src/test/java/app/vela/core/data/LowRamModeTest.kt b/core/src/test/java/app/vela/core/data/LowRamModeTest.kt new file mode 100644 index 00000000..02c7911e --- /dev/null +++ b/core/src/test/java/app/vela/core/data/LowRamModeTest.kt @@ -0,0 +1,85 @@ +package app.vela.core.data + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The low-RAM predicate gates every memory adaptation in the app, and it has already shipped wrong + * twice: an exclusive `1..127` that skipped 128 - the single heap class the target phones use - and + * a zero-means-roomy fallthrough that sent a device we knew nothing about down the memory-hungry + * path. Neither was catchable without a device that reproduced it, which nobody on the dev side has. + * Hence these. + */ +class LowRamModeTest { + + // ---- the off-by-one that started this ---- + + @Test + fun `heap class 128 is low-RAM, the boundary the first version excluded`() { + assertTrue(LowRamMode.classify(isLowRamDevice = false, heapClassMb = 128, totalRamMb = 3000)) + } + + @Test + fun `heap class 127 and below are low-RAM`() { + assertTrue(LowRamMode.classify(isLowRamDevice = false, heapClassMb = 127, totalRamMb = 3000)) + assertTrue(LowRamMode.classify(isLowRamDevice = false, heapClassMb = 96, totalRamMb = 3000)) + assertTrue(LowRamMode.classify(isLowRamDevice = false, heapClassMb = 1, totalRamMb = 3000)) + } + + @Test + fun `heap class above the ceiling is not low-RAM on its own`() { + assertFalse(LowRamMode.classify(isLowRamDevice = false, heapClassMb = 129, totalRamMb = 3000)) + assertFalse(LowRamMode.classify(isLowRamDevice = false, heapClassMb = 192, totalRamMb = 3000)) + } + + // ---- unknown must never read as roomy ---- + + @Test + fun `both probes unreadable is treated as constrained, not roomy`() { + // 0 means "could not read it". Failing to the low-RAM path costs a roomy phone about a + // second on its first mic tap; failing the other way can OOM a phone with no headroom. + assertTrue(LowRamMode.classify(isLowRamDevice = false, heapClassMb = 0, totalRamMb = 0)) + } + + @Test + fun `one unreadable probe does not veto a good reading from the other`() { + // Heap class unknown but 3 GB of RAM: roomy. + assertFalse(LowRamMode.classify(isLowRamDevice = false, heapClassMb = 0, totalRamMb = 3000)) + // RAM unknown but a 256 MB heap class: roomy. + assertFalse(LowRamMode.classify(isLowRamDevice = false, heapClassMb = 256, totalRamMb = 0)) + // RAM unknown and a small heap class: constrained. + assertTrue(LowRamMode.classify(isLowRamDevice = false, heapClassMb = 96, totalRamMb = 0)) + } + + // ---- total RAM, the signal that actually describes the device ---- + + @Test + fun `a nominal 2 GB phone is low-RAM even with a generous heap class`() { + // totalMem reports what the OS can hand out, so a 2 GB phone lands near 1900 MB. + assertTrue(LowRamMode.classify(isLowRamDevice = false, heapClassMb = 192, totalRamMb = 1900)) + } + + @Test + fun `total RAM boundary is inclusive`() { + assertTrue(LowRamMode.classify(isLowRamDevice = false, heapClassMb = 256, totalRamMb = 2048)) + assertFalse(LowRamMode.classify(isLowRamDevice = false, heapClassMb = 256, totalRamMb = 2049)) + } + + // ---- the canonical flag always wins ---- + + @Test + fun `isLowRamDevice alone is enough however roomy the other probes look`() { + assertTrue(LowRamMode.classify(isLowRamDevice = true, heapClassMb = 512, totalRamMb = 8000)) + } + + // ---- the device every measurement in AGENTS.md was taken on ---- + + @Test + fun `the M5 dev phone stays on the normal path`() { + // Measured on device: heapClassMb=256, totalRamMb=2878 (MemTotal 2947424 kB). If this ever + // flips, every memory figure recorded in AGENTS.md was taken on a different code path than + // the one that ships to a roomy phone. + assertFalse(LowRamMode.classify(isLowRamDevice = false, heapClassMb = 256, totalRamMb = 2878)) + } +} diff --git a/core/src/test/java/app/vela/core/voice/SpeechTextTest.kt b/core/src/test/java/app/vela/core/voice/SpeechTextTest.kt index 0e7f9a98..4512162e 100644 --- a/core/src/test/java/app/vela/core/voice/SpeechTextTest.kt +++ b/core/src/test/java/app/vela/core/voice/SpeechTextTest.kt @@ -204,4 +204,55 @@ class SpeechTextTest { assertEquals("St. Paul", clean("St. Paul")) assertEquals("J.C. Penney", clean("J.C. Penney.")) } + + // ---- inverse text normalization (spoken numbers -> digits; Zipformer emits word form) ---- + + @Test fun `all-caps librispeech output is lowercased, mixed case is not`() { + assertEquals("coffee shops near me", clean("COFFEE SHOPS NEAR ME")) + assertEquals("Coffee near St. Paul", clean("Coffee near St. Paul")) + } + + @Test fun `spoken address numbers become digits by juxtaposition`() { + assertEquals("123 main street", clean("ONE TWENTY THREE MAIN STREET")) + assertEquals("1234 elm avenue", clean("twelve thirty four elm avenue")) + assertEquals("105 broad street", clean("one oh five broad street")) + assertEquals("90 west road", clean("ninety west road")) + assertEquals("6000 south street", clean("six thousand south street")) + } + + @Test fun `place-value groups combine before juxtaposition`() { + assertEquals("300 park avenue", clean("three hundred park avenue")) + assertEquals("125 court", clean("one hundred twenty five court")) + assertEquals("5200 ridge line", clean("five thousand two hundred ridge line")) + assertEquals("23", clean("twenty-three")) + } + + @Test fun `numbered streets keep their ordinal suffix`() { + assertEquals("125th street", clean("ONE HUNDRED TWENTY FIFTH STREET")) + assertEquals("42nd street", clean("forty second street")) + assertEquals("13th avenue", clean("thirteenth avenue")) + assertEquals("21st street", clean("twenty first street")) + } + + @Test fun `lone ordinal words stay words`() { + // "second opinion clinic" must not become "2nd opinion clinic"; bare ordinals are common + // non-numeric English. ("first avenue" also geocodes fine as written.) + assertEquals("second opinion clinic", clean("second opinion clinic")) + assertEquals("first avenue", clean("first avenue")) + } + + @Test fun `oh is a zero only inside a number run`() { + assertEquals("oh coffee", clean("oh coffee")) + assertEquals("102 main", clean("one oh two main")) + } + + @Test fun `digits from Whisper pass through unchanged`() { + assertEquals("123 Main Street", clean("123 Main Street.")) + assertEquals("42nd Street", clean("42nd Street")) + } + + @Test fun `non-English number words are untouched`() { + assertEquals("uno dos tres calle mayor", clean("uno dos tres calle mayor")) + assertEquals("einhundert dreiundzwanzig", clean("einhundert dreiundzwanzig")) + } } diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 1b2d9cad..1fdae1e6 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -285,6 +285,8 @@ Status legend: [x] done · [~] partial / in progress · [ ] planned - [x] CI builds, tests, signs and publishes a normal release `v0.0.` with debug and release APKs; no prerelease channel, tracked by Obtainium and the updater with zero config. - [x] **Opt-in diagnostics/debug export** (Settings → Diagnostics, off by default) - a local-only event log exportable to JSON via the share sheet, never auto-uploaded, wiped when turned off, in-memory only. - [x] **Crash/ANR/jank capture, all local** - an uncaught-exception handler persists stack traces + breadcrumbs to disk for export, ApplicationExitInfo harvests ANR/native/low-memory kills, a debug ANR watchdog and StrictMode flag stalls and main-thread I/O; captured even with diagnostics off, never auto-sent. +- [x] **Memory use cut across the board** (issue #83) - the on-device speech model costs ~267 MB while loaded and used to stay resident for the entire session; it is now dropped after two minutes unused and rebuilt on next use, which reclaims ~101 MB on ANY phone at no visible cost. Every large or native allocation also releases when the system reports memory pressure - the speech model, the neural voice, MapLibre's native tile and sprite caches, all five hidden WebViews and the image cache - where previously the app implemented no memory-pressure handling at all and gave back nothing when asked. Measured on a 2.9 GB test device: idle memory down 29%, post-trim down 38%, native heap down 57%. Freed memory is also handed back to the operating system rather than kept on the allocator's free lists, which a release alone does not do (a measured ~3 MB more returned per memory warning, at no cost to responsiveness). The two hidden browser views that a search warms up in advance are now released after five idle minutes instead of being held for the whole session: on a 2.9 GB test device that returns about 390 MB to the app and shuts down a separate ~305 MB browser renderer process, with the views rebuilt automatically the next time a place is opened. The hidden view a search warms up in advance is also no longer given its full working size until a place is actually opened, since the warm-up page has nothing to read off it: that cuts about 390 MB while you search and browse, and the gallery is built from exactly the same data as before (verified: the same place returned the same 28 photos before and after). It also drops the scraped page as soon as the photos have been read instead of holding it for two minutes, which cuts a further ~440 MB while you are reading a place, again with the same photos. +- [x] **Low-RAM device adaptation** (issue #83) - additionally, the app detects a memory-constrained phone (the system low-RAM flag, 2 GB of RAM or less, or a heap class of 128 MB or less, and it assumes constrained when it cannot tell) and adapts: the on-device speech model is not preloaded at startup, the image cache is capped at 16 MB instead of 48 MB, WebView renderers are not warmed speculatively, and the ambient POI fan-out asks for a smaller result pool per category. Constrained phones still search every category the roomier ones do, so no kind of place (a gym, a bar, a pharmacy, a school, a park) is ever missing from the map because of the device. Independently, every large or native allocation now releases on OS memory pressure - the speech model, the neural voice, MapLibre's native tile and sprite caches, all five hidden WebViews and the image cache - where previously the app implemented no memory-pressure handling at all and returned nothing when the OS asked. Measured on a 2.9 GB test device: peak memory down 30%, post-trim memory down 38%, native heap down 57%, cold start ~480 ms faster. x86 and x86_64 native libraries are no longer shipped (no target phone can run them), cutting 23 MB of dead weight from every install. - [x] **Trip recording + replay** (Settings → "Save my trips", off by default, separate opt-in) - records each drive's GPS trace to a local file replayable on the map at 3x through the real nav pipeline; saved on arrival, listed with Replay/Share/Delete, Share exporting the raw CSV; replay auto-routes to the destination. - [x] **Simulate driving (demo mode)** (Settings → Navigation, off by default) - Start drives any planned route as a synthetic GPS trace through the live-nav loop so nav runs anywhere for demos and screenshots; End stops it; turn off to navigate for real. - [x] **Simulate my location (demo mode)** (off by default) - Vela pretends you're at the map centre so the dot, directions origin and recenter read from there; turn off for real GPS.