Skip to content

perf(video-editor): give back the memory that camera and editor sessions kept - #10306

Merged
hm21 merged 6 commits into
mainfrom
perf/release-creation-flow-images
Oct 20, 2026
Merged

hm21 merged 6 commits into
mainfrom
perf/release-creation-flow-images

Conversation

@hm21

@hm21 hm21 commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Description

After a stretch of recording and editing, the app kept a lot more memory than it started with, and leaving the camera or editor did not give it back. After 50 camera → editor → camera round trips a Galaxy S26 held ~500 MB more graphics memory on the home screen, and an iPhone 12 Pro's footprint had grown by ~340 MB without coming back. On phones with less RAM that makes a kill during the next export, or in the background, more likely.

Cause. The memory was Flutter's decoded-image cache. Every editor visit decoded the clip thumbnail twice (full size for the timeline tile fallback, 1080 px for the editor poster) plus timeline strip thumbnails, and those entries stayed after the clips were gone until the cache's 256 MB budget was full. Impeller keeps each decoded image as a GPU texture with a full mip chain, so a full cache measured as about twice its budget in graphics memory. Emptying the cache on Android brought graphics memory straight back to the baseline, so nothing else was holding it.

Fix.

  • The camera route and both editor routes empty the image cache when they close (ReleaseCachedImagesOnDispose). Waiting for the existing trims is not enough: the app empties the cache when it goes to the background or the OS reports memory pressure, but neither happens while the user stays in the app, so a user who records, edits and exports again carries the full cache into the next export. Images still on screen are live and stay.
  • The timeline tile fallback decodes the clip thumbnail at tile height instead of full size. Its comment said it shared a cache entry with the editor poster; the poster decodes at its own height now, so the full-size decode was an extra ~2.8 MB per clip. The regression test that pinned the old behaviour now pins the tile-height decode.

Related Issue: Closes #10301

Out of Scope

  • The cache budget itself: perf(images): size the image cache by RAM and free GPU memory after clearing it #10308 now sizes it by device RAM and draws a frame after every clear (clearDecodedImageCache), which this PR uses when the camera or editor closes.
  • Other places that decode a still at full size for a small tile (stop-motion loop seam, effects bar). They only matter inside an editor session and are freed on close by the change above.

Verification

  • flutter analyze lib test integration_test, dart format, and the affected tests: release_cached_images_on_dispose_test.dart, video_editor_timeline_clip_strip_test.dart, test/router/routes/.
  • Verified on Galaxy S26 and iPhone 12 Pro with profile builds and a scripted run of 50 camera → editor → camera round trips:
Before After
Galaxy S26, GL mtrack on the home screen after the run 743–859 MB (baseline 236–372) 212–331 MB (baseline 240–362)
Galaxy S26, during the run 252 → 738 MB flat at ~257 MB
iPhone 12 Pro, footprint on the home screen after the run 844–853 MB (baseline 725–747) 393–429 MB (baseline 476–556)
iPhone 12 Pro, during the run 492 → 835 MB flat at 424–446 MB
  • By hand on the Galaxy S26: timeline thumbnails are there from the first frame when the editor opens (screen recording, no black tiles), the camera, its library button and the ghost frame load normally after the editor closes, and explore-grid thumbnails that were dropped reload without a visible placeholder when scrolled back to.

Full numbers and the attribution are in #10301.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

After camera and editor sessions the app kept up to ~500 MB more graphics
memory on Android, and on iOS the footprint grew by ~340 MB without
coming back. The holder was Flutter's image cache, filled with thumbnails
of clips that no longer exist until its 256 MB budget was full; Impeller
stores each image with a full mip chain, so the GPU cost is about twice
the budget. On iOS a later clear does not give the footprint back, so the
cache is now emptied each time the camera or editor route closes instead
of relying on background or memory-pressure trims. The timeline tile
fallback also decoded the 640 px clip thumbnail at full size for a 64 dp
tile; it now decodes at tile height.
@hm21 hm21 self-assigned this Oct 19, 2026
@hm21
hm21 marked this pull request as ready for review October 19, 2026 08:22
@hm21
hm21 requested a review from a team as a code owner October 19, 2026 08:22
@hm21
hm21 requested review from NotThatKindOfDrLiz and rabble and removed request for a team October 19, 2026 08:22
@github-actions

This comment has been minimized.

@divine-roborev

divine-roborev Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

roborev: Combined Review (00fcd03)

Verdict: No findings at or above high severity.


Reviewers: 2 done | Synthesis: opencode, 30s | Total: 8m4s

The doc comment said that on iOS a later image-cache clear gave almost
none of the footprint back. A follow-up measurement showed the memory
does come back once Flutter draws its next frame (824 -> 552 MB); the
earlier reading came from a run where no new frame followed. Releasing
on close is still needed because the background and memory-pressure
trims never run while the user stays in the app.
@github-actions

This comment has been minimized.

hm21 added 3 commits October 19, 2026 16:29
…-flow-images

# Conflicts:
#	mobile/lib/constants/app_constants.dart
…r helper

main now has clearDecodedImageCache(), which schedules a frame after the
clear because ImageCache only frees the images it drops once a frame is
drawn. Closing the camera or editor uses it too, so the release does not
depend on the pop animation drawing that frame.
@github-actions

This comment has been minimized.

…-flow-images

# Conflicts:
#	mobile/test/router/routes/video_routes_test.dart
@github-actions

Copy link
Copy Markdown

Mobile PR Preview

Preview refreshed for 00fcd03

Last refresh: 00fcd03 at 2026-10-19 18:57:54 UTC (preview run)

Property Value
Preview URL https://a0bbe7cd.openvine-app.pages.dev
Pages project openvine-app
Preview branch pr-10306
PR branch perf/release-creation-flow-images
Commit 00fcd03

@NotThatKindOfDrLiz NotThatKindOfDrLiz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking code findings. Reviewed the route disposal wrappers, shared cache-clearing helper, live-image retention, thumbnail decode sizing, and regression tests. Analysis and all eight test shards ran successfully on this head; I did not rerun tests locally or reproduce the reported device measurements.

Before merge, record the authoring-model declaration and establish the independent-review provenance required by the shared review policy. This approval records the code-review conclusion and does not waive that requirement.

Agent-Model: openai/gpt-6
Agent-Effort: unknown
Agent-Harness: codex/unknown
Agent-Context: fresh
Initiated-By: @NotThatKindOfDrLiz

@rabble rabble left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No actionable code findings in the complete eight-file diff at 00fcd03. Approved for the code-review conclusion.

Checked the recorder/editor route disposal scopes, shared cache-clearing helper, retention of live images, thumbnail decode sizing, and regression coverage. Actual Analyze and all eight Flutter test-shard execution steps succeeded on this head in Mobile CI run 39306344733. I did not rerun tests locally or reproduce the device memory measurements.

Before merge, the author must add the required authoring-model declaration and establish independent-review provenance. The declaration is absent, so distinct model lineage cannot currently be established; this approval does not waive that requirement. The author retains the merge decision.

Agent-Model: openai/gpt-6
Agent-Effort: unknown
Agent-Harness: codex/unknown
Agent-Context: fresh
Initiated-By: @rabble

@hm21
hm21 added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit 45eaeac Oct 20, 2026
47 of 49 checks passed
@hm21
hm21 deleted the perf/release-creation-flow-images branch October 20, 2026 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

task(perf): find out why the app keeps ~0.5 GB of graphics memory after camera and editor sessions

3 participants