search

Vincit / tarn.js

Simple and robust resource pool for node.js
MIT License
472 stars 39 forks source link

Dropping support for node v6 and using async/await #25

Closed alubbe closed 5 years ago

alubbe commented 5 years ago

With v6 out of LTS and v12 introducing async stacktraces, would you at all be interested in refactoring the codebase by replacing all .then/.catch with async/await and try/catch for the performance & debugging improvements? If you are, I'll be happy to help out with a few PRs

alubbe commented 5 years ago

I just wanted to follow up with some more details about the improved debugging: https://v8.dev/blog/fast-async

And I've been asked about the performance impact of this. In v12, native async/await has caught up:

/bluebird $ ./bench doxbee

results for 10000 parallel executions, 1 ms per I/O op

file                                       time(ms)  memory(MB)
callbacks-baseline.js                           167       25.14
callbacks-suguru03-neo-async-waterfall.js       215       40.11
callbacks-caolan-async-waterfall.js             225       46.23
promises-bluebird-generator.js                  229       32.55
promises-bluebird.js                            263       49.07
promises-native-async-await.js                  269       54.89
promises-lvivski-davy.js                        280       89.95
promises-ecmascript6-native.js                  295       64.70
promises-cujojs-when.js                         303       59.53
promises-then-promise.js                        347       72.50
promises-tildeio-rsvp.js                        412       86.03
generators-tj-co.js                             491       62.53
promises-calvinmetcalf-lie.js                   499      139.76
promises-dfilatov-vow.js                        589      133.29
promises-obvious-kew.js                         635      114.16
streamline-generators.js                        687       90.89
promises-medikoo-deferred.js                    737      127.87
observables-pozadi-kefir.js                     799      153.48
streamline-callbacks.js                         945      118.20
observables-Reactive-Extensions-RxJS.js        1235      228.34
promises-kriskowal-q.js                        2762      484.91
observables-caolan-highland.js                 3158      455.48
observables-baconjs-bacon.js.js                4611      815.02

Platform info:
Darwin 18.5.0 x64
Node.JS 12.0.0
V8 7.4.288.21-node.16
Intel(R) Core(TM) i7-7567U CPU @ 3.50GHz × 4
adrianhelvik commented 5 years ago

I would also be interested in contributing to this, by code or review.

koskimas commented 5 years ago

This package is mostly designed for knex. Once knex drops support for node 6, tarn can do it too. Has it already done that?

kibertoad commented 5 years ago

Not yet, but likely after one more release in 0.16

elhigu commented 5 years ago

For knex we need to get all tests (mainly oracle has been broken for a while) back online and fix any bugs found during that to have the last node 6 compatible knex version to be as stable as possible.

alubbe commented 5 years ago

ok. I can't help with oracle, but when that's been fixed, I'm happy to start refactoring while keeping the tests green

elhigu commented 5 years ago

Node 6 support is now dropped. I'll release 1.2.0 briefly. I don't mind if async / await is used... but looking from the code in some cases bluebird promise stuff did seem more clear and easy to deal with than using async / await.