1136117 - [music][metadata] Improve performance of scanning music

Status: RESOLVED WONTFIX

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

Description

[Hubert Figuiere [:hub]]

The Music app seems to be slow scanning for music. This when I insert my 20GB music SD card.

We need to identify both how fast we want it and how faster we can make it.

Comment 1

[Jim Porter (:squib)]

So, I figured out how to make scanning TWICE as fast: hide that damned spinner. I have about 300 songs on my Flame, and it normally takes about 45 seconds to scan. I accidentally broke the spinner and it wasn't rendering. The new time? 25 seconds. If we hide the scanning progress *entirely*, we can get down to 18 seconds. This is almost identical to the scanning process when the screen is off, leading me to believe that none of our other repaints (i.e. refreshing the album list) are a major factor. (Note that these numbers are after only a few runs, but I could do more runs if folks think it matters.)

We really need to fix this somehow because a factor of two performance improvement is HUGE, even if it's not shown on Raptor. :)

Needinfo'ing a few people so this is on their radar. We have a few ways we could solve this: 1) fix our app if we're doing something dumb, 2) fix Gecko so it doesn't burn as much CPU on an animation, or 3) change the UI so it doesn't animate like this.

Comment 2

[Jim Porter (:squib)]

For those who want to look at our code in the Music app, here it is: <https://github.com/mozilla-b2g/gaia/blob/master/apps/music/elements/music-scan-progress.js#L25-L38>. The keyframes are near the bottom of the file.

Comment 3

[Jim Porter (:squib)]

(In reply to Jim Porter (:squib) from comment #1)
> So, I figured out how to make scanning TWICE as fast: hide that damned
> spinner. I have about 300 songs on my Flame, and it normally takes about 45
> seconds to scan.

I think this should actually be 35, not 45. I might have regressed something before I discovered this. In any case, that's still a 25% improvement, which is nothing to sneeze at. I looked into it, thanks to :birtle, and found that we're causing a lot of repaints, probably because of the text updating. If we fix those, we should be doing much better.

Comment 4

[David Flanagan [:djf]]

I agree that it is probably the changing text causing the repaints, not the CSS animation of the spinner. 

I don't know whether UX ever said what kind of animation they want there. I just made up what we've got now. It seemed cool to me to see the names of artists and songs flashing by quickly. But I never realized how much it slowed things down.

In the absence of any other requests from UX, maybe music could just use a crawling ants animation like gallery and video do? Its pure CSS now and only takes up about 3 pixels at the top of the screen.  IIRC the music app only shows the scanning animation when it actually finds new music and is parsing metadata. It might be better to do like gallery and video and show the animation whenever it starts scanning, even if nothing new has been found.

Comment 5

[Jim Porter (:squib)]

(In reply to David Flanagan [:djf] from comment #4)
> I agree that it is probably the changing text causing the repaints, not the
> CSS animation of the spinner. 

It seems to be the combination of the two. Just text is fast, and just the spinner is fast, but the two together are slow.

Comment 6

[Justin D'Arcangelo [:justindarc]]

(In reply to Jim Porter (:squib) from comment #5)
> (In reply to David Flanagan [:djf] from comment #4)
> > I agree that it is probably the changing text causing the repaints, not the
> > CSS animation of the spinner. 
> 
> It seems to be the combination of the two. Just text is fast, and just the
> spinner is fast, but the two together are slow.

FWIW, it could also be related to the fact that we have to paint the animation frames of the spinner and then also paint the overlaying number text on top of the spinner.

Also, there is an updated "spinner" implemented in the <gaia-loading> web component you can see here:

http://gaia-components.github.io/gaia-components/

It would be worth investigating if <gaia-loading> performs better than our existing spinner (it would also allow us to remove the spinner image file from the music repo). We should also check if removing the overlaying "number" on the existing spinner helps to any significant degree.

Comment 7

[Justin D'Arcangelo [:justindarc]]

Oh! I just thought of another possible idea. In the <music-scan-progress> web component, we should maybe put our textContent updates inside of a `requestAnimationFrame`. Maybe that will help?

Comment 8

[Jim Porter (:squib)]

If I hide *just* the number, it doesn't help. To go fast, you have to hide *all* the text (or the spinner).

Comment 9

[Jim Porter (:squib)]

(In reply to David Flanagan [:djf] from comment #4)
> In the absence of any other requests from UX, maybe music could just use a
> crawling ants animation like gallery and video do? Its pure CSS now and only
> takes up about 3 pixels at the top of the screen.  IIRC the music app only
> shows the scanning animation when it actually finds new music and is parsing
> metadata. It might be better to do like gallery and video and show the
> animation whenever it starts scanning, even if nothing new has been found.

I think we should do this. It's easy, and then we don't even need to hide the scanning progress when we're in sub-views/activities! My guess it that we'll still be wasting some time on showing progress (the spinner alone is about 10% of our time), but it'll be a lot closer to optimal. After that, the big perf improvements will hopefully be in the metadata parser and/or deviceStorage. :)

Comment 10

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Clearing ni since we discussed this on IRC. Let me know if there's anything else you need from me.

Comment 11

[Jim Porter (:squib)]

I think we're good, thanks! The plan is to show only a spinner, which neatly avoids this problem. The constantly-updating text isn't actually that useful and is just kind of distracting.

Comment 12

[Hema Koka [:hema]]

Adding to s1 milestone for tracking purpose as Jim is currently investigating this.

Comment 13

[BMO Automation]

Firefox OS is not being worked on