New issue
Advanced search Search tips

Issue 867055 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

ToolbarPhone.onMeasure gets called multiple times unnecessarily

Project Member Reported by mheikal@chromium.org, Jul 24

Issue description

ToolbarPhone.onMeasure gets called at least twice sometimes 4 times with the same parameters in the same android.view.Choreographer#doFrame call tree.

In other words, if toolbar is inflated and added to the activity view hierarchy once, on the next Choreographer#doFrame call, ToolbarPhone.onMeasure is called multiple times with the same parameters.

I am unsure of the cause of this and if this happens to all views or just ToolbarPhone. The only reason I detected this was because ToolbarPhone.onMeasure takes a long time and I was trying to see if it can be improved when I found this.

Attached screenshot of a method trace (captured using Debug.startMethodTracing)
 
B03HnBNmHQg.png
371 KB View Download
Cc: tedc...@chromium.org
Attaching actual trace file. +cc tedchoc@
measure_draw.trace
2.3 MB Download
Labels: android-fe-triaged
Status: Available (was: Untriaged)
I think this is just something that FrameLayout does? In FrameLayout.onMeasure(), I can find a call to measureChildWithMargins() and one to child.measure() for each child, which would result in exponential blowup in the number of nested frame layouts.

Maybe we could return early in ToolbarPhone.onMeasure() if we're being called with the same measure spec as previously?
I think that makes sense, I was not sure why only toolbar_phone is being called 4 times but that is because of the nested FrameLayouts.

So where to memoise? Should we implement our own FrameLayout and memoise there? or should we memoise in the ToolbarLayout level or the ToolbarPhone level?
Owner: mheikal@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2eb57d5b5ea08f7bf15a67056fa03b2b6aabfd4b

commit 2eb57d5b5ea08f7bf15a67056fa03b2b6aabfd4b
Author: Mohamed Heikal <mheikal@chromium.org>
Date: Tue Aug 21 18:30:23 2018

Memoise FrameLayout#onMeasure in OptimizedFrameLayout

FrameLayout#onMeasure calls View#measure twice on each child. Some views, that
have expensive onMeasure implementations, (eg: ToolbarLayout), are nested within
multiple FrameLayouts, thus, the onMeasure function is called 8 times per
layout sweep.

This cl creates a new OptimizedFrameLayout that makes sure to not call measure
on its children more than once with the same measure spec.
ToolbarControlContainer and ViewResourceFrameLayout now extend this class
rather than FrameLayout, thus making layout passes containing these layouts
more efficient.

Improves FCP by 25ms https://pinpoint-dot-chromeperf.appspot.com/job/1590b13a640000

Bug:  867055 
Change-Id: Ic95bfa70aa4238d2f7167f2d75b18041b08675cf
Reviewed-on: https://chromium-review.googlesource.com/1179251
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584850}
[modify] https://crrev.com/2eb57d5b5ea08f7bf15a67056fa03b2b6aabfd4b/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarControlContainer.java
[modify] https://crrev.com/2eb57d5b5ea08f7bf15a67056fa03b2b6aabfd4b/chrome/android/java/src/org/chromium/chrome/browser/widget/ViewResourceFrameLayout.java
[modify] https://crrev.com/2eb57d5b5ea08f7bf15a67056fa03b2b6aabfd4b/ui/android/BUILD.gn
[add] https://crrev.com/2eb57d5b5ea08f7bf15a67056fa03b2b6aabfd4b/ui/android/java/src/org/chromium/ui/widget/OptimizedFrameLayout.java

Status: Fixed (was: Assigned)

Sign in to add a comment