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.

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

MenuView performs rebinding/layout/inflation on every click event #61

Open
AttwellBrian opened this Issue Feb 21, 2018 · 0 comments

Comments

Projects
None yet
1 participant
@AttwellBrian

AttwellBrian commented Feb 21, 2018

Overview

Clicking on a menu entry causes the menu's notifyDataSetChanged() to be invoked internally. This causes text appearance to be rebound and a few views to be re-inflated as well. This adds 30-40ms of delay to opening a menu entry on a high end phone. See screenshot of trace. Or open the attached trace file in chrome:tracing.

Note: this trace is based on an internal fork of android that inserts a couple arm instructions in every method call. The tracing overhead is only a few percentage points. So this trace file is trustworthy. But in case you have reasonable doubts, the systrace screenshot shows similar issues & performance overhead.

Reproduction steps

  1. Create app with a MenuView and 10ish menu entries. The Uber app, for example :)
  2. Start your profiler of choice
  3. Click on a menu entry

Version number

Tested on Android 6P with android 7.1.2, with design library version 27.0.2.

Attached

To view the full trace file, download the trace from https://issuetracker.google.com/issues/73723207 where I originally posted this issue. Then open it in chrome:tracing.

vmtrace screenshot
vmtracing_screenshot_menu_click

systrace screenshot
systrace_screenshot_menu_click

AttwellBrian added a commit to AttwellBrian/material-components-android that referenced this issue Feb 21, 2018

Don't rebind/relayout/reinflate any views after clicking a menu item
If none of the check state has changed, let's not update the menu. Background for this diff is at material-components#61. While larger improvements may make sense, this diff is designed to mitigate the most serious issue in a way that doesn't create risk.

Test: used these modified classes inside the Android Uber app and ran a profiler. The problematic 50ms delay caused by this bug was fixed. You can see the before/after traces in https://issuetracker.google.com/issues/73723207.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment