revert(video): two cleanups from the round-4 pass that didn't earn their keep

- GifVideoView: revert the dimensions remember() to the original one-line
  expression. Unlike VideoView's equivalent block, GifVideoView only
  *reads* — there's no MediaAspectRatioCache.add() side effect to gate.
  The replaced code spent three slot reads + three equality checks per
  recompose to skip an int division and an LruCache.get(), neither of
  which allocates. It was a wash at best, a small loss at worst. The
  original is simpler and roughly the same cost.

- PlaybackServiceClient: bump the executor from newSingleThreadExecutor()
  back up to newFixedThreadPool(4). The work per listener is genuinely
  trivial in the steady state, but a single thread leaves us exposed to
  one stuck listener (e.g. the defensive 5s controllerFuture.get()
  timeout actually firing) stalling every other video on screen behind
  it. With a feed often holding several visible videos at once, that's
  a real regression risk. A fixed pool of 4 keeps us bounded against
  churn while letting independent listeners proceed in parallel.
This commit is contained in:
Claude
2026-04-26 14:25:25 +00:00
parent d7bd78cc32
commit 45fb5119e8
2 changed files with 13 additions and 18 deletions
@@ -37,11 +37,14 @@ import kotlin.uuid.Uuid
object PlaybackServiceClient {
// Runs the MediaController.buildAsync() completion callbacks. The work per callback is
// trivial Future.get() on an already-completed future plus a non-blocking trySend into
// the callbackFlow channel so a single thread is plenty. The previous newCachedThreadPool
// could spin up an unbounded number of threads when many videos appeared at once, each
// sticking around for the executor's keep-alive (60s) afterwards.
val executorService: ExecutorService = Executors.newSingleThreadExecutor()
// trivial in the steady state (Future.get() on an already-completed future + a non-blocking
// trySend into this video's own callbackFlow channel), so the IPC bind itself dominates and
// happens on Media3's own threads regardless. We size the pool small enough to stay bounded
// under churn but parallel enough that one stuck listener (e.g. the defensive 5s get()
// timeout actually firing) can't stall the rest of the videos onscreen behind it. The
// original newCachedThreadPool was unbounded and could spin up a thread per concurrent
// video, each lingering for the 60s keep-alive afterwards.
val executorService: ExecutorService = Executors.newFixedThreadPool(4)
fun shutdown() {
executorService.shutdown()
@@ -35,7 +35,6 @@ import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.SideEffect
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
@@ -69,18 +68,11 @@ fun GifVideoView(
accountViewModel: AccountViewModel,
thumbhash: String? = null,
) {
// Keys are primitive width/height so a freshly parsed DimensionTag instance for the same
// event doesn't re-run this lambda — DimensionTag uses reference equality, not structural.
val dimW = dimensions?.width
val dimH = dimensions?.height
val ratio =
remember(videoUri, dimW, dimH) {
if (dimW != null && dimH != null && dimH > 0) {
dimW.toFloat() / dimH.toFloat()
} else {
MediaAspectRatioCache.get(videoUri)
}
}
// Pure read path — DimensionTag.aspectRatio() is a one-line int division and
// MediaAspectRatioCache.get() is a synchronized LruCache lookup. Wrapping this in
// remember() to avoid the recompute would cost more (slot read + N equality checks)
// than the work it saves; that's why this stays as a plain expression.
val ratio = dimensions?.aspectRatio() ?: MediaAspectRatioCache.get(videoUri)
val autoPlay = accountViewModel.settings.autoPlayVideos()
val borderModifier = if (roundedCorner) MaterialTheme.colorScheme.imageModifier else Modifier
val context = LocalContext.current