1104151 - Attention indicator causes b2g main thread 10%-20% usage while in a Loop call.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Sotaro Ikeda [:sotaro]]

+++ This bug was initially created as a clone of Bug #1097235 +++

From Bug 1097235 Comment 38, attention indicator consumes a lot of b2g main thread's cpu time. The attention indicator is implemented as css animation.

From the analysis, the css animation are work as async animation. The async animation move the animation task to compositor side, therefore, b2g main thread task should be reduced to low cpu usage.

Comment 1

[Sotaro Ikeda [:sotaro]]

b2g main thread's profile.
http://people.mozilla.org/~bgirard/cleopatra/#report=a93ff06eabc2c43134425283e0b4a866ac00c601

Comment 2

[Sotaro Ikeda [:sotaro]]

The following is HTML of Attention Indicator.

>      return '<div id="attention-indicator" class="attention-indicator" ' +
>              'data-z-index-level="attention-indicator">' +
>              '<div class="handle handle1"></div>' +
>              '<div class="handle handle2"></div>' +
>              '<div class="handle handle3"></div>' +
>            '</div>';

https://git.mozilla.org/?p=releases/gaia.git;a=blob;f=apps/system/js/attention_indicator.js;h=cf1c45e97fc17cb205096f1927e7ed8db8bdf087;hb=refs/heads/v2.1#l32

Comment 3

[Sotaro Ikeda [:sotaro]]

CSS of Attention Indicator.

https://git.mozilla.org/?p=releases/gaia.git;a=blob;f=apps/system/style/attention_indicator.css;h=67cefb5ee3e7b55d90532e287637ffa7eabf6112;hb=refs/heads/v2.1

Comment 4

[Sotaro Ikeda [:sotaro]]

When AsyncAnimationFailure log is enabled, logcat log did not have the animation error log.

Comment 5

[Sotaro Ikeda [:sotaro]]

b2g main thread was busy, because animation's throttling did not happen. AnimationPlayerCollection::CanThrottleAnimation() return false because FrameLayerBuilder::GetDedicatedLayer() return nullptr.

http://dxr.mozilla.org/mozilla-central/source/layout/style/AnimationCommon.cpp#804
http://dxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp#4138

Comment 6

[Sotaro Ikeda [:sotaro]]

In FrameLayerBuilder::GetDedicatedLayer(), the animated frame did not have LayerManagerDataProperty.

Comment 7

[Sotaro Ikeda [:sotaro]]

From FrameLayerBuilder::DisplayItemData::AddFrame(), all frames that are used in DisplayList should have LayerManagerDataProperty. Therefore, the animated frame seems not be used in DisplayList.

http://dxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp#75

Comment 8

[Sotaro Ikeda [:sotaro]]

In Attention Indicator, handle3 causes the problem. When I changed the handle3's attribute to "display: none" or "margin: -0.4333rem auto", the problem was fixed.

Comment 9

[Sotaro Ikeda [:sotaro]]

From comment 8, it seems like that the following attribute triggers the problem.

> margin: -0.5333rem auto;

Comment 10

[Sotaro Ikeda [:sotaro]]

The handle3's computed attributes are following. They are got by using Firefox WebIDE. The elements height is 2.66667px, but margin-top is -5.33333px. Element seems to exist off screen.

------------------------------------
animation-delay 0.666s
animation-direction normal
animation-duration 1s
animation-fill-mode none
animation-iteration-count infinite
animation-name handle
animation-play-state running
animation-timing-function cubic-bezier(0.25, 0.1, 0.25, 1)
background-attachment scroll
background-clip border-box
background-color #B2F2FF
background-image none
background-origin padding-box
background-position 0% 0%
background-repeat repeat
background-size auto auto
color #FFF
font-size 10px
font-weight 500
height 2.66667px
margin-bottom -5.33333px
margin-left 0px
margin-right 0px
margin-top -5.33333px
width 86px

Comment 11

[Sotaro Ikeda [:sotaro]]

DisplayItem is created by sIFrame::BuildDisplayListForChild(), the handle3's DisplayItem creation was failed in the following code, because, it does not have valid area. Both the dirty area and child frame area were (x,y,w,h)=(0,0,0,0).

    // We can stop if child's frame subtree's intersection with the
    // dirty area is empty.
    // If the child is a scrollframe that we want to ignore, then we need
    // to descend into it because its scrolled child may intersect the dirty
    // area even if the scrollframe itself doesn't.
    // There are cases where the "ignore scroll frame" on the builder is not set
    // correctly, and so we additionally want to catch cases where the child is
    // a root scrollframe and we are ignoring scrolling on the viewport.
    nsIPresShell* shell = PresContext()->PresShell();
    bool keepDescending = child == aBuilder->GetIgnoreScrollFrame() ||
        (shell->IgnoringViewportScrolling() && child == shell->GetRootScrollFrame());
    if (!keepDescending) {
      nsRect childDirty;
      if (!childDirty.IntersectRect(dirty, child->GetVisualOverflowRect()))
        return;
      // Usually we could set dirty to childDirty now but there's no
      // benefit, and it can be confusing. It can especially confuse
      // situations where we're going to ignore a scrollframe's clipping;
      // we wouldn't want to clip the dirty area to the scrollframe's
      // bounds in that case.
    }
  }

http://dxr.mozilla.org/mozilla-central/source/layout/generic/nsFrame.cpp#2323

Comment 12

[Sotaro Ikeda [:sotaro]]

From comment 11, Attention Indicator's handle3 is not rendered at all, just adds b2g main thread's cpu usage.

Comment 13

[Sotaro Ikeda [:sotaro]]

By the investigation, the following becomes clear. This is a gecko's defect. Talked with mstange and BenWa about this problem. It is not easy to fix on b2g v2.1. For b2g v2.1, it is better to fix the problem by changing gaia side.

- When css animated element is positioned off screen, the element is not rendered at all, but it requests to render by 60fps.

Comment 14

[Sotaro Ikeda [:sotaro]]

The problem could be fixed by either ways.
- [1] Remove Attention Indicator's handle3
- [2] Change Attention Indicator's handle3 position from offscreen to on screen.

Comment 15

[Sotaro Ikeda [:sotaro]]

[Blocking Requested - why for this release]: This bug increase b2g main thread's cpu usage. It could affect to heavy cpu usage applications like WebRTC.

Comment 16

[Sotaro Ikeda [:sotaro]]

Changed the component to Gaia::System. For b2g v2.1, it is better to fix the problem by gaia side.

Comment 17

[Sotaro Ikeda [:sotaro]]

Bug 1104151 is created for gecko's side bug.

Comment 18

[Gregor Wagner [:gwagner]]

(In reply to Sotaro Ikeda [:sotaro] from comment #17)
> Bug 1104151 is created for gecko's side bug.

I guess you mean bug 1104726 is for the gecko side?

Comment 19

[Gregor Wagner [:gwagner]]

We need to figure out how much risk we are willing to take here before we can start working on it since it will be a 2.1 only patch.

Comment 20

[Michael Henretty [:mikehenrty][:mhenretty]]

Alberto, please take a look at comment 14 and see if this is an easy fix for us.

Comment 21

[Alberto Pastor [:albertopq]]

It seems that 'handle3' was removed in master:

https://github.com/mozilla-b2g/gaia/commit/25452e1f5e22c7ab99d30817d842e66df5e435fc

Can we try to uplift that? Alive, the bug number in the commit doesn't seem to match. Do you think is save to uplift that patch? Thanks!

Comment 22

[Sotaro Ikeda [:sotaro]]

(In reply to Gregor Wagner [:gwagner] from comment #18)
> (In reply to Sotaro Ikeda [:sotaro] from comment #17)
> > Bug 1104151 is created for gecko's side bug.
> 
> I guess you mean bug 1104726 is for the gecko side?

Yes :-) sorry for my mistake.

Comment 23

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Alberto Pastor [:albertopq] from comment #21)
> It seems that 'handle3' was removed in master:
> 
> https://github.com/mozilla-b2g/gaia/commit/
> 25452e1f5e22c7ab99d30817d842e66df5e435fc
> 
> Can we try to uplift that? Alive, the bug number in the commit doesn't seem
> to match. Do you think is save to uplift that patch? Thanks!

It is https://bugzilla.mozilla.org/show_bug.cgi?id=1072616 and fabrice deny my request :/
If the patch is helping with the performance here could you ask approval again?

Comment 24

[Alberto Pastor [:albertopq]]

:sotaro, could you please double check that the patch in Bug 1072616 fixes this performance issue? Thanks!

Comment 25

[Sotaro Ikeda [:sotaro]]

I checked the latest master flame ROM. When Galley app is in foreground, b2g main thread seems busy because of attention notification animation. When enabling "Log Slow animations". Logcat has the following warning log.

> Gecko   : Performance warning: Async animation disabled because frame size (90, 4) is bigger than the viewport (360, 2) or the visual rectangle (104, 4) is larger than the max allowable value (17895698) [div]

On master, attention notification is changed a lot, it seems to be affected to this.

Comment 26

[Sotaro Ikeda [:sotaro]]

The log of Comment 25 is from nsDisplayTransform::ShouldPrerenderTransformedContent().

Comment 27

[Sotaro Ikeda [:sotaro]]

b2g main thread fps could be estimated from "Frames per second" that could be enabled from "Developer HUD". middle is "transaction fps"

http://dxr.mozilla.org/mozilla-central/source/gfx/layers/composite/FPSCounter.cpp#445

Comment 28

[Sotaro Ikeda [:sotaro]]

(In reply to Alberto Pastor [:albertopq] from comment #24)
> :sotaro, could you please double check that the patch in Bug 1072616 fixes
> this performance issue? Thanks!

On v2.1 flame, it works!

But on master, async animation did not work as in comment 25. It seems to be caused by animated Element size problem. It is a different problem. It seems to be caused by re-factoring of attention indicator in master.

Comment 29

[bhavana bajaj [:bajaj]]

(In reply to Gregor Wagner [:gwagner] from comment #19)
> We need to figure out how much risk we are willing to take here before we
> can start working on it since it will be a 2.1 only patch.

We have a low risk patch in https://bugzilla.mozilla.org/show_bug.cgi?id=1072616, that seems to help here.

Comment 30

[Gregor Wagner [:gwagner]]

Can we verify that the patch from 1072616 fixed this problem and we can dupe?

Comment 31

[Gregor Wagner [:gwagner]]

Should be fixed by bug 1072616.

Comment 32

[Lance（leave from Mozilla）]

Clone bug 1127757 to track this bug.