es6-shim icon indicating copy to clipboard operation
es6-shim copied to clipboard

Promise performance: Most time spent in defineProperties

Open gwicke opened this issue 10 years ago • 9 comments

Here is a screenshot of a profile of a HTTP server using es6-shim promises on Node 0.10 (created using https://github.com/gwicke/nodegrind):

es6-0 10

The stand-out method is defineProperties, with 62% of all CPU time spent in es6-shim. Any optimizations in that area would likely have a big impact on es6-shim promise performance.

gwicke avatar Jul 17 '14 15:07 gwicke

The Promise implementation probably needs a bit of maintenance to match the latest es6 spec, where a bunch of functions were simplified. (As a result of a decision to abandon support for "monadic promises".) We're using 'defineProperties' in order to hide our implementation fields; I'll have to look closely at how we might reduce its use.

cscott avatar Jul 17 '14 15:07 cscott

Any interest in using es6-promise as the Promise implementation?

xtian avatar Jul 18 '14 17:07 xtian

rsvp was slower than the existing es6-shim promise implementation, last time I benchmarked it. More benchmarking always welcome. It's not clear that es6-promise supports subclassing, either.

cscott avatar Jul 18 '14 18:07 cscott

Oh, and I'll note for the record that es6-shim in general values spec-correctness over raw performance. The current es6-shim promise implementation follows the spec text very closely (or did at the time it was written -- and that's what the needed "bit of maintenance" will fix). Although I have submitted and landed patches to Map/Set/etc to improve performance, at the cost of making the correspondence with the es6 spec less direct, generally es6-shim will not chase the absolute best performance if that means significantly complicating the code base. For example, the fastest Promise implementation out there is bluebird. It is truely impressive -- but if you look through the code, it will be clear that that amount and complexity of code has no place in es6-shim.

If you want the best possible performance, load something like bluebird before es6-shim (although your spec compliance may suffer).

(All that said, when I update es6-shim to match the latest spec, I will look to see if there is any low-handing fruit that might further improve performance.)

cscott avatar Jul 18 '14 18:07 cscott

I agree that spec correctness should be prioritized over performance. Why do you bring up benchmarks then? Does Promise implementation speed actually make a tangible difference in real-world async situations?

I suggested it only as an alternative to maintaining yet another Promise implementation.

xtian avatar Jul 18 '14 19:07 xtian

Well, I brought up benchmarks because that's the title of this bug. If you think that using es6-promise instead of our own code is a good idea for some other reason, feel free to open a new bug with an appropriate title.

cscott avatar Jul 18 '14 20:07 cscott

Any news on this? I think there could be value in shifting in https://github.com/jakearchibald/es6-promise

It's regularly maintained (347 commits), performant and tracks the spec closely.

sandstrom avatar Oct 07 '14 13:10 sandstrom

It doesn't actually track the spec closely, it just uses RSVP's implementation -- which was significantly slower that ours last time I checked. (Redoing the benchmarks is worthwhile if anyone is interested.)

The problem is that RSVP implements a lot of stuff which isn't in the spec. We need a more minimal implementation.

cscott avatar Oct 07 '14 14:10 cscott

@cscott I see, you know this better than myself, so I trust your judgement.

sandstrom avatar Oct 07 '14 14:10 sandstrom