Kevin Grandon :kgrandon Reporter
	Description • 10 years ago

We currently load init.json every time if VersionHelper.isUpgrade returns false. If we already have a configuration saved, we should not need to load and parse this file.

My guess is that we could save between ~20-40ms off initial startup.

	Kevin Grandon :kgrandon Reporter
	Comment 1 • 10 years ago

This that we might be able to improve upon:

 - Do not always load init.json
 - Streamline or remove datastore preference access.
 - Consider caching navigator.getFeature

	Kevin Grandon :kgrandon Reporter
	Comment 2 • 10 years ago

Turns out most of the calls in this file are rather tiny - the bulk of the delay here comes from VersionHelper(). Seems like we should ditch it.

	Johan Lorenzo [:jlorenzo] - On PTO, back on September 11th
	Comment 3 • 10 years ago

Nominating as per bug 1037706 comment 28.

	Kevin Grandon :kgrandon Reporter
	Comment 4 • 10 years ago

I think we need to profile to determine performance savings amount before we block on this.

	Cristian Rodriguez (:crdlc) Assignee
	Comment 5 • 10 years ago

I gonna try do something here during morning

	Cristian Rodriguez (:crdlc) Assignee
	Comment 6 • 10 years ago

Hi Kevin, the purpose of this patch is to reduce the startup time. The idea is not to load "icc_helper.js", "shared/js/version_helper.js" and "js/configurator.js" when the home has been initialized. Can you take some measure here? I think that we have obtained almost 1 second. My hamachi is much faster :)

	Kevin Grandon :kgrandon Reporter
	Comment 7 • 10 years ago

Comment on attachment 8460110 [details]
Configuration, version helper and SV only for first time

I'm currently measuring to access impact, but the code seems solid to me and should provide a nice boost. I would also like to confirm with Carmen that SV customization should continue to work. Thanks!

	Lucas Adamski [:ladamski]
	Comment 8 • 10 years ago

This is a dependency of a CAF bug but isn't blocking 2.0.  Should it be?

	Jason Smith [:jsmith]
	Comment 9 • 10 years ago

(In reply to Lucas Adamski [:ladamski] (plz needinfo) from comment #8)
> This is a dependency of a CAF bug but isn't blocking 2.0.  Should it be?

Kevin - What do you think?

	Kevin Grandon :kgrandon Reporter
	Comment 10 • 10 years ago

[Blocking Requested - why for this release]:

(In reply to Jason Smith [:jsmith] from comment #9)
> (In reply to Lucas Adamski [:ladamski] (plz needinfo) from comment #8)
> > This is a dependency of a CAF bug but isn't blocking 2.0.  Should it be?
> 
> Kevin - What do you think?

I don't think we can make blocking decisions until we get a proper measurement here. I think our options are to either unblock until we get a measurement, or if it would make people feel better, let's just block and assume this will provide a slight boost.

It looks like this should be good though and I'll do some measurements tomorrow. Feel free to block for now, thanks.

	Kevin Grandon :kgrandon Reporter
	Comment 11 • 10 years ago

I measured and this appears to save us around 200ms. It's worth taking that for 2.0.

	Cristian Rodriguez (:crdlc) Assignee
	Comment 12 • 10 years ago

1) Delay the drag-drop module after rendering icons and load it lazily
2) Move "sort" method in item.js to "save" method that is only used the first time so we can avoid to parse it next times

These are some ideas that don't hurt us and we can gain some milliseconds thought

	Kevin Grandon :kgrandon Reporter
	Comment 13 • 10 years ago

Comment on attachment 8460787 [details]
Dragdrop feature loaded lazily

I'm confused about moving the save method - I guess the idea is that this moves performance from parse-time to runtime? I'm not really sure if it's worth the readability hit, we should measure the impact of it I think?

Happy about the dragdrop changes, we could also lazy load it inside the collection app I guess.

	Cristian Rodriguez (:crdlc) Assignee
	Comment 14 • 10 years ago

(In reply to Kevin Grandon :kgrandon from comment #13)
> Comment on attachment 8460787 [details]
> Just another idea
> 
> I'm confused about moving the save method - I guess the idea is that this
> moves performance from parse-time to runtime? I'm not really sure if it's
> worth the readability hit, we should measure the impact of it I think?

I answered in github

> 
> Happy about the dragdrop changes, we could also lazy load it inside the
> collection app I guess.

Yes, it is done in the pr

	Kevin Grandon :kgrandon Reporter
	Comment 15 • 10 years ago

Comment on attachment 8460787 [details]
Dragdrop feature loaded lazily

Looks good to me, can you be sure to fix the unit tests before landing? Thanks!

	Carmen Jimenez Cabezas
	Comment 16 • 10 years ago

Comment on attachment 8460110 [details]
Configuration, version helper and SV only for first time

I've tested this with and without single variant and with and without a valid SIM present on first run. 
It works perfectly, and the patch looks good to me.

	Cristian Rodriguez (:crdlc) Assignee
	Comment 17 • 10 years ago

First patch (Configuration, version helper and SV only for first time) merged in master:

https://github.com/crdlc/gaia/commit/bb5e48c0536703b89ef8020bd5bb18ca38283301

	Cristian Rodriguez (:crdlc) Assignee
	Comment 18 • 10 years ago

Second patch (Load drag-drop lazily) merged in master:

https://github.com/crdlc/gaia/commit/d96402f14f4e17659e9a8126c0f17f944f6e9112

	Cristian Rodriguez (:crdlc) Assignee
	Comment 19 • 10 years ago

v2.0:

https://github.com/mozilla-b2g/gaia/commit/d1437e4c4246a7e8c7e16001f4d6bd7d61e9da60
https://github.com/mozilla-b2g/gaia/commit/abdb110c6421c53221ab2d59ddb536bfc062e7a8

	bhavana bajaj [:bajaj]
	Comment 20 • 10 years ago

Kevin, do we have the updated measurements here which you were planning to do to see how much improvement the patch gives in 2.0 ?

	Kevin Grandon :kgrandon Reporter
	Comment 21 • 10 years ago

(In reply to bhavana bajaj [:bajaj] from comment #20)
> Kevin, do we have the updated measurements here which you were planning to
> do to see how much improvement the patch gives in 2.0 ?

This patch is already in 2.0. In comment 11 we measured the savings as ~200ms.