This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Increase the memory usage estimate for EngineLayer #8700
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Would it be feasible to calculate a more accurate estimate of the size instead of bumping the constant? This looks like an improvement to me generally but there's still going to be cases where this is too low, and extra garbage collection when this is too high isn't ideal. |
Member
Author
|
I don't know of a way to do that accurately. @liyuqian any ideas? |
Contributor
|
I think the main cost may either come from SkPicture or SkImage. I should be able to count those during the |
mklim
approved these changes
Apr 23, 2019
Contributor
mklim
left a comment
There was a problem hiding this comment.
LGTM. This fixes the leak so I think it's worth merging as-is even if we don't have a solution for really returning the correct allocation size yet.
| // Dart GC. The ContainerLayer may hold references to a tree of other layers, | ||
| // which in turn may contain Skia objects. | ||
| return 3000; | ||
| return 200000; |
Contributor
There was a problem hiding this comment.
nit: Please add a TODO here linking to flutter/flutter#31498.
EngineLayers can hold references to Skia objects and may consume significant resources. This change will result in more aggressive cleanup of EngineLayers by the Dart GC. See flutter/flutter#31303
1c7fa36 to
b58e0d8
Compare
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 23, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 24, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 24, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 24, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 25, 2019
engine-flutter-autoroll
added a commit
to flutter/flutter
that referenced
this pull request
Apr 25, 2019
flutter/engine@3e47b4b...26b30a4 git log 3e47b4b..26b30a4 --no-merges --oneline 26b30a4 Roll src/third_party/skia 9adc82c73df0..46d0f9aad1e6 (41 commits) (flutter/engine#8736) fdd8fdb Fix reflective ctor invocation in FlutterFragment (flutter/engine#8735) a56aa95 [scenic] Purge references to Mozart (flutter/engine#8712) e4c439d Fix include paths in libtxt to prepare for upcoming Skia build change (flutter/engine#8723) 7c8ec37 Document that OpacityLayer's children are nonempty (flutter/engine#8707) 30fb4a6 Increase the memory usage estimate for EngineLayer (flutter/engine#8700) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff (liyuqian@google.com), and stop the roller if necessary.
Contributor
|
This appears to have cause dthe regressions in flutter/flutter#31590 |
dnfield
added a commit
that referenced
this pull request
Apr 25, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
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.
EngineLayers can hold references to Skia objects and may consume significant
resources. This change will result in more aggressive cleanup of EngineLayers
by the Dart GC.
See flutter/flutter#31303