8000 Image animation performance regression on Android · Issue #63876 · flutter/flutter · GitHub
[go: up one dir, main page]
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Image animation performance regression on Android #63876

Closed
deckerst opened this issue Aug 16, 2020 · 11 comments · Fixed by flutter/engine#20673
Closed

Image animation performance regression on Android #63876

deckerst opened this issue Aug 16, 2020 · 11 comments · Fixed by flutter/engine#20673
Assignees
@jason-simmons
Labels
a: images Loading, displaying, rendering images c: performance Relates to speed or footprint issues (see "perf:" labels) c: regression It was better in the past than it is now engine flutter/engine repository. See also e: labels. found in release: 1.20 Found to occur in 1.20 found in release: 1.21 Found to occur in 1.21 has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list

Comments

@deckerst
Copy link
deckerst commented Aug 16, 2020 •
edited
Loading

I noticed a performance regression when switching to the latest stable (1.20). It happens when animating images (for example during hero animation, or when programmatically zooming in/out in something like InteractiveViewer). It is particularly noticeable in debug mode, on low-end devices and with larger images (say >5 MB). In profile mode, performances are way better but we can still observe a regression compared to the previous flutter versions.

Specifically, this engine roll brought the issue: #58751
but I don't know enough about Skia and the engine to dig deeper...

I made a simple repro project:
https://github.com/deckerst/flutter_image_anim_regression

Here's a video record of this repro built with a recent master commit of Flutter (notice the jank), and another record built with Flutter v1.19.0-3.0.pre (smooth like butter). Both are records from debug mode, because it's easier to see, but as I said there is also a regression in profile mode.

It happens with physical Android devices and emulators. I can't say whether it happens on iOS.

edit: here's a timeline taken in profile mode with current Flutter master, on a Sony Xperia Z5 Compact (E5823):
dart_devtools_2020_8_16-1597547958570000.json.zip

Target Platform:
Android

Target OS version/browser:
7.1.1 (API 25), 10 (API 29)

Devices:
Sony Xperia Z5 Compact (E5823), Samsung Galaxy S10e (SM-G970N)

flutter doctor -v

[✓] Flutter (Channel stable, 1.20.2, on Mac OS X 10.15.6 19G73, locale en-KR)
    • Flutter version 1.20.2 at /Users/thibault/code/flutter
    • Framework revision bbfbf1770c (2 days ago), 2020-08-13 08:33:09 -0700
    • Engine revision 9d5b21729f
    • Dart version 2.9.1

 
[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.0)
    • Android SDK at /Users/thibault/Library/Android/sdk
    • Platform android-30, build-tools 30.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jre/jdk/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b3-6222593)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 11.6)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 11.6, Build version 11E708
    • CocoaPods version 1.9.3

[✓] Android Studio (version 4.0)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin version 48.1.2
    • Dart plugin version 193.7361
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b3-6222593)

[!] IntelliJ IDEA Ultimate Edition (version 2020.2)
    • IntelliJ at /Applications/IntelliJ IDEA.app
    ✗ Flutter plugin not installed; this adds Flutter specific functionality.
    ✗ Dart plugin not installed; this adds Dart specific functionality.
    • For information about installing plugins, see
      https://flutter.dev/intellij-setup/#installing-the-plugins

[!] VS Code (version 1.47.3)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    ✗ Flutter extension not installed; install from
      https://marketplace.visualstudio.com/items?itemName=Dart-Code.flutter

[✓] Connected device (2 available)
    • E5823 (mobile)          • CB5A28Q7EL    • android-arm64 • Android 7.1.1 (API 25)
    • sdk gphone x86 (mobile) • emulator-5554 • android-x86   • Android 11 (API 30) (emulator)

! Doctor found issues in 2 categories.
@deckerst deckerst added the from: performance template Issues created via a performance issue template label Aug 16, 2020
@Jiack214
Copy link
Jiack214 commented Aug 16, 2020 •
edited
Loading

I also noticed a similar problem.

I investigated a bit and and I find out that #58751 is the cause (as you said). I went deeper and I found out that more specifically the engine's commit is this: flutter/engine@f46dde1 (in my case, but I think it applies to you too)

I don't use hero animation or InteractiveViewer either, in my case I just render some images and I noticed that comparing 1.19 and 1.20, FPS dropped from 60 to 30 (in debug mode I reach 12 FPS!).

It happens with an old physical Android 7 device and on an Android 10 emulator (I have not tested it elsewhere).

@christopherfujino christopherfujino mentioned this issue Aug 16, 2020
Closed
@itssidhere itssidhere mentioned this issue Aug 17, 2020
Closed
@markusaksli-nc markusaksli-nc added a: images Loading, displaying, rendering images engine flutter/engine repository. See also e: labels. c: performance Relates to speed or footprint issues (see "perf:" labels) c: regression It was better in the past than it is now and removed from: performance template Issues created via a performance issue template labels Aug 17, 2020
@markusaksli-nc
Copy link
Contributor

Reproducible on the latest master 1.21.0-10.0.pre.114. Was only able to reproduce this on my physical SM G950F, did not repro on emulator.

flutter doctor -v 
[√] Flutter (Channel master, 1.21.0-10.0.pre.114, on Microsoft Windows [Version 10.0.19041.450], locale en-US)
    • Flutter version 1.21.0-10.0.pre.114 at C:\Development\flutter_master
    • Framework revision b2c0970aa7 (23 hours ago), 2020-08-16 06:11:02 -0400
    • Engine revision b300be3df3
    • Dart version 2.10.0 (build 2.10.0-34.0.dev)

