1011865 - [B2G][Marketplace][Poppit] The 'Poppit' app is too slow

Status: RESOLVED FIXED

(Core :: Graphics, defect, P1)

Description

[Sotaro Ikeda [:sotaro]]

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

This bug is created based on Bug 1009980 Comment 8.

*STR
[1] Download the app 'Poppit' from the marketplace.
[2] Launch 'Poppit'.
[3] Start to play game

*Actual result
The poppit app is too slow to play.


This problem happen only on master. I confirmed this by using master hamachi, master flame.

The problem did not happen v1.4 flame.

Comment 1

[Jason Smith [:jsmith]]

Switching to qawanted to get a video & a logcat.

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

I wonder if this is similar to 999841.

Comment 3

[Lionel Mauritson]

Attachment: logcat_20140519_1012_poppit.txt

Video: http://youtu.be/8c4FY1b6uS4

Comment 4

[bhavana bajaj [:bajaj]]

Blocking as observed in the video that the lag was very obvious and this is a regression in 2.0 Hoping the regression-window will help us find the root cause.

Comment 6

[Peter Chang[:pchang]]

Attachment: profiler on b2g master

Comment 7

[Peter Chang[:pchang]]

The following are two profilers of guimark2 on b2g master and 1.4 local build.

b2g master spends more time on SurfaceStream_TripleBuffer::SwapProducer which invokes SharedSurfaceGralloc::Fence to read pixels.

profiler on b2g master
http://people.mozilla.org/~bgirard/cleopatra/#report=e05813461f2a43d6a67405543754f9065227327d

profiler on b2g 1.4
http://people.mozilla.org/~bgirard/cleopatra/index.html?#report=676ff0707442bf07644d7a3ffe16457b2f295877

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

For what its worth, I saw a lot of SurfaceStream_TripleBuffer::SwapProducer recently in a game on win8 over in bug 1013039.

Comment 9

[Mason Chang [Inactive] [:mchang]]

Triage: From comment 4, this looks like a regression. Waiting on a regression window before acting.

Comment 10

[Lionel Mauritson]

Regression window in Mozilla inbound

Last Working
Build: 20140415105558
Gaia: 3d47c0627017ef77b1adf179792c6543a349af72
Gecko: e134b3aa718f
Version: 31.0a1

First Broken
Build: 20140415110403
Gaia: 3d47c0627017ef77b1adf179792c6543a349af72
Gecko: a54345bfd338
Version: 31.0a1

New Gaia old Gecko: Working
Gaia: 3d47c0627017ef77b1adf179792c6543a349af72
Gecko: e134b3aa718f

Old Gaia new Gecko: Broken
Gaia: 3d47c0627017ef77b1adf179792c6543a349af72
Gecko: a54345bfd338

Determined to be a Gecko issue.
Push log: hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e134b3aa718f&tochange=a54345bfd338

Comment 11

[Peter Chang[:pchang]]

I just confirmed this issue was caused by below commit.

changeset:   178703:a54345bfd338
user:        Jonathan Watt <[e-mail]>
date:        Tue Apr 15 19:02:23 2014 +0100
files:       browser/components/shell/src/nsWindowsShellService.cpp content/svg/content/src/SVGFEImageElement.cpp gfx/2d/DataSurfaceHelpers.h image/public/imgIContainer.idl image/src/ClippedImage.cpp image
description:
Bug 950372 - Convert imgIContainer::GetFrame to return a Moz2D SourceSurface instead of a Thebes gfxASurface. r=mattwoodrow

Comment 12

[Milan Sreckovic [:milan] (needinfo for best results)]

Jonathan, I'd rather not just back this out if you have a chance to take a look and get us a "real" fix.

Comment 13

[Jonathan Watt [:jwatt]]

Peter, did bug 996929 not then fix this (in which case there must have been a later regression that regressed things again).

Comment 14

[Jonathan Watt [:jwatt]]

It did not.

Comment 15

[Matt Woodrow (:mattwoodrow)]

Taking this at the request of jwatt.

Comment 16

[Matt Woodrow (:mattwoodrow)]

I looks like bug 996929 did the right thing to fix this, but the skia(-gl) backend code still wasn't doing the right thing.

Comment 17

[Matt Woodrow (:mattwoodrow)]

Attachment: Make OptimizeSourceSurface smarter when skia-gl is enabled

Comment 18

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb71ae94a9fd

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bb71ae94a9fd

Comment 20

[Michael Wu [:mwu]]

Does this actually cause images to be cached in GPU? I had a similar patch, but I had to mark the surface as non-volatile for skia to do the right thing. Without that, I couldn't see any difference in the fishietank benchmark.

static_cast<SourceSurfaceSkia*>(result.get())->mBitmap.setIsVolatile(false);