Cap release log level to reduce hitching - #1724
Conversation
235aa80 to
3a9fa5d
Compare
Release builds logged every FINE FINER and FINEST record, and each one runs message censoring and a synchronous file write on the main isolate. The large majority of log volume is this trace, which the user never sees in a release build and which adds up as playback hitching. Keep ALL in debug and cap at INFO in release. Add a Verbose Logging toggle on the logs screen so users reporting a bug can restore full detail on demand. The preference is persisted and applied at startup after settings load, and re-applied immediately when toggled. Also move the Trusted Android user certs log inside the Android only branch so it no longer prints on other platforms.
3a9fa5d to
16b72bc
Compare
|
Look good to me. |
|
I think this is fine to add, yes. Especially if it improves UI performance. Would it be feasible to keep the full logs around without sanitizing and persisting them to "disk" immediately? Maybe we could batch this to improve performance? |
|
I think the most common sort of issue where the logs are really helpful is actual exceptions, which this doesn't affect. A lot of what's being cut is all the sync logger output, which I honestly feel isn't that great - I think I need to redo the logging there to record a different set of information, because the current stuff seems to be frequently useless. I am currently considering a quick pass through everything to check that we're logging messages at an appropriate level. |
|
Two main thoughts on this: Log Utility: While debug logs are definitely valuable, how often are the current verbose logs actually helping us resolve edge cases versus just adding overhead? First Impressions: Finamp and Jellyfin are gaining a lot of momentum right now. Ensuring a smooth, performant first impression for new users should be a priority. Experience is critical for retaining new users. As a returning user, I’m focusing on cleaning up these smaller items, optimizing logging feels like a low hanging win for overall responsiveness. |
Yeah, this has been bugging me as well for a while already, but I haven't had the chance to properly look into it. I frequently check the logs to check playback events and for failed network requests, and the huge number of sync traces makes the logs much harder to read. It might make sense to only log these events if there are problems, and not at all for successful syncs. Thanks for looking into it! |
What
Release builds set the root logger to
Level.ALL, so every FINE, FINER, and FINEST record is emitted in production. Each record runs message censoring plus a synchronous file write on the main. Profiling a short session showed roughly 74 percent of log lines were this verbose trace (SyncBuffer alone emitted over 1400 lines), and that main isolate work shows up as UI or playback hitching.This keeps
Level.ALLin debug and caps release at INFO, so the trace never reaches the record listener in production. A Verbose Logging toggle on the logs screen restores full detail on demand for bug reports. It is persisted and applied at startup after settings load, so it survives a restart (needed to reproduce startup issues) and re-applies immediately when toggled.Also moves the "Trusted Android user certs" log inside the Android only branch so it no longer prints on other platforms.
I am not entirely sure on the logging history here in general, but this seems like a nice low risk change here?
Changes
setup_logging.dart:_levelFor({verbose})returns ALL in debug or when verbose is on, else INFO.applyLogLevel()applies the persisted preference frommainafter Hive is ready.verboseLoggingsetting (default off) with aVerboseLoggingSwitchon the logs screen.Testing
Follows Flutter's build modes guidance: keep expensive diagnostics in debug, not release.