Launch latency of Music application needs to be improved

RESOLVED WORKSFORME

Status

P2
normal
RESOLVED WORKSFORME
6 years ago
5 years ago

People

(Reporter: mvikram, Assigned: hub)

Tracking

({perf})

unspecified

Firefox Tracking Flags

(b2g18+)

Details

(Whiteboard: [c= p=3 s= u=])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.64 Safari/537.31

Steps to reproduce:

We are seeing varying launch latencies (first time launch)for the Music app over the last few releases. The general trend has been that the launch times have been going higher (please see below). These numbers have been captured on a QRD device.


Actual results:

Here are some results captured over many weeks. 

Build                                      Launch time (ms)                          
QRD B2G AU01.00.01.19.070          2277
QRD B2G AU01.01.00.019.077        3271
QRD B2G AU01.01.00.019.086        3646


Expected results:

We would like to see the launch time improved to ~ 3000 ms.
blocking-b2g: --- → leo?
(Reporter)

Comment 1

6 years ago
Is this being looked at? Thanks.

Comment 2

6 years ago
David, any idea whats up here?
Flags: needinfo?(dflanagan)

Comment 3

6 years ago
Also Mandyam, we need a _precise_ test case. What exactly did you measure? What state? Empty music apps? With files loaded? If with files, it would be great if you can share those. If you can't, it would be great if you can try to reproduce with publicly available files. Otherwise our team will be in the dark trying to reproduce what you are seeing.
Flags: needinfo?(mvikram)
If there is a specific 3000ms target you want us to hit, we need to know exactly how you're defining "launch".  Is this a launch where new music files are being scanned, or just starting up with already scanned files?  Are you timing from the first tap to a fully-painted screen?  Or are you waiting until the scanning progress indicator goes away?

Dominic: have there been any changes to the music app that might affect startup time? I remember seeing new code for the Pick activity, but that probably wouldn't affect normal startup.

