Closed Bug 703336 Opened 13 years ago Closed 12 years ago

Regroup JS async utilities

Categories

(Core :: General, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 692420

People

(Reporter: Yoric, Unassigned)

Details

We seem to have async utilities spread in a number of places around the code. We should gather them as a sole -- and discoverable -- module called, say, AsyncUtils.jsm.
Starting a list:
- Sync has an async modules
- browser/devtools/shared/Promise.jsm
- a number of reimplementations of setTimeout and/or executeSoon.
(In reply to David Rajchenbach Teller [:Yoric] from comment #1)
> Starting a list:
> - Sync has an async modules

Only Async.chain() from that module may be of use. Please do not put the event-loop spinning helpers in a more prominent place. Event loop spinning is evil and we've been trying to rid Sync of it for the past year or so.

> - browser/devtools/shared/Promise.jsm

That looks really nice. I can see some additions to that, e.g. Promise.serially, Promise.barriered(), etc. Sadly, this promise stuff is probably going to be allocation-happy for Sync's tight loops. (We used to have and to a certain degree still have problems with too much GC pressure.)

> - a number of reimplementations of setTimeout and/or executeSoon.

Yes! I can recommend uplifting Sync's Utils.nextTick() [1]. This is, in some variant or another, also sometimes called executeSoon() elsewhere, which is a rather poor name if you ask me ("soon? how soon?"). Any of nextTick(), executeNext(), queue() would be good IMHO. Point is, it should reflect that we're adding stuff to the event queue. "Soon" for me has a timing connotation.

[1] https://mxr.mozilla.org/mozilla-central/source/services/sync/modules/util.js#849
(In reply to Philipp von Weitershausen [:philikon] from comment #2)
> (In reply to David Rajchenbach Teller [:Yoric] from comment #1)
> > Starting a list:
> > - Sync has an async modules
> 
> Only Async.chain() from that module may be of use. Please do not put the
> event-loop spinning helpers in a more prominent place. Event loop spinning
> is evil and we've been trying to rid Sync of it for the past year or so.

Good to know.

> > - browser/devtools/shared/Promise.jsm
> 
> That looks really nice. I can see some additions to that, e.g.
> Promise.serially, Promise.barriered(), etc. Sadly, this promise stuff is
> probably going to be allocation-happy for Sync's tight loops. (We used to
> have and to a certain degree still have problems with too much GC pressure.)

Gasp. I am now making large use of Promise in the Schedule API and in OS.File. I hope that I will not encounter the same kind of problems.

By the way, Promise.serially and Promise.barriered are now implemented in the Schedule API: Bug 692420.

> > - a number of reimplementations of setTimeout and/or executeSoon.
> 
> Yes! I can recommend uplifting Sync's Utils.nextTick() [1]. This is, in some
> variant or another, also sometimes called executeSoon() elsewhere, which is
> a rather poor name if you ask me ("soon? how soon?"). Any of nextTick(),
> executeNext(), queue() would be good IMHO. Point is, it should reflect that
> we're adding stuff to the event queue. "Soon" for me has a timing
> connotation.
> 
> [1]
> https://mxr.mozilla.org/mozilla-central/source/services/sync/modules/util.
> js#849

Good point. The Schedule API offers |Schedule.soon| which implements both |nextTick| and |setTimeout|. |Schedule.queue| could indeed be clearer.
(In reply to David Rajchenbach Teller [:Yoric] from comment #3)
> > > - browser/devtools/shared/Promise.jsm
> > 
> > That looks really nice. I can see some additions to that, e.g.
> > Promise.serially, Promise.barriered(), etc. Sadly, this promise stuff is
> > probably going to be allocation-happy for Sync's tight loops. (We used to
> > have and to a certain degree still have problems with too much GC pressure.)
> 
> Gasp. I am now making large use of Promise in the Schedule API and in
> OS.File. I hope that I will not encounter the same kind of problems.

I wouldn't worry about it too much, but it's good to be aware of it. Sync is a pretty extreme case: it unmarshals, transforms, and applies potentially thousands of records in a few seconds. We've found that this can create pretty long GC pauses. Though the GC has been getting better and better and with stuff like incremental and generational GC, it will be less of a problem.

Still, an object allocated is an object that will have to be cleaned up eventually. Best to avoid what you can, especially when the potential use is in a critical path (e.g. startup). See http://philikon.wordpress.com/2011/03/09/javascript-perf-avoid-creating-objects-2/ for my detailed blog post on this topic.

> By the way, Promise.serially and Promise.barriered are now implemented in
> the Schedule API: Bug 692420.

Nice!

> > > - a number of reimplementations of setTimeout and/or executeSoon.
> > 
> > Yes! I can recommend uplifting Sync's Utils.nextTick() [1]. This is, in some
> > variant or another, also sometimes called executeSoon() elsewhere, which is
> > a rather poor name if you ask me ("soon? how soon?"). Any of nextTick(),
> > executeNext(), queue() would be good IMHO. Point is, it should reflect that
> > we're adding stuff to the event queue. "Soon" for me has a timing
> > connotation.
> 
> Good point. The Schedule API offers |Schedule.soon| which implements both
> |nextTick| and |setTimeout|. |Schedule.queue| could indeed be clearer.

Yeah, Sync too had a utility that would do both things, depending on what you would pass. We changed that to be clearer. (Also, timers are not a good tool when you really want to put something on the event queue: they have a minimal delay of ~5ms, I think.)
Most of the features have now been implemented as part of the Schedule API. Closing this bug.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.