⚡ Bolt: Optimize Jetpack Compose color allocation in VideoSurface - #73
⚡ Bolt: Optimize Jetpack Compose color allocation in VideoSurface#73Shangjin-Xiao wants to merge 1 commit into
Conversation
- Extract statically derived `Color.copy(alpha = ...)` calculations out of `remember` blocks in `VideoSurface.kt`. - `Color` is a value class in Compose. Applying `remember` forces boxing of the primitive into an object to be stored in the Compose slot table, causing unnecessary memory allocation and overhead. - Deriving it directly is just lightweight bitwise math. Co-authored-by: Shangjin-Xiao <84136399+Shangjin-Xiao@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/com/shangjin/frameecho/app/ui/player/components/VideoSurface.kt">
<violation number="1" location="app/src/main/java/com/shangjin/frameecho/app/ui/player/components/VideoSurface.kt:144">
P3: `whiteColor60` doesn't depend on `colorScheme`, so it's recomputed on every recomposition even though its value never changes. Hoist it to a top-level `private val whiteColor60 = Color.White.copy(alpha = 0.6f)` so the `.copy()` runs once instead of on every recomposition, consistent with this PR's allocation-avoidance goal.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| val scrimColor18 = colorScheme.scrim.copy(alpha = 0.18f) | ||
| val inverseSurfaceColor70 = colorScheme.inverseSurface.copy(alpha = 0.7f) | ||
| val inverseSurfaceColor60 = colorScheme.inverseSurface.copy(alpha = 0.6f) | ||
| val whiteColor60 = Color.White.copy(alpha = 0.6f) |
There was a problem hiding this comment.
P3: whiteColor60 doesn't depend on colorScheme, so it's recomputed on every recomposition even though its value never changes. Hoist it to a top-level private val whiteColor60 = Color.White.copy(alpha = 0.6f) so the .copy() runs once instead of on every recomposition, consistent with this PR's allocation-avoidance goal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/com/shangjin/frameecho/app/ui/player/components/VideoSurface.kt, line 144:
<comment>`whiteColor60` doesn't depend on `colorScheme`, so it's recomputed on every recomposition even though its value never changes. Hoist it to a top-level `private val whiteColor60 = Color.White.copy(alpha = 0.6f)` so the `.copy()` runs once instead of on every recomposition, consistent with this PR's allocation-avoidance goal.</comment>
<file context>
@@ -132,13 +132,16 @@ fun VideoSurface(
+ val scrimColor18 = colorScheme.scrim.copy(alpha = 0.18f)
+ val inverseSurfaceColor70 = colorScheme.inverseSurface.copy(alpha = 0.7f)
+ val inverseSurfaceColor60 = colorScheme.inverseSurface.copy(alpha = 0.6f)
+ val whiteColor60 = Color.White.copy(alpha = 0.6f)
LaunchedEffect(tapFeedbackIsPlaying) {
</file context>
⚡ Bolt: 优化 VideoSurface 的 Compose 颜色分配性能
💡 内容 (What)
移除了
VideoSurface.kt中对Color.copy(alpha = ...)使用的remember代码块,改为直接计算派生的颜色值。🎯 原因 (Why)
在 Jetpack Compose 中,
Color是一个内联的 value class(底层包装了一个ULong基本类型)。调用.copy()只是一个极低成本的位运算。但是,当把这个操作放入remember { ... }块中时,由于remember依赖于泛型,会强迫将这个轻量级的 value class 装箱(Boxing)成一个堆内存对象,以便存储到 Compose 的 slot table 中。在像
VideoSurface这样响应高频手势(缩放、平移)导致频繁重组(Recomposition)的组件中,避免非必要的内存分配(Allocation)能显著降低垃圾回收(GC)的压力,减少因为 GC 引起的界面掉帧。📊 影响 (Impact)
消除了由于
remember导致的不必要的颜色对象装箱和 Compose slot table 读写开销。🔬 测量 (Measurement)
可以通过 Android Studio 的 Memory Profiler 在频繁缩放/平移视频时观察堆内存分配情况,分配的临时对象应当有所减少。通过
./gradlew :app:assembleDebug和测试保证代码的正确性未受影响。PR created automatically by Jules for task 14064141893790037513 started by @Shangjin-Xiao
Summary by cubic
Removes
rememberblocks aroundColor.copy()calls inVideoSurfaceso theColorvalue class is no longer boxed and stored in the Compose slot table. Derived colors are now computed directly on each recomposition, cutting unnecessary memory allocation with no behavioral change.Written for commit 4a43620. Summary will update on new commits.