Repository navigation
[flutter_svg] Add a byte budget for the compiled SVG cache - #13596
Conversation
github-actions
Bot
added
p: flutter_svg
triage-engine
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a configurable byte budget for the compiled SVG cache in flutter_svg, defaulting to 32 MiB, along with least-recently-used (LRU) eviction based on both entry count and byte size. It also updates the cache lifecycle management to prevent failed or invalidated pending loads from incorrectly repopulating the cache. Comprehensive unit tests have been added to verify these cache lifecycle and memory budget behaviors. There are no review comments, so I have no additional feedback to provide.
|
@flar can you take a look here please? |
flar
left a comment
flar
left a comment
There was a problem hiding this comment.
Suggestion for a comment, but otherwise LGTM
gaaclarke
left a comment
gaaclarke
left a comment
There was a problem hiding this comment.
This is mostly looking good, I'll refrain from reviewing it until its base PR is landed. Please "rerequest review" after that one lands.
Oh, sorry, I requested a re-review before I saw your note. |
I restored it back to its previous "request changes". |
a41a86c to
a8ee27e
Compare
a8ee27e to
f0f907f
Compare
…3595) Rebased on current main (`8041ccab1`), preserving the failed-load fix and its regression test from #13604. This PR adds invalidation of pending loads on `Cache.clear` and `Cache.evict`: an obsolete completion cannot repopulate the cache or remove a replacement request, even when loaders reuse the same Future. Documents pending-load invalidation on both methods. The cache lifecycle tests live in `cache_test.dart`. The shared-Future regression checks that an evicted request does not populate the cache before the replacement's completion callback runs. Preserves synchronous cache hits, coalescing and entry-count LRU. Bumps the package to 2.3.4 with a changelog entry. Related: flutter/flutter#182486 (failed-load cleanup is already on main via #13604). Validation on Windows: `flutter analyze` clean; full `flutter test` suite: 77 passed, 1 existing skip; formatting and `git diff --check` passed. No golden files changed. Overlapping proposals: #13613 also concerns failed pending loads; main already includes #13604. This PR retains that fix and focuses on clear/evict invalidation. #13596 is stacked on this branch and was rebased with it. ## Pre-Review Checklist This draft was prepared with AI assistance and remains pending the contributor's human review and confirmation of the unchecked checklist items.
gaaclarke
added
the
autosubmit
…lutter#196558) Manually rolls flutter/packages from 14f9933a8cd6 to 015df97cbc3f (5 revisions). This is the same one-line change the [flutter/packages → flutter/flutter autoroller](https://autoroll.skia.org/r/flutter-packages-flutter-autoroll) makes. Today's two automatic rolls, flutter#196527 and flutter#196547, failed on presubmit jobs that don't use the packages pin (`Windows tool_integration_tests_5` and `Linux_android_emu android_engine_opengles_tests`). The roller closed the second one after its roll window ended. It only rolls on weekdays and doesn't support manual rolls, so its next roll would be on Monday. flutter/packages@14f9933...015df97 2026-11-13 [camera_windows] Fall back to temp dir for capture path. (flutter/packages#12755) 2026-11-13 [video_player] Convert event channel to Swift and Pigeon (flutter/packages#13375) 2026-11-13 [material_ui] Document and demonstrate safe MaterialBanner dismissal across routes (flutter/packages#13267) 2026-11-12 [flutter_svg] Add a byte budget for the compiled SVG cache (flutter/packages#13596) 2026-11-12 Roll Flutter from b98afb5 to 9c427b5 (29 revisions) (flutter/packages#13636)
Merged main (14f9933) in d01b5b2, preserving branch history. Main now contains the pending-load changes from #13595. The Files changed tab shows the byte-budget implementation, tests, and release metadata.
Adds a byte budget to the compiled SVG cache: Cache.maximumByteSize defaults to 32 MiB, Cache.currentByteSize reports retained compiled-data bytes, and completed entries obey both the existing count limit and the byte limit using LRU eviction. Oversized results are returned to callers without displacing cached entries. Reducing a limit evicts immediately; zero disables retention. Adds API documentation, regression tests, version 2.4.0 and changelog.
Related memory report: flutter/flutter#184564. This proposal bounds retained compiled cache data; it does not claim to solve the report's peak memory during compilation or memory retained by live widgets.
Validation on Windows: flutter analyze (no issues); complete flutter test suite (84 passed, 1 pre-existing skip); formatting and git diff --check. Tests cover byte/count LRU, changing limits, oversized results, out-of-order completions, and disabling/re-enabling during pending loads. Small platform golden differences are accepted by the existing comparator; no goldens were updated.
Pre-Review Checklist
This draft was prepared with AI assistance and remains pending the contributor's human review and confirmation of the unchecked checklist items.