865187 - [Music] With 700 songs loaded in SD card, most played playlist should be available in < 2 sec

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::Music, defect, P2)

Description

[Leo]

1. Title : Time taken from playlist(most played) to playscreen is huge.
2. Precondition : Load SD card with 700 Music files
3. Tester's Action :Open Music player
4. Detailed Symptom (ENG.) :
			1. Loaded SD card with 700 Music files
			2. Open Music player
			3. After the scanning is complete
			4. Goto playlist
			5. Select most played
			6. The time taken is very huge to go to playscreen.
			
This behaviour is also seen,if least played or highest rated from playlist is selected as well.			

5. Expected : Time taken should have be less
6.Reproducibility: Y
	1)Frequency Rate : 100%
7.Gaia Master/v1-train : Reproduced
8.Gaia Revision: b97dddebcb755978041b5f3f146b04ea493e50b0
9.Personal email id:  [e-mail]

Comment 1

[Leo]

Right now we are calling the enumerateAll method of mediaDB, which will return the result in bulk.

Cant we use the enumerate method of mediaDB here? This will give a better user experience i think.

Comment 2

[Leo]

Attachment: Proposed patch for the Music list loading issue.

Proposed patch for the Music list loading issue.

Here I am trying to call the enumerate method instead of the enumerateAll method of mediaDB.

Please comment on the UI impact it may have.

Comment 3

[Dominic Kuo [:dkuo]]

(In reply to Leo from comment #2)
> Created attachment 741684 [details] [diff] [review]
> Here I am trying to call the enumerate method instead of the enumerateAll
> method of mediaDB.
> 
> Please comment on the UI impact it may have.

If you look on the code of MediaDB, you can see the comment says, enumerateAll() batches the results and passes them in an array to the callback function, and actually enumerateAll() is the sum of enumerate(). So the total time of enumerating all the records in MediaDB will the same with enumerate() and enumerateAll().

Assuming there are 700 songs and if we enumerate() them, we can get the first record very fast, but we will get 700 callbacks to update the ui, which will block the ui for a while for sure and users cannot get a smooth ui while enumerating.

The reason we use enumerateAll() in playlists is, there is only one callback to update the ui, although we have to wait for it, but the thread dosen't need to switch between rendering the ui and enumerating the records.

I think the real problem here is we shouldn't do too many indexedDB actions after music app is launched, we should try to keep in-memory records so that we don't have to enumerate from MediaDB everytime.

I will consider this as a performance issue and I would like to take this, but this will need a lot of changes because we have to get rid of some DB actions in music app.

Comment 4

[Alex Keybl [:akeybl]]

We can't block on this bug until we've agreed upon a new v1.1 performance target.

Comment 5

[Dylan Oliver [:doliver]]

Triage: spoke with Leo and got a target number of 2 seconds to see the most played playlist. 

Tracking for now, leaving needinfo for product. Please re-nominate if this should block. From comment #3 sounds like this is not a trivial amount of work.

Comment 6

[Peter Dolanjski [:pdol]]

Sandip, given that you've been tracking performance from a Product perspective, I'll let you comment here if you disagree with the non-blocking status.

Comment 7

[Sandip Kamat]

Understand the amount of work in comment #3. Can we get a comparative number between 1.0.1 and 1.1? Need to ensure no degradation. Also what timeframes is this being deferred to?

Comment 8

[Mike Lee [:mlee]]

Dominic, please reply to Sandip's question in comment #7.

Comment 9

[Dominic Kuo [:dkuo]]

(In reply to Sandip Kamat from comment #7)
> Understand the amount of work in comment #3. Can we get a comparative number
> between 1.0.1 and 1.1? Need to ensure no degradation. Also what timeframes
> is this being deferred to?

I think for v1.0.1 and v1.1 should be the same because that part didn't make big changes. And I was thinking to use MediaDB.getAll() to retrieve all the song records then sort them with some comparison function, this probably will be faster then waiting MediaDB.enumerateAll() finishes.

Comment 10

[gbennett]

Hello Mason, what can QA help you with?

Comment 11

[Dominic Kuo [:dkuo]]

Last time when UX week was hold in Taipei I have discussed this with Rob. He mentioned for the playlists(except Shuffle all) ordering by different criteria, music should only display the first 25 songs instead of all the songs in user's collection, because the playlists were intended to classify the songs by the user's listening habits, so displaying some particular number like 25 is more useful for the users. By displaying the top 25 songs music should be able to retrieve them in a short and constant time, then this bug can be resolved.

Comment 12

[Mason Chang [Inactive] [:mchang]]

We just wanted to see if this bug was still active and if a 2 second requirement was being met. Looks like Dominic is on it from comment #11. Thanks!

Comment 13

[gbennett]

(In reply to Mason Chang [:mchang] from comment #12)

Removing qawanted per comment 12. Feel free to throw it back on if need. :)

Comment 14

[Hubert Figuiere [:hub]]

There is indeed a serious performance issue loading the playlist.

Comment 16

[BMO Automation]

Firefox OS is not being worked on