Dave: I don't know what a QRD device is, but could your device storage changes to support internal and external storage have affected scanning time?
Flags: needinfo?(dkuo)
Flags: needinfo?(dhylands)
Flags: needinfo?(dflanagan)
(In reply to David Flanagan [:djf] from comment #4)
> If there is a specific 3000ms target you want us to hit, we need to know
> exactly how you're defining "launch".  Is this a launch where new music
> files are being scanned, or just starting up with already scanned files? 
> Are you timing from the first tap to a fully-painted screen?  Or are you
> waiting until the scanning progress indicator goes away?
> 
> Dominic: have there been any changes to the music app that might affect
> startup time? I remember seeing new code for the Pick activity, but that
> probably wouldn't affect normal startup.
> 
> Dave: I don't know what a QRD device is, but could your device storage
> changes to support internal and external storage have affected scanning time?

I honestly can't imagine that the composite storage stuff would have added anything to the scanning time, unless it was a phone where we were just scanning one volume previously and we're scanning two volumes now.

The composite support adds a tiny amount of overhead, once per enumerate call, which I would imagine should be measured in sub-millisecond, perhaps a few milliseconds at most. This is assuming that you're scanning the same number of volumes before and after.

We also need to find out whether the device storage area being scanned was internal or a physical SD Card, and if its a physical SD Card, what its ratings are.

I'm also not familiar with the QRD device.
Flags: needinfo?(dhylands)

Comment 6

6 years ago
(In reply to David Flanagan [:djf] from comment #4)
> Dominic: have there been any changes to the music app that might affect
> startup time? I remember seeing new code for the Pick activity, but that
> probably wouldn't affect normal startup.

David, I think recently there are no changes that might affect the startup time of Music app.
I looked on https://datazilla.mozilla.org/b2g/ , and didn't find noticeable changes on the startup time on both master and v1-train.

Mandyam, I don't really fully understand what's the startup time of "first time launch", I probably need more infomation like how you measure the startup time, how many songs on SD card or something like that, those information will be helpfule for us to identify the problems, thanks.
Flags: needinfo?(dkuo)

Updated

6 years ago
Keywords: perf
Whiteboard: c=
(Reporter)

Comment 7

6 years ago
I will provide updates with some details.
Flags: needinfo?(mvikram)

Comment 8

6 years ago
Bug 874317 is focused on the SD Scanning part of this, should it be marked as a dup of this?

Comment 9

6 years ago
We'll track for v1.1, we'll set koi? for v1.2 and we'll accept a low risk fix if found for v1.1.
blocking-b2g: leo? → koi?
tracking-b2g18: --- → +
(Assignee)

Comment 10

5 years ago
Will have a look.
Assignee: nobody → hub
(Assignee)

Updated

5 years ago
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Updated

5 years ago
Whiteboard: c= → c= ,
(Assignee)

Updated

5 years ago
Whiteboard: c= , → c= , p=3 ,
(Reporter)

Comment 11

5 years ago
The launch latency is trending down since the last few releases:

Here are some results captured over many weeks. 

Build                            Launch time (ms)                          
QRD B2G AU01.00.01.019.102        3240
QRD B2G AU01.01.00.019.109        2406
QRD B2G AU01.01.00.019.122        2283


(Note: these are measured with a high speed camera starting with touch down to the first app screen paint)
(Assignee)

Comment 12

5 years ago
I have several question about your measurement.

First, with the performance measurements we do at, we don't see the Music application launch time being bigger than most of the other apps.

Here is an example:
https://datazilla.mozilla.org/b2g/?branch=master&device=unagi&range=7&test=cold_load_time&app_list=browser,contacts,music&app=browser&gaia_rev=fd7449359a3355fb&gecko_rev=c6e29ce82de86d2c

We do the test on the Unagi phone, and this is using "master" on both gecko and gaia, and updated to track the development.

So I have several questions:

1. What kind of workload to you put on the Music application. By workload I mean the user data set that you use to simulate real use case

2. What kind of device is this run on

3. What release to you use. I can find a correspondence between your builds and ours.

4. What do you use and where can we get it?

This would be helpful in helping to reproduce the test case.

Thanks
Flags: needinfo?(mvikram)

Updated

5 years ago
Priority: -- → P2

Comment 13

5 years ago
FYI, we have music mock data generation at: https://github.com/mozilla/b2gpopulate
(Assignee)

Comment 14

5 years ago
Nice. I had to fix it too. Pull request submitted.
(Assignee)

Comment 15

5 years ago
Created attachment 765615 [details] [review]
Pull request
Attachment #765615 - Flags: review?(dkuo)
(In reply to Hubert Figuiere [:hub] from comment #12)
> I have several question about your measurement.
> 
> First, with the performance measurements we do at, we don't see the Music
> application launch time being bigger than most of the other apps.
> 
> Here is an example:
> https://datazilla.mozilla.org/b2g/
> ?branch=master&device=unagi&range=7&test=cold_load_time&app_list=browser,
> contacts,
> music&app=browser&gaia_rev=fd7449359a3355fb&gecko_rev=c6e29ce82de86d2c
> 
> We do the test on the Unagi phone, and this is using "master" on both gecko
> and gaia, and updated to track the development.
> 
> So I have several questions:
> 
> 1. What kind of workload to you put on the Music application. By workload I
> mean the user data set that you use to simulate real use case
> 
> 2. What kind of device is this run on
> 
> 3. What release to you use. I can find a correspondence between your builds
> and ours.
> 
> 4. What do you use and where can we get it?
> 
> This would be helpful in helping to reproduce the test case.
> 
> Thanks

We are using QRD. We already saw improvement in launch music app launch latency in latest AU. Now it is similar to Android music app. We don't require further improvement at this stage. We will create new bugid if we see some regression in future.
Flags: needinfo?(mvikram)
(Assignee)

Comment 17

5 years ago
Where can we get QRD?
Flags: needinfo?(mvikram)
(Assignee)

Comment 18

5 years ago
oh QRD is the reference design - ie the hardware. I meant the tools you use to measure it.
(In reply to Hubert Figuiere [:hub] from comment #17)
> Where can we get QRD?

We used High FPS camera to measure launch latency music app . We are OK with current launch latency of music app.
Flags: needinfo?(mvikram)

Comment 20

5 years ago
Comment on attachment 765615 [details] [review]
Pull request

Hubert, sorry for the late reply, I took a quick look on your patch and found you tried to lazy load the JS code of the SublistView. This probably can improve some startup time but after I pulled and tested, I didn't see a noticeable change of the startup.(I used the Developer settings of "Show time load" in the settings app, it's around 800ms before and after)

I think what really slows down the Music app is the unused DOM elements at the first sight, which I have fixed in bug 844029. Also comment 16 confirmed the startup time is already improved. But I think your patch is good from the perspective of refactoring, probably music app should isolate all the views to a single JS file and load it when we need it, so I will consider this is more like a code refactoring.
Attachment #765615 - Flags: review?(dkuo)
(Assignee)

Comment 21

5 years ago
I use b2gperf which is the tool that measure this, using marionette, for the perf-o-matic data: 

https://datazilla.mozilla.org/b2g/

And I did it after using b2g populate.

Since I use master (gaia and gecko), I have the patches from bug 844029 as well.

Yes comment 16 confirm some of the changes prior to this.
(Assignee)

Comment 22

5 years ago
Dominic,

Any reason why we couldn't have this merge into master?
Flags: needinfo?(dkuo)

Comment 23

5 years ago
Hubert, to lazy load the JS code of the SublistView is a good idea because Music app doesn't need to load it at startup, but this also means if we are trying to lazy load some JS, all the JS that are not needed at startup like ListView and TilesView should also be considered, not just SublistView.

And if I recall correctly, if we just "not" to load music.js, which includes TilesView, ListView, SublistView and the other code, the startup time is almost the same, so that's why I found what really slows down the startup time of Music app is not the JS code, but the unnecessary DOM elements before launched.

Or can you share your result? I am curious that with your patch, what's the exact improvement you got?
Flags: needinfo?(dkuo)
(Assignee)

Comment 24

5 years ago
(In reply to Dominic Kuo [:dkuo] from comment #23)
> Hubert, to lazy load the JS code of the SublistView is a good idea because
> Music app doesn't need to load it at startup, but this also means if we are
> trying to lazy load some JS, all the JS that are not needed at startup like
> ListView and TilesView should also be considered, not just SublistView.

I investigated doing it with TilesView too, but I need to hook more stuff to have it work

 
> And if I recall correctly, if we just "not" to load music.js, which includes
> TilesView, ListView, SublistView and the other code, the startup time is
> almost the same, so that's why I found what really slows down the startup
> time of Music app is not the JS code, but the unnecessary DOM elements
> before launched.
> 
> Or can you share your result? I am curious that with your patch, what's the
> exact improvement you got?

The problem is that in the current configuration of the master branch I don't seem to be able to reproduce any of the performance improvements.

My methodology to test is as follow.

Use |b2gpopulate --music=100| to populate the music database with 100 songs.
( see https://github.com/mozilla/b2gpopulate )

Then use |b2gperf music| to measure the startup time before and after applying the patch.

I previously measured a 100ms performance improvement. (this is on Unagi)

Not anymore, BUT I get an even lower number now.

I'm wondering if it is worth any effort now.
(Assignee)

Comment 25

5 years ago
Since we finally did nothing directly and the performance have been deemed correct by the reporter, and that we got already better performance with m-c in master, I'll close this as WORKSFORME.


If there are any future preformance problems, please open a new bugzilla with a proper methodology for testing.

Thanks.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WORKSFORME

Comment 26

5 years ago
Clearing koi? flag since this is resolved as worksforme and fxos-perf believes all work to be done is complete.
blocking-b2g: koi? → ---
Whiteboard: c= , p=3 , → [c= p=3 s= u=]
Attachment mime type: text/plain → text/x-github-pull-request
You need to log in before you can comment on or make changes to this bug.