fix: Remove static view caching to improve memory management #548
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The static
ReusableViewCache
s used inListProperties
would cause views to be retained beyond the lifetime of the owning list and if the views themselves referenced other objects (directly or through closures) then large parts of application state could be inadvertently "leaked" for the lifetime of the application.This change instantiates new caches every time
PresentationState
is instantiated. There is a measuring performance impact since each reuse identifier will need to be recreated once per layout. On a long homogenous list that probably isn't a big deal, but for a heterogenous list it could be more of an issue (in the extreme case of a list composed of unique cells for every row this caching would provide no benefit). I haven't done any formal profiling on this.Ideally we'd be able to associate the caches to the list views themselves so that their lifetimes were bound together. That would help avoid many of the memory issues we've seen while improving the cache effectiveness.
Checklist
Please do the following before merging:
Main
section.