1090966 - The LazyLoad library should return a promise

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Julien Wajsberg [:julienw]]

LazyLoader should return a promise in addition to take a callback.

I think we can easily support both methods until all applications stop using the callback.

Comment 1

[Julien Wajsberg [:julienw]]

Attachment: github PR

Hey Kevin,

I wanted to do this for a long time :) It will make it easier to handle navigation and lazy loading in SMS app if we can have promises in the whole chain.

Thanks!

Comment 2

[Kevin Grandon :kgrandon]

Thanks Julien! This is a change I support, I just figured no one had time to actually do this :) If we go down this route, I think we should make the push to use promises *everywhere* that LazyLoader is used. I've filed bug 1091014 for this. Thank you for getting the ball rolling on this, I will look at the patch soon.

Comment 3

[Kevin Grandon :kgrandon]

Comment on attachment 8513527 [details] [review]
github PR

Julien - this looks good, but my only concern is about performance. What do you think about the external interface returning a promise, but continuing to use callbacks for internal methods?

Comment 4

[Julien Wajsberg [:julienw]]

I don't think there would be any performance implication.

We can check with gecko developers (NI :nsm), but I think Promises use a microtask-like behavior, so there should be no delay between the end of a function and running the Promise's resolution handler if the promise is resolved already.

I think it would be a pity to not use the power and simplicity of promises because of supposed performance drawbacks :/

Comment 5

[Kevin Grandon :kgrandon]

From what I've seen anytime we have promises in a loop, or need multiple of them they are slower than straight callbacks (see the simple jsperf test on github). I agree that it's a shame if we can't use them, but I also think performance is critical for something like lazy_loader which almost every app uses, and something we should absolutely be fast as possible at.

Comment 6

[Julien Wajsberg [:julienw]]

Nikhil, especially check the jsperf test in [1]. Thanks!

[1] http://jsperf.com/promises-overhead-in-loop/2

Comment 7

[Julien Wajsberg [:julienw]]

(In reply to Kevin Grandon :kgrandon from comment #5)
> From what I've seen anytime we have promises in a loop, or need multiple of
> them they are slower than straight callbacks (see the simple jsperf test on
> github). I agree that it's a shame if we can't use them, but I also think
> performance is critical for something like lazy_loader which almost every
> app uses, and something we should absolutely be fast as possible at.

I'd agree if our numbers were big, but we don't load more than 10-30 files in an application at once, and it's never done in a loop.

Still, I can make your requested changes if you want to!

Comment 8

[Julien Wajsberg [:julienw]]

Attachment: github PR

Here is a new PR with a change to only the main interface

The nice thing is: the unit tests are the same than in the previous PR :D

Comment 9

[Kevin Grandon :kgrandon]

Comment on attachment 8513622 [details] [review]
github PR

Sorry for the extra work here, this looks good to me. I'm not opposed to landing the original one if we can prove that there is a negligible perf impact. Since they have the same external interface, we can totally land this, and work on the internals in parallel. Thanks!

Comment 10

[Julien Wajsberg [:julienw]]

I'll wait for a full try run ;)

Comment 11

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

I agree that promises will be slower than callbacks (as julien's test also shows). If your internal architecture uses callbacks and you don't have problems with it, it isn't worth the effort to change it. Promises suffer from perf due to their (sometimes seemingly excessive) asyncness and the fact that they aren't implemented in SpiderMonkey, nor optimized to any degree.

Comment 12

[Julien Wajsberg [:julienw]]

master: 4ee0a68bc71b00808313b21ebd7c3b6a60e98d14