[√] Android toolchain - develop for Android devices (Android SDK version 30.0.1)
    • Android SDK at C:\Users\marku\AppData\Local\Android\sdk
    • Platform android-30, build-tools 30.0.1
    • Java binary at: C:\Users\marku\AppData\Local\JetBrains\Toolbox\apps\AndroidStudio\ch-0\193.6626763\jre\bin\java
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b01)
    • All Android licenses accepted.

[√] Chrome - develop for the web
    • Chrome at C:\Program Files (x86)\Google\Chrome\Application\chrome.exe

[√] Visual Studio - develop for Windows (Visual Studio Community 2019 16.6.5)
    • Visual Studio at C:\Program Files (x86)\Microsoft Visual Studio\2019\Community
    • Visual Studio Community 2019 version 16.6.30320.27
    • Windows 10 SDK version 10.0.18362.0

[√] Android Studio (version 4.0)
    • Android Studio at C:\Users\marku\AppData\Local\JetBrains\Toolbox\apps\AndroidStudio\ch-0\193.6626763
    • Flutter plugin version 47.1.2
    • Dart plugin version 193.7361
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b01)

[√] Connected device (5 available)
    • SM G950F (mobile)           • ce12171c51cc001c03 • android-arm64  • Android 9 (API 28)
    • sdk gphone x86 arm (mobile) • emulator-5554      • android-x86    • Android 11 (API 30) (emulator)
    • Windows (desktop)           • windows            • windows-x64    • Microsoft Windows [Version 10.0.19041.450]
    • Web Server (web)            • web-server         • web-javascript • Flutter Tools
    • Chrome (web)                • chrome             • web-javascript • Google Chrome 84.0.4147.125

• No issues found!

@markusaksli-nc markusaksli-nc added found in release: 1.21 Found to occur in 1.21 has reproducible steps The issue has been confirmed reproducible and is ready to work on labels Aug 17, 2020
@chinmaygarde chinmaygarde added the P2 Important issues not at the top of the work list label Aug 17, 2020
@erickzanardo erickzanardo mentioned this issue Aug 19, 2020
Closed
@jason-simmons
Copy link
Member

I can reproduce this with the attached example app running on a Moto G4.

flutter/engine@f46dde1 changes the size of a Picture as seen by Dart so that it includes the sizes of any images used by the picture. However, that can lead to double counting of an image's memory usage because multiple pictures can reference the same image object.

In this case, the Hero animation is creating a new Picture on each iteration, but each Picture holds a reference to the same Image instance held in the framework's image cache. Dart thinks each Picture is the size of the image and starts aggressively running the garbage collector and deleting the Pictures. The frequent GCs lead to jank.

The extra GCs are not helping overall heap usage because the real impact of the underlying SkPicture is much smaller, and the Image continues to be held in the cache and is not freed by the GC.

The jank disappears if I remove image_allocation_size from Picture::GetAllocationSize. With that change, the GCs will be deferred until the animation completes.

@dnfield Would it make sense to remove images from Picture::GetAllocationSize? There will be many cases where including image sizes will overstate the memory consumption of a Picture.

@dnfield
Copy link
Contributor
dnfield commented Aug 19, 2020

I'm curious about how well the hint_freed patch I just landed will play with this.

I think it's fine to remove the image from GetAllocationSize, but we still need to track the external allocation size of the picture (including the image) so we can tell the Dart VM how much it might free if it runs a GC now.

@jason-simmons were you able to tell if the GCs were running at idle periods or not?

jason-simmons added a commit to jason-simmons/flutter_engine that referenced this issue Aug 20, 2020
@jason-simmons
Remove image sizes from Picture::GetAllocationSize
17cd7d8 
Including images in the size of a Picture may overstate the picture's
memory usage.  The picture may not be the only object holding a
reference to the image (possibly because the image is owned by the
framework's image cache).

If Dart thinks that the size of the picture includes the image's size,
then Dart will more aggressively garbage collect the picture.  But
deleting the picture will not delete the image, and the extra GCs may
cause jank.

Fixes flutter/flutter#63876
@jason-simmons jason-simmons mentioned this issue Aug 20, 2020
Merged
@glennmichaelmejias
Copy link

is this already fixed?

@christopherfujino
Copy link
Member

is this already fixed?

@glennmichaelmejias landed in the engine in this commit flutter/engine#20673. Not sure if it's rolled to the framework yet.

@christopherfujino
Copy link
Member

@jason-simmons do you think this should be a cherry pick candidate?

@jason-simmons
Copy link
Member

Yes - I think this is having a noticeable effect on performance

@christopherfujino
Copy link
Member

Yes - I think this is having a noticeable effect on performance

Will add the CP request label

@christopherfujino christopherfujino added found in release: 1.19 Found to occur in 1.19 found in release: 1.20 Found to occur in 1.20 cp: 1.20 and removed found in release: 1.19 Found to occur in 1.19 labels Aug 24, 2020
@tvolkert
Copy link
Contributor

FYI, this rolled into the framework in 9ee552d

This was referenced Aug 26, 2020
Merged
Merged
@github-actions
Copy link

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 flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jason-simmons jason-simmons

Labels
a: images Loading, displaying, rendering images c: performance Relates to speed or footprint issues (see "perf:" labels) c: regression It was better in the past than it is now engine flutter/engine repository. See also e: labels. found in release: 1.20 Found to occur in 1.20 found in release: 1.21 Found to occur in 1.21 has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove image sizes from Picture::GetAllocationSize jason-simmons/flutter_engine
10 participants
@chinmaygarde @christopherfujino @dnfield @deckerst @jason-simmons @tvolkert @Jiack214 @glennmichaelmejias @pcsosinski @markusaksli-nc
0