Address various ANR and crashes in 1.0.12#951
Open
davecraig wants to merge 6 commits into
Open
Conversation
There was a race here between LazeColumn item instantation and LaunchedEffect. Moving the title text out of the LazyColumn avoids this. Crashlytics saw a crash at this point.
We still see timeouts when starting the foreground service. This change makes sure that startAsForegroundService is called immediately in onCreate rather than waiting until onStartCommand. This should improve behaviour on slower phones.
There was the possibility of an ANR due to some locking. Each WAV decode was doing resampling so it's a bit more work than just opening a file. Claude took a look and said: The individual decode is not the 5-second cost. I checked the actual earcon files: they range from ~2.6KB up to ~1MB (the largest, 2.4_roadFinder_loop_rev2_wFades.wav, is ~1MB). .wav is in Android's default aapt no-compress list, so these are stored uncompressed in the APK — AAssetManager_open doesn't need to inflate anything, it just locates the entry and returns a pointer. Parsing + float conversion + resampling even the biggest file is low-single-digit milliseconds. So no single WavDecoder call should ever take seconds. What actually stretches this to 5+ seconds is cumulative lock starvation, not one long hold: 1. engineMutex in NativeAudioEngine.kt is a plain Kotlin synchronized(Object) monitor — Java/ART intrinsic locks give no fairness guarantee. A thread that keeps re-acquiring the same lock in a tight loop can keep winning the race against a thread that's been waiting the whole time. 2. speakCallout() doesn't take the lock once for its whole body — it calls createEarcon()/createTextToSpeech() per item in callout.positionedStrings, and each one re-enters its own separate synchronized(engineMutex) block. For a callout describing several POIs, that's many short lock acquisitions back-to-back on thread 35. 3. GeoEngine.startMonitoringLocation's flow re-invokes speakCallout on every location emission. If updates arrive faster than callouts finish (or several nearby POIs trigger a burst of callouts), thread 35 can be in a near-continuous stream of createEarcon/createTextToSpeech calls. 4. Each of those also funnels through AAssetManager_open's own internal native mutex — also unfair — so the same "whoever's already spinning wins" dynamic happens at that layer too. So the main thread isn't blocked on one slow disk read — it's losing dozens-to-hundreds of small, fast races in a row while the worker thread churns through a burst of earcon/TTS creations, and repeated small losses compound past the 5-second ANR threshold. That also matches the trace: thread 35 was caught mid-AAssetManager_open for one specific earcon, but the real damage was already done by however many iterations came before it in that burst. Where this leaves the fix: caching decoded WAVs collapses every repeat play (which is most plays — EARCON_MODE_ENTER/EARCON_MODE_EXIT and the common callout earcons repeat constantly) down to a cache lookup instead of an asset open + parse + resample. That shrinks each synchronized block enough that the main thread should win the lock race quickly even under a burst, rather than losing it repeatedly. It doesn't add fairness to the locks themselves — a sufficiently large burst of first-time (cold-cache) earcon loads could in theory still starve the main thread — but in practice the earcon set is small and fixed, so after the first few seconds of any session everything is warm.
startBeacon was unintentionally happening all on the main thread. Thie change fixes that along with some other beacon control functions which can run asynchronously.
Maplibre can read assets directly and so doesn't need the file copying done by processMaps. processMaps could be time consuming and so this improves start up.
On each new map that is displayed maplibre is opening the font assets and decompressing them. By storing the resources uncompressed it makes this much more efficient.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.