SMS scroll FPS regressed in FFOS v.1.3

RESOLVED FIXED in mozilla30

Status

()

defect
P1
blocker
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: tkundu, Unassigned)

Tracking

({perf})

unspecified
mozilla30
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?
in-moztrap +

Firefox Tracking Flags

(blocking-b2g:1.3+, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

Details

(Whiteboard: [caf priority: p2][CR 620477][c=handeye p= s= u=1.3])

STR:

1) Use |make reference-workload-light| to load sms in SMS app.
2) launch SMS App and scroll.

You will see big FPS difference between these two commits:

1) Good Commit: (FPS ~55)
   
   gaia : 

commit 467ef8c9145d9a57d35b0619db541d23b522b958
Author: Germ<C3><A1>n Toro del Valle <gtorodelvalle@gmail.com>
Date:   Wed Feb 5 15:33:38 2014 +0100
Merge pull request #15950 from gtorodelvalle/contacts-967540-invalid-dates-in-gmail. Bug 967540 - [Contacts] Exporting to SD card contacts imported from Gmail with invalid dates never ends


   gecko :

commit 669119e24ef04c9ce1ccde3af068fc64e83dd201
Author: Gaia Pushbot <release+gaiajson@mozilla.com>
Date:   Thu Feb 6 00:35:59 2014 -0800
Bumping gaia.json for 1 gaia-1_3 revision(s) a=gaia-bump


2) Bad Commit: (FPS <30, Big visible stutters during scroll):

   gaia :
   
   commit dd94a5ba3f56ce1e8b378feeecc8a2d7a0b30fc7
   Merge: 464c279 e439f86
   Author: David Flanagan <dflanagan@mozilla.com>
   Date:   Fri Feb 7 16:46:10 2014 -0800
   Merge pull request #16011 from davidflanagan/bug942199v1.3
   Bug 942199: Allow minimum EXIF preview size to be configured at build time for Camera and Gallery r=dmarcos

   gecko:
   
   commit e4c69b6a3ec68c7e570b75484f47ac24879364fb
   Merge: 44d2a3a 5002d57
   Author: Ryan VanderMeulen <ryanvm@gmail.com>
   Date:   Fri Feb 7 20:23:42 2014 -0500
   Merge mozilla-beta to b2g28. a=merge
blocking-b2g: --- → 1.3?
Component: Gaia::E-Mail → Gaia::SMS
The commits here for gecko isn't helpful. I need the short version of the gecko commit to do a push log here. Can you include the short version of each gecko commit here for bad vs. good?
Flags: needinfo?(tkundu)
bug 965593 is in this range (which is known to cause scrolling perf regressions) - can you guys try backing out the 1.3 branch in your gecko branch & seeing if there's any difference here?
(In reply to Jason Smith [:jsmith] from comment #3)
> bug 965593 is in this range (which is known to cause scrolling perf
> regressions) - can you guys try backing out the 1.3 branch in your gecko
> branch & seeing if there's any difference here?

backing out the 1.3 patch*
(In reply to Jason Smith [:jsmith] from comment #4)
> (In reply to Jason Smith [:jsmith] from comment #3)
> > bug 965593 is in this range (which is known to cause scrolling perf
> > regressions) - can you guys try backing out the 1.3 branch in your gecko
> > branch & seeing if there's any difference here?
> 
> backing out the 1.3 patch*

If I pickup gaia from Bad Commit and gecko from Good Commit then I don't see this regressions any more. I have *NOT* verified with high speed camera. But it seems to me that all visible stutters in SMS APP goes away.
(In reply to Tapas Kumar Kundu from comment #5)
> (In reply to Jason Smith [:jsmith] from comment #4)
> > (In reply to Jason Smith [:jsmith] from comment #3)
> > > bug 965593 is in this range (which is known to cause scrolling perf
> > > regressions) - can you guys try backing out the 1.3 branch in your gecko
> > > branch & seeing if there's any difference here?
> > 
> > backing out the 1.3 patch*
> 
> If I pickup gaia from Bad Commit and gecko from Good Commit then I don't see
> this regressions any more. I have *NOT* verified with high speed camera. But
> it seems to me that all visible stutters in SMS APP goes away.

Interesting. That actually means the regression is on the Gaia side, not the Gecko side.
(In reply to Jason Smith [:jsmith] from comment #6)
> (In reply to Tapas Kumar Kundu from comment #5)
> > (In reply to Jason Smith [:jsmith] from comment #4)
> > > (In reply to Jason Smith [:jsmith] from comment #3)
> > > > bug 965593 is in this range (which is known to cause scrolling perf
> > > > regressions) - can you guys try backing out the 1.3 branch in your gecko
> > > > branch & seeing if there's any difference here?
> > > 
> > > backing out the 1.3 patch*
> > 
> > If I pickup gaia from Bad Commit and gecko from Good Commit then I don't see
> > this regressions any more. I have *NOT* verified with high speed camera. But
> > it seems to me that all visible stutters in SMS APP goes away.
> 
> Interesting. That actually means the regression is on the Gaia side, not the
> Gecko side.

Oh wait - I've got it backwards. That actually confirms this is a gecko regression, not a Gaia regression.
I'm just operate under the analysis we've already done that bug 965593 is likely the cause here.
Depends on: 965593
Keywords: perf
Whiteboard: [CR 620477] → [CR 620477][c=handeye p= s= u=1.3]
move this to general component for now as it looks like gecko bug
Component: Gaia::SMS → General
Whats the status here?
blocking-b2g: 1.3? → 1.3+
This should be fixed now that I have backed out bug 965593 everywhere. Please test and confirm.
Component: General → Panning and Zooming
Product: Firefox OS → Core
Jason

Can you please have this tested?
Flags: needinfo?(jsmith)
Merge of bug 965593 backout:
https://hg.mozilla.org/mozilla-central/rev/56f2d96b6e12

You can ask RelEng to trigger a new nightly off m-c tip if you need a build to test.
This isn't really something QA can test - A lot of us aren't trained to do formal FPS studies. The performance team I think knows how though. I'm going to redirect this to Mason, as he worked identifying this problem previously in his approach.
Flags: needinfo?(jsmith) → needinfo?(mchang)
We can't do accurate scroll FPS tests, but I already tested during the process of bug 974081, where it is much smoother without bug 965593. Tapas, can you please retest with a high speed camera. Thanks!
Flags: needinfo?(mchang) → needinfo?(tkundu)
Going to close because I'm pretty sure the backout of that bug is going to fix this. If Tapas's testing proves otherwise, then reopen.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Flags: needinfo?(tkundu)
Flags: in-testsuite?
Flags: in-moztrap-
Test case has been added here: https://moztrap.mozilla.org/manage/case/13734/
Flags: in-moztrap- → in-moztrap+
Whiteboard: [CR 620477][c=handeye p= s= u=1.3] → [caf priority: p2][CR 620477][c=handeye p= s= u=1.3]
You need to log in before you can comment on or make changes to this bug.