20% graphics memory increase in latest roll #88745
Comments
xster
added
c: regression
|
It looks like this happens when DisplayList is flipped on: b/197437240#comment9 |
|
Related to flutter/engine#28158 ? |
|
Potentially also a consequence fixed by flutter/engine#28287 |
|
@flar I assigned this to you for now since it looks like it might be related to what you were working on. |
|
@Hixie The information is in the internal report above. See the comment |
|
Per the internal conversation, this is still not resolved. Can we flip the flag back until we figure out a regression test for this and what's going on? |
|
Revert submitted... |
|
In general I agree that a revert is the right thing to do, however I need to reiterate that it is very much appreciated when customers can supply minimal reproducible test cases before asking for reverts and deep investigations. In this case the benchmark in b/197437240 takes 45 minutes to run, which does not meet that bar. |
|
In those 45 minutes it looks like it ends up running the benchmark a couple of hundred times looking for statistical significance which seems overkill. If we can figure out how to set the run count to 3 or 4 then we could get faster turnaround. I do agree that we shouldn't revert on the drop of a hat (or benchmark), though. But initial investigations couldn't turn up a reliable cause and since it's easy to flip the switch, I reverted for now. |
|
(FWIW, we should revert at the drop of a hat. Please see the Tree Hygiene wiki page. Reverting is our default position. It's cheap, it's quick, and it's reliable. Everyone should feel completely happy to revert any commit at any time for even the most tenuous of reasons.) |
|
Jim has a speculative fix for this in flutter/engine#28287. |
|
flutter/engine#28287 did not seem to have any impact. We reverted the flag change pending an investigation into the cause. Even the patch for the memory leak which was indicated in early testing to affect some of the memory consumption turned out not to be reproducible (and we should have been suspect because the leak being fixed was in the wrong type of memory). Right now the only change that seems to fix the memory use is to turn off DL. |
|
Based on #88745 (comment), I'm assuming this shouldn't have been closed by flutter/engine#28287. @flar please correct me if I'm mistaken. |
|
Ah, yes, I guess the bots can't interpret "hopefully fix" as a non-binding reference. |
|
I think I have a probable cause now. I was able to instrument our flutter_gallery demo to save memory statistics and found that our raster cache usage was higher using the DisplayList. This could happen for a couple of reasons, one of which is that we are more likely to cache small pictures, though that doesn't appear to happen in practice. The other reason is that we have a slightly different way to compute the bounds of our display lists and it appears to be more conservative than the Skia algorithms. Patching the raster cache code to run the display list through an SkPicture recorder to get the bounds reduces the cache memory on the flutter_gallery to less than what is used for SkPicture. That's an interesting result and I'll have to test further to see why, but for now it points at a much more probably cause. Patching into the internal test also showed gains that surpassed even the memory efficiency of the standard SkPicture implementation by a wide margin - thus we need to understand why it is less now before we can actually implement a final solution. |
|
Nice work tracking this down!
Understanding where the efficiency comes from sgtm, especially if it exposes opportunities to push things down further. But caching less is only problematic if it results in performance regressions because we're not caching things that should be cached. |
|
There's a service extension that reports raster cache usage. It'd be helpful to know if more raster cache usage was beneficial to frame time. |
|
I semi-broke down the internal automated tests and just ran the app behind the test manually before and after the roll. I can sort of reproduce the issue. Before: After: Looks like the graphics memory went up by 17mb. |
TahaTesser
added
passed first triage
c: regression
|
Enabling the display list has been postponed till Jim investigates the cause. The downstream issue has been marked as fixed. Closing this issue. Please re-open if there are active lines of discussion here. |
|
Jim is going to use this bug to track the ongoing investigation. Re-opening. |
|
We are still investigating this issue because it is blocking enabling the DisplayList by default, but it shouldn't be impacting developers right now as DisplayList is currently disabled. The issue as to why DisplayList uses the extra memory, however is still an ongoing issue so I'm reopening the bug. |
|
It looks like this will be fixed by flutter/engine#28656 |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of |
Internal bug b/197437240
Latest g3 roll cl/392225672 caused a 20% increase in the graphics memory usage.
The list of commits are
Commits in the roll
3dd8160 4092390 Embed OCMock and iOS tests into IosUnitTests app (flutter/engine#27899) 4092390 Embed OCMock and iOS tests into IosUnitTests app 7bbcdc2 Prevent Scrollbar axis flipping when there is an oriented scroll controller bbdd8a1 [ci.yaml] Add gradle cache to devicelab host only targets 5429afd 431ac60 Roll Skia from 8ba1e71a1f59 to 7893d2d0862d (1 revision) (flutter/engine#27910) 431ac60 Roll Skia from 8ba1e71a1f59 to 7893d2d0862d (1 revision) 6b32482 Make kMaterialEdges const 6dfa53b [flutter_conductor] pretty-print state JSON file 972bdf8 Wait for module UI test buttons to be hittable before tapping them 40e8620 Update dart preamble code. 1c01a97 [ci.yaml] Empty commit to verify LUCI configs a694778 c0e59bc fix a typo in https://github.com/flutter/engine/pull/27311 (flutter/engine#27687) c0e59bc fix a typo in https://github.com/flutter/engine/pull/27311 28dfb44 Add native iOS screenshots to integration_test bd04a22 [ci.yaml] Fix firebase recipe, xcode version, and web_benchmarks bffa11e fix: fix BuildableMacOSApp pass no projectBundleId to super error 497eb13 Migrate python invocations to python3 d3bdbed [EditableText] preserve selection/composition range on unfocus 6876c52 [ci.yaml] Add clang to tool integration tests, gems fix 947d3b6 Run --update-packages 8d117c6 Doc: default value for enableInteractiveSelection. 4f51f8e showModalBottomSheet documentation 0893234 Roll Engine from c0e59bc to 4fef55d (12 revisions) 606cb91 Fix wrong EGL value in AndroidEnvironmentGL 0b4396a Prevent a race between SurfaceTexture.release and attachToGLContext 80a09a0 Roll Skia from 7893d2d0862d to c0bfdffe3d53 (1 revision) 8f33539 Roll Dart SDK from 131d357297e2 to 6d96e6063edc (2 revisions) e3a61ca Roll Skia from c0bfdffe3d53 to e53c721d781f (3 revisions) a8368be [web] Fix types of some event listeners 67a157e Roll Skia from e53c721d781f to 134c5f7f690b (12 revisions) 3a59090 Reland enable DisplayList by default 015b680 Roll Skia from 134c5f7f690b to 462e18821630 (1 revision) d7cea9f Roll Skia from 462e18821630 to aef5dc78f38a (1 revision) 95c7ed2 fix crash SemanticsObject dealloc and access the children 4fef55d Roll Skia from aef5dc78f38a to 94fcb37e5b4f (15 revisions) 5792c73 Use Device specific gesture configuration for scroll views 90d845c Updated skipped tests for scheduler directory. 271ed8b Updated skipped tests for flutter_test directory. 4c7af3b Fix compile error in SliverAppBar sample code 11a0284 Reland "update ScrollMetricsNotification" e2fb5cc Updated skipped tests for widgets directory. 7d76a82 Updated skipped tests for services directory. 724c0eb Changing ElevatedButton.child to be non-nullable 1ca0333 Overridable action 66597ff Fixed mouse cursor of disabled IconButton a628e4f `showModalBottomSheet` should not dispose the controller provided by user b08366c Updated skipped tests for flutter_tools. 41ff30c Added a check to the analyzer script to detect skipped tests. 91f8b6b Revert "Revert "Restores surface size in the postTest of test binding"" 1c6d263 Roll Engine from 4fef55d to d8bbebe (45 revisions) e4a8d23 Implementation of two or more threads merging for multiple platform views ea4b353 Fix routerinformationupdate can take complex json e0ebbb2 Roll Fuchsia Mac SDK from rQOi2N8BM... to XuRmqkWX2... 6291be0 Roll Dart SDK from 6d96e6063edc to cf20a69c75cf (2 revisions) 1bb8627 Roll Fuchsia Linux SDK from q6H_ZE5Bs... to wX-ifEGo5... cabe67c Fix the SurfaceTexture related crash by Replacing the JNI weak global reference with a WeakReference 532a5ca Roll Skia from 94fcb37e5b4f to ca13a3acc4b2 (10 revisions) 613490f Fix sample analyzer errors in text.dart 2b5b412 Roll Dart SDK from cf20a69c75cf to 749ee4e9e053 (1 revision) ceac55d Roll Fuchsia Mac SDK from XuRmqkWX2... to 6XsAe3sPr... be6865b Roll Fuchsia Linux SDK from wX-ifEGo5... to yZ4KOzDE1... 51764ef Roll Skia from ca13a3acc4b2 to 6bc126d24d1b (1 revision) 35dfc27 Do not call CoreWindow::Visible from raster thread b9965ec Roll Fuchsia Mac SDK from 6XsAe3sPr... to aDswwwLbx... 9bdcd8a Roll Fuchsia Linux SDK from yZ4KOzDE1... to GBIahjoek... aa4992b Roll Skia from 6bc126d24d1b to a28795fd64a4 (2 revisions) 9ffd0af Roll Fuchsia Mac SDK from aDswwwLbx... to iVa6afHb7... 3f8910f Roll Fuchsia Linux SDK from GBIahjoek... to TK8mj-iQr... 4607bc3 [ci.yaml] Remove deprecated builder field d0ee553 Roll Skia from a28795fd64a4 to d27a0d39cea2 (1 revision) 74f162a Roll Fuchsia Mac SDK from iVa6afHb7... to 0iDbdLRq3... 710d94e Roll Fuchsia Linux SDK from TK8mj-iQr... to D6VF_8x5s... 56dba3b Roll Skia from d27a0d39cea2 to fdf7b3c41f8e (1 revision) d07acf9 Roll Skia from fdf7b3c41f8e to 717ef9472b56 (1 revision) 7429ad7 Roll Skia from 717ef9472b56 to ad5944cf8dbb (7 revisions) facce47 UWP: Fix uwptools that does not uninstall package 7476b03 Roll Fuchsia Mac SDK from 0iDbdLRq3... to CYrOc5v2a... a6ce951 Fix memory leak in PlatformViewsController 816a1ec Roll Fuchsia Linux SDK from D6VF_8x5s... to 78gBCb4IK... 6f8134e Windows host builds don't need android SDK 266435d Support iOS universal links route deep linking 3ee19b4 Fix: modifier keys not recognized on macOS Web 2fa5f69 Fix StandardMessageCodec test leaks 44f5f4b Use runtime checks for arguments, out directory f88aea6 Roll Dart SDK from 749ee4e9e053 to defd2ae02a3f (2 revisions) c2f2291 Eliminate Mac Android Debug Engine shard a1fd1e1 Avoid crashing when FlutterImageView is resized to zero dimension 734f1c8 Roll Dart SDK from defd2ae02a3f to 2ca55ab863e9 (1 revision) 8f272d2 Roll buildroot to c37164f39300c6e42560a5853e16a919688f7b86 c291389 Roll Fuchsia Mac SDK from CYrOc5v2a... to 65ocmUBj5... 4d675db Roll Dart SDK from 2ca55ab863e9 to 0e1f5a68c69f (1 revision) 05988b4 Roll Dart SDK from 0e1f5a68c69f to d68dff4d0820 (1 revision) e72c319 Roll Skia from ad5944cf8dbb to 23d8f9453581 (17 revisions) 7ec35b0 Roll Skia from 23d8f9453581 to d89d445dea5e (10 revisions) d8bbebe [web_ui] Fixed aria-live attribute on web 4b53875 [ci.yaml] Add roller 8d89632 Notification doc fixes d8da091 Android 12 overscroll stretch effect cb65135 Fix errors in examples 4ad8c15 Make no-response plugin no-op for forks. 4ef8088 feat: migrate macos/application_package.dart to null-safety d2f9c6c feat: migrate base/dds.dart to null-safety 6400913 Roll Engine from d8bbebe to 4daceb9 (9 revisions) 6f36d7b Roll Dart SDK from d68dff4d0820 to 081dad8fea3e (1 revision) 5541f1d Engine build configuration for web engine. f9d717c Roll Fuchsia Mac SDK from 65ocmUBj5... to KL3oDHUYH... e1580d4 Roll Skia from d89d445dea5e to b5de6be2a85d (21 revisions) e7fe754 Roll Dart SDK from 081dad8fea3e to 32238a66b5ca (1 revision) 96939be Roll Fuchsia Mac SDK from KL3oDHUYH... to Gou0gs-Pc... 3a46ace Roll Skia from b5de6be2a85d to ae43303ce5fd (8 revisions) d53aa58 kick builds 4daceb9 Python 3 compatibility fix for running Java tests using run_tests.py e127665 Update MaterialScrollBehavior.buildScrollbar for horizontal axes 871386b Revert "Changing ElevatedButton.child to be non-nullable" 0340319 Roll Engine from 4daceb9 to a2e6047 (10 revisions) b681779 [canvaskit] Release overlays to the cache once they have been used cab494a Roll Skia from ae43303ce5fd to ed7b4f6a237d (12 revisions) d0adcdd Roll Skia from ed7b4f6a237d to b65b4da55418 (2 revisions) c02d9be Provide Open JDK 11 3373d2d Bump buildroot version 05c819f Roll Dart SDK from 32238a66b5ca to 624de6ea9e59 (4 revisions) 833b4f1 ui.KeyData.toString 6215e29 Windows keyboard integration tests and fix a number of issues 75933d4 Roll Skia from b65b4da55418 to b7f2215bbb50 (7 revisions) a2e6047 Roll Fuchsia Linux SDK from 78gBCb4IK... to DuaoKMSCx... 10e4b04 Switch document generation to use the snippets package a17723d feat: migrate windows/application_package.dart to null-safety 9e9ca69 Home/End key support for Linux 047d35d feat: refactor IosProject and MacOSProject to extends XcodeBasedProject to share common 16b5af9 06da8b0 Fix getLineBoundary for TextAffinity (flutter/engine#28008) 06da8b0 Fix getLineBoundary for TextAffinity 9b1ed46 Update flutter create templates for Xcode 13 9764d86 Support flutter create --platform singular flag df34960 Roll Engine from 06da8b0 to 8bf3cc6 (6 revisions) 27e90b8 Roll Dart SDK from 624de6ea9e59 to 8e3be460559f (1 revision) cd24385 Revert '[fuchsia] Make dart_runner work with cfv2' d5fa510 Roll Fuchsia Mac SDK from Gou0gs-Pc... to j0tWnUn2K... 9db59a9 Roll Fuchsia Linux SDK from DuaoKMSCx... to G1rAOfmj_... 01bb42d Roll Skia from b7f2215bbb50 to ec07af1279ec (8 revisions) 8bf3cc6 Update builder configurations to apply the new generators format. e1485e9 670a681 Fix some warnings seen after the migration to JDK 11 (flutter/engine#28045) 670a681 Fix some warnings seen after the migration to JDK 11 5848a16 [gen_l10n] to handle arbitrary DateFormat patterns f0e832c [flutter_tool] Fix DesktopLogReader to capture all output 7bedd0f Fix incorrect logging. 758c28b [flutter_tools] Reland "Make upgrade only work with standard remotes" 5b5eaf0 add platform tags fbc4e9b Revert "Update MaterialScrollBehavior.buildScrollbar for horizontal axes" 6e224ee [EditableText] call `onSelectionChanged` only when there're actual selection/cause changes b860982 remove _AbortingSemanticsFragment 2a42069 Bump snippets to 0.2.3, fix redundant global activate in docs.sh 4bd8b28 ScaleUpDetails focalPointDelta bf3ff7e feat: migrate fuchsia/application_package.dart to null-safe 93d6954 Fix AnimatedCrossFade would focus on a hidden widget 85297c0 b324876 Roll Skia from ec07af1279ec to 1fc789943486 (8 revisions) (flutter/engine#28048) b324876 Roll Skia from ec07af1279ec to 1fc789943486 (8 revisions) 91e0759 Bump to Gradle 7 and use Open JDK 11 79ff8a5 Revert "Bump to Gradle 7 and use Open JDK 11" 10aad08 add ? 325cfb0 Make doctor Xcode version requirement clearer c5d5463 [flutter_tools] Fix hang in DesktopLogReader 72a1e65 Adding vs_build dependency to Windows hot_mode_dev_cycle_win_target__benchmark b3d12eb Revert "[EditableText] call `onSelectionChanged` only when there're actual selection/cause changes" bfe4759 Roll Engine from b324876 to 6227940 (17 revisions) e3207a4 Roll Dart SDK from 8e3be460559f to 7b142a990f2b (2 revisions) 777f539 Roll Skia from 1fc789943486 to c01225114a00 (1 revision) 9cefdf8 added python version note to the luci script d856fc3 Enable Dart compressed pointers for 64-bit mobile targets 56cf819 Sets focus before sending a11y focus event in Android ee8ca2b Roll Skia from c01225114a00 to ab005016f9aa (2 revisions) 8732428 Roll Dart SDK from 7b142a990f2b to 3272e5c97914 (2 revisions) 4b05f54 Windows: Add multi-touch support 3f1c479 Roll Fuchsia Linux SDK from G1rAOfmj_... to aRsjkcQxJ... c00a89f Roll Skia from ab005016f9aa to 801e5f7eb9b7 (2 revisions) 8e58276 Roll Skia from 801e5f7eb9b7 to 35371bd92f10 (2 revisions) b87e626 Fix dead key crashes on Win32 b72caee Roll Fuchsia Mac SDK from j0tWnUn2K... to qyqXnccT8... af08256 Roll Dart SDK from 3272e5c97914 to eb7661b63527 (1 revision) 1fe5b16 GN build rules for tests using Fuchsia SDK Dart libraries and bindings b288e8a Roll Skia from 35371bd92f10 to 68e4e20fae84 (6 revisions) 6227940 Roll Dart SDK from eb7661b63527 to 481bdbce9a92 (1 revision) 949f4f0 Roll Plugins from 6010090 to c8570fc (17 revisions) 195e7de Revert "Roll Engine from b324876 to 6227940 (17 revisions)" eb1d1ce Roll Plugins from c8570fc to 3ae3a02 (3 revisions) 1422bfa Roll Engine from b324876 to a447901 (30 revisions) e3207a4 Roll Dart SDK from 8e3be460559f to 7b142a990f2b (2 revisions) 777f539 Roll Skia from 1fc789943486 to c01225114a00 (1 revision) 9cefdf8 added python version note to the luci script d856fc3 Enable Dart compressed pointers for 64-bit mobile targets 56cf819 Sets focus before sending a11y focus event in Android ee8ca2b Roll Skia from c01225114a00 to ab005016f9aa (2 revisions) 8732428 Roll Dart SDK from 7b142a990f2b to 3272e5c97914 (2 revisions) 4b05f54 Windows: Add multi-touch support 3f1c479 Roll Fuchsia Linux SDK from G1rAOfmj_... to aRsjkcQxJ... c00a89f Roll Skia from ab005016f9aa to 801e5f7eb9b7 (2 revisions) 8e58276 Roll Skia from 801e5f7eb9b7 to 35371bd92f10 (2 revisions) b87e626 Fix dead key crashes on Win32 b72caee Roll Fuchsia Mac SDK from j0tWnUn2K... to qyqXnccT8... af08256 Roll Dart SDK from 3272e5c97914 to eb7661b63527 (1 revision) 1fe5b16 GN build rules for tests using Fuchsia SDK Dart libraries and bindings b288e8a Roll Skia from 35371bd92f10 to 68e4e20fae84 (6 revisions) 6227940 Roll Dart SDK from eb7661b63527 to 481bdbce9a92 (1 revision) 3f9e4a5 Roll Skia from 68e4e20fae84 to 9032cc61bbe2 (6 revisions) dbae80e Roll Fuchsia Linux SDK from aRsjkcQxJ... to WoiEzs7XB... 352da9c Roll Skia from 9032cc61bbe2 to 73339ada9d13 (1 revision) 057c49c [web] add CanvasKit to CIPD; make it a DEPS dependency; add a manual roller script 41ca6d4 Add asan flag to run_tests.py aa9ce6c Roll Skia from 73339ada9d13 to 364ea352b002 (3 revisions) fbaf9e8 [web] rename watcher.dart to pipeline.dart dfcb48c Roll Fuchsia Mac SDK from qyqXnccT8... to DV0pzjjZ4... 872b0de Roll Dart SDK from 481bdbce9a92 to 3e8633bcbf8f (2 revisions) 5e515af [web] Don't reset history on hot restart 18a40b2 Roll Skia from 364ea352b002 to 1049d8206120 (3 revisions) 5cc54f5 Roll Dart SDK from 3e8633bcbf8f to bedc39b7a80b (1 revision) a447901 Revert "Enable Dart compressed pointers for 64-bit mobile targets" 338ade8 Roll Engine from a447901 to 1af0a20 (4 revisions) c388bfa Roll Skia from 1049d8206120 to abe39f5cb932 (6 revisions) 5ba9ed5 [web] Clean up legacy Paragraph implementation b1ccc41 Revert "Sets focus before sending a11y focus event in Android" 1af0a20 Roll Fuchsia Linux SDK from WoiEzs7XB... to 54i7Z2kLS... 85ffa8c [Fonts] Improved icons update script 06be7e5 99c5f61 Move test packages from `dependencies` to `dev_dependencies` (flutter/plugins#4231) fe337dc Animation controller test e7f9bcb Removed no-shuffle tag and fixed leak in debug_test.dart 31d0787 Revert "Revert "Revert "Restores surface size in the postTest of test binding""" 57efbd5 Rename IOSDeviceInterface to IOSDeviceConnectionInterface 4d4f337 Marks Linux test_ownership to be unflaky a40c5c2 Migration text selection manipulation. 3216e9f Roll Engine from 1af0a20 to 9d33093 (8 revisions) 2718a1a Roll Fuchsia Mac SDK from DV0pzjjZ4... to a27NGktH6... 5bc8a0e Roll Skia from abe39f5cb932 to 72171b75ed20 (1 revision) 656dfe9 Roll Skia from 72171b75ed20 to b1f34bf3c2cb (1 revision) e9497e8 Prevent leak of a closable render surface 93cec54 Roll Skia from b1f34bf3c2cb to 6b9c7bb81419 (1 revision) 4d47174 Roll Skia from 6b9c7bb81419 to fc3bee1232fa (1 revision) cf012c8 Roll Skia from fc3bee1232fa to 7b2af606d352 (1 revision) 9d33093 Roll Skia from 7b2af606d352 to 55ca4e692d8f (1 revision) af4faf4 Microtasks Animations isn't flaky c49eba6 [TextInput] minor fixes a4b724d fix a scrollbar updating bug 51353b5 Roll Engine from 9d33093 to 7dc8eba (4 revisions) 1b52010 roll buildroot 35ad0f6 Roll Skia from 55ca4e692d8f to 9a6f3990aff7 (6 revisions) fa05362 Build dart:zircon and dart:zircon_ffi 7dc8eba [ci.yaml] Add default properties and dimensions e761e64 Add theme support for choosing android overscroll indicator ddb8bfb Fix URL construction in the test entry point generated by the web bootstrap script db64aed partial revert of gesture configReducing the amount of commits per roll is a goal (though there are technical impediments). In the meantime, we should aim to have equivalent coverage outside google3 for memory usage including graphics.
The text was updated successfully, but these errors were encountered: