Huge performance regression when dragging icons in edit mode of Gaia homescreen

VERIFIED FIXED

Status

()

defect
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: cwiiis, Assigned: cwiiis)

Tracking

({regression})

Trunk
All
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.1+, b2g-v2.0 unaffected, b2g-v2.1 verified, b2g-v2.2 fixed)

Details

(Whiteboard: [systemsfe])

Attachments

(1 attachment)

Assignee

Description

5 years ago
If you use Gecko master with current Gaia master, the animations for rearranging icons on the homscreen have a terrible frame-rate.

Reverting Gecko to revision 203057 (just before multi-layer-apz landed (bug 967844)) exhibits much better performance. I'm not, however, quite ready to say that that's the cause of this issue, so I'm asking for a regression window and leaving the component as 'General'.

An aside, looking at the layer boundaries and paint flashing, they both look ok from a quick inspection.

I'm currently building the v2.1 Gecko branch to see if it's affected - if so, this should be a 2.1 blocker - the visible performance of these animations is unacceptable and a major regression vs. 2.0.

Cc'ing some folks that may be interested/already know about this just in case.
AFAIK the scrolling that happens while dragging around an icon is all controlled from the gaia side anyway.
See Also: → 1047069
Assignee

Comment 2

5 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1)
> AFAIK the scrolling that happens while dragging around an icon is all
> controlled from the gaia side anyway.

I don't mean the scrolling, I mean the animation that runs when you hover an item over a new position. This is fluid in 2.0 and very janky in master. Running master Gaia with an older Gecko restores fluid performance.
Ah, I see. Is that animation done with an OMTA animation? Can you point to the gaia code that triggers it?
Assignee

Comment 4

5 years ago
If we were previously falling back to non-apz scrolling in older Gecko, that could explain the poor performance (due to the displayport being huge vs. screen-sized).

That would go some way to explain this regression, but it remains an issue. We may be able to work around it by setting overflow: hidden during animations, but I think that's a flaky solution and this ought to be fixed at the gfx level, if indeed it is a gfx issue causing it.

Do we do any visibility culling in layers currently? I know tiled layers do (thanks Bas), but do standard non-tiled layers check to see if they're within the clip rect before drawing?
Assignee

Comment 5

5 years ago
[Blocking Requested - why for this release]: Visually obvious performance regression.

This does indeed affect the 2.1 branch.
blocking-b2g: --- → 2.1?
It would be better to get a profile of the behaviour than jump to conclusions prematurely.
Assignee

Comment 7

5 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #3)
> Ah, I see. Is that animation done with an OMTA animation? Can you point to
> the gaia code that triggers it?

The home-screen is a GaiaGrid: https://github.com/mozilla-b2g/gaia/tree/master/shared/elements/gaia_grid
This function triggers the reorder: https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/grid_dragdrop.js#L342
Which works by just changing the transform and using this rule: https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/style.css#L121

This should indeed be an OMTA.
Assignee

Comment 8

5 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #6)
> It would be better to get a profile of the behaviour than jump to
> conclusions prematurely.

I'm just rebuilding now and will try to get a profile.
Oh, it might also be worth seeing if this happens with user_pref("layout.async-containerless-scrolling.enabled", false); - if not, then it's definitely a regression from multi-layer-apz. Should be easy to test.
Assignee

Comment 10

5 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #9)
> Oh, it might also be worth seeing if this happens with
> user_pref("layout.async-containerless-scrolling.enabled", false); - if not,
> then it's definitely a regression from multi-layer-apz. Should be easy to
> test.

It doesn't happen with this pref disabled.
Assignee

Comment 11

5 years ago
I long-pressed an icon and just dragged it back and forth in a way to trigger the animation a few times. Worth noting that dragging the icon was smooth enough, it's just the animation that runs badly. Given that paint flashing and layer boundaries look normal, kats's theory of it missing OMTA due to layer structure sound pretty viable.

Profile of compositor: https://people.mozilla.org/~bgirard/cleopatra/#report=369204d67683c6ff4b0dfa6a2ba5d49ed6096392
Profile of homescreen: https://people.mozilla.org/~bgirard/cleopatra/#report=6090ae2a16f5b6121ca9ccca67398932cea6e969

These profiles don't look particularly illuminating to me... They were captured with the default settings.
(In reply to Chris Lord [:cwiiis] from comment #4)
> Do we do any visibility culling in layers currently? I know tiled layers do
> (thanks Bas), but do standard non-tiled layers check to see if they're
> within the clip rect before drawing?

Yes, bug 1010584 landed does some culling. There's also another type of culling in container layer. I don't recall what the exact different is.

If we think we need to cull better then file a bug with a testcase please.
(In reply to Chris Lord [:cwiiis] from comment #11)

You're not getting a combined profile which is significantly less useful. If you need stackwalk then you should use the -f option as well.

We also have OMTA failure logging, it's very informative. If we're hitting that then you should turn that on.
Assignee

Comment 14

5 years ago
I didn't notice the third symbol file it exported, here's the combined profile: http://people.mozilla.org/~bgirard/cleopatra/#report=439cda40ea13f2db2d479e28544a7ce2cff39e05
Assignee

Comment 15

5 years ago
Another profile with native stack walking: http://people.mozilla.org/~bgirard/cleopatra/#report=47a88a8bee6ed239e19f5659e7b3b43267f5991c

That also has OMTA failure logging on, but the logging tab is empty. Going to observe manually.
Assignee

Comment 16

5 years ago
(In reply to Chris Lord [:cwiiis] from comment #15)
> Another profile with native stack walking:
> http://people.mozilla.org/~bgirard/cleopatra/
> #report=47a88a8bee6ed239e19f5659e7b3b43267f5991c
> 
> That also has OMTA failure logging on, but the logging tab is empty. Going
> to observe manually.

I had to hard-code the failure logging to on as every time I added the pref to the user_prefs, it just got removed again afterwards (no idea...) - doing so though, I see no failure warnings.

On the other hand, I'm becoming more and more convinced that that is the problem (these animations aren't getting OMTA anymore) - I could do with some advice on the best way to help out here.
Would probably help to get a layers dump and a display-list dump as well.

roc, any idea why multi-layer-apz might have affected OMTA?
Component: General → Layout
Flags: needinfo?(roc)
No idea! Should have had no effect. Layer tree and display list dump are definitely the first step.
Flags: needinfo?(roc)
(In reply to Chris Lord [:cwiiis] from comment #15)
> Another profile with native stack walking:
> http://people.mozilla.org/~bgirard/cleopatra/
> #report=47a88a8bee6ed239e19f5659e7b3b43267f5991c
> 
> That also has OMTA failure logging on, but the logging tab is empty. Going
> to observe manually.

The profile shows that there's an async feature being run. You can tell because the 'Composite' stage runs continuously (not as a response to a main thread transaction). If you're not scrolling (APC) then it's like OMTA triggering that. Could the bug be that OMTA is just busted?
Regression = blocker
blocking-b2g: 2.1? → 2.1+
Assignee

Comment 21

5 years ago
I need to double-check, but this does appear to be fixed in master - I'll report back later if that's the case, at which point we'd need a window to see what needs uplift.
(In reply to Chris Lord [:cwiiis] from comment #21)
> I need to double-check, but this does appear to be fixed in master - I'll
> report back later if that's the case, at which point we'd need a window to
> see what needs uplift.

Chris, any news here?
Flags: needinfo?(chrislord.net)
Chris, can you confirm that this is fixed on master? Should we ask QA to find the cset that fixed it?
Assigning to :cwiis per comment 21.
Assignee: nobody → chrislord.net
Assignee

Comment 25

5 years ago
I can confirm that this is fixed in master and still an issue in 2.1. Could QA find when this was fixed please?

STR:

1. Install an engineering build (I suspect that this may not so easily reproduce if there are fewer apps installed)
2. Long press on any icon and drag it into another position

Expected:

After a short pause, there should be a smooth transition where the icons move to their new positions.

Actual:

After a short pause, there is a terrifically janky transition where the icons move to their new positions.
Flags: needinfo?(chrislord.net)
Keywords: qawanted
Assignee

Comment 26

5 years ago
Can also confirm that disabling layout.async-containerless-scrolling.enabled fixes the issue, so it's very likely a Gecko bug/behaviour that has since been fixed/changed wrt bug 967844.
Tested with Full Flash on 319mb using Engineering builds

This bug repro's on Flame KK builds: Flame 2.1 KK

Actual Results: When icons are shuffling around the homescreen when you are holding an icon on your finger and trying to find a place to put it, the performance of the icons shuffling is poor.

Repro Rate: 2/2

Environmental Variables:
Device: Flame 2.1 KK
BuildID: 20141020202821
Gaia: e458f5804c0851eb4e93c9eb143fe044988cecda
Gecko: ee86921a986f
Version: 34.0 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

-----------------------------------------------------------------
-----------------------------------------------------------------

This bug does NOT repro on Flame kk build: Flame 2.2 KK, Flame 2.0 KK

Actual Result: Moving and shifting of homescreen icons is smooth.

Repro Rate: 0/4

Environmental Variables:
Device: Flame 2.2 KK
BuildID: 20141020185822
Gaia: 457a54fc3200b80e4f5e1cd3acaa062309230732
Gecko: 29fbfc1b31aa
Version: 36.0a1 (2.2)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
-----------------------------------------------------------------
Environmental Variables:
Device: Flame 2.0 KK
BuildID: 20141020233820
Gaia: 63b56a7a7453726b9e12ad1afe02c68c83c5aeca
Gecko: 40584eecdc75
Version: 32.0 (2.0)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawanted
QA Contact: croesch
QA-Wanted: they are looking for a 'fixed' regression-window
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
QA Contact: croesch
Cody, comment 25 is asking for a reverse regression window on 2.2.  please indicate when it was broken, and when it started working again.   Again, this is only against trunk.  Thanks.
Flags: needinfo?(croesch)
Keywords: qawanted
QA Contact: pcheng
b2g-inbound reverse regression window:

Last Broken Environmental Variables:
Device: Flame
BuildID: 20140911180753
Gaia: 8746bc86466677c3e1ed019c58c288577017362e
Gecko: c3d20247c3b4
Version: 35.0a1 (2.2 Master)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

First Working Environmental Variables:
Device: Flame
BuildID: 20140911224753
Gaia: 164cd3d0bb887aea2e10e0dc892a6444ddcff8fc
Gecko: bfcbc17baf33
Version: 35.0a1 (2.2 Master)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Last Broken Gaia & First Working Gecko - issue DOES repro
Gaia: 8746bc86466677c3e1ed019c58c288577017362e
Gecko: bfcbc17baf33

Last Broken Gecko & First Working Gaia - issue does NOT repro
Gaia: 164cd3d0bb887aea2e10e0dc892a6444ddcff8fc
Gecko: c3d20247c3b4

Gaia pushlog: https://github.com/mozilla-b2g/gaia/compare/8746bc86466677c3e1ed019c58c288577017362e...164cd3d0bb887aea2e10e0dc892a6444ddcff8fc

Fixed by bug 1038178.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(croesch) → needinfo?(jmitchell)
fixed by bug 1038178
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
QA Contact: pcheng
Assignee

Comment 32

5 years ago
(In reply to Joshua Mitchell [:Joshua_M] from comment #31)
> fixed by bug 1038178

Ok, this is worrying then, as this indicates that this is a platform bug, and that it may still exist... Either way, we have a work-around, so let's go with it.

n?kats to find out if all the work related to scrollbars on the root frame has been merged to aurora (or beta now? b2g34, anyway) so that we can uplift this patch without getting broken behaviour.
Flags: needinfo?(bugmail.mozilla)
Yeah all the scrollbar fixes have been uplifted to 2.1. However i was holding on to the option of disabling root frame scrollbars as a backup plan in case any other issues were found with them. If we uplift the homescreen change that won't be an option any more.

That being said i would like to know if backing out the homescreen change on master brings back the performance problem on master. If so, the underlying issue still exists and we should fix it. If not, a fix window with the homescreen change backed out would help us find the root cause fix.
Flags: needinfo?(bugmail.mozilla)
Assignee

Comment 34

5 years ago
Just checked Gaia revision 0de4fd947b7c023ac6d894d67530e3b74c855127 (before bug 1038178) against master Gecko and the bug still exists :(
Assignee

Comment 35

5 years ago
Unless platform fancies investigating this, I'm marking this as dependent on bug 1038178 and requesting 2.1 approval on that.
Depends on: 1038178
Assignee

Comment 36

5 years ago
This should be fixed, the patches in question have been uplifted.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
icon moving seems much smoother now in 2.1
Status: RESOLVED → VERIFIED
This issue has been verified successfully on Flame 2.1, there is a smooth transition where the icons move to their new positions.

See attachment: Verify_Video_Flame v2.1.MP4
Reproducing rate: 0/10

Flame v2.1 version:
Gaia-Rev        afdfa629be209dd53a1b7b6d6c95eab7077ffcd9
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/dc3018cbdbe6
Build-ID        20141123001201
Version         34.0
Whiteboard: [systemsfe]
You need to log in before you can comment on or make changes to this bug.