-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cleanup(wip): remove bluebird #5511
Conversation
How to testgit clone -b remove-bluebird https://github.com/talentlessguy/hexo.git
cd hexo
npm install
npm test |
Ah crap, didn't see this. I'll switch to nativebird instead. Or close the PR? |
You can give |
@SukkaW oh awesome, I'll revert some of my changes then and use nativebird |
Temporarily blocked by doodlewind/nativebird#13 And a few other methods which implementations of I'll add to nativebird later |
Just refactor the bb specific stuff instead of wrapping it? You don't need Promise.asCallback when you have util.callbackify built in, Promise.promisify with util.promisify etc |
The Hexo was born way before ES Promise was available/stable, so way too many Bluebird-specific API usages here and there in the Hexo codebase. A wrapper can ease the transition. |
@SukkaW IMO just rip the bandaid, bluebird isn't getting major updates. We worked hard to make sure there are alternatives for every API we had in bluebird (I'm a maintainer) in Node.js (I'm.. also a maintainer). Most stuff should just be refactored to native (e.g. Promise.method to async functions), other stuff to platform alternatives e.g. AbortSignal/AbortController for cancellation, util.promisify for converting callback functions, streams for map+concurrency etc. The amount of stuff actually missing is very small (stuff like typed catch) and can be relatively easily worked around. Also yes, I realize bluebird has no dependencies, we didn't want risk and to slow things down in the library - the library also has internal build tooling to reduce its size and improve its speed by not having a lot of runtime checks :] |
What I am really curious about is the bluebird performance. We've tried to replace bluebird 4 years ago: #3939, #4019. Our internal benchmark shows that there was about 175% performance regression at the moment. To this day, bluebird still has comparable (where Bluebird is 2% faster) performance with the native promise in sequential executing while consuming 50% of the memory: The benchmark suite can be found here: https://github.com/SukkaW/promise-performance-tests (originated from v8's promise benchmark suite). Also, Hexo heavily relies on parallel job queueing, and as shown above, Bluebird's And I know native @benjamingr IMHO v8 could put more effort into improving Promise's performance. It has been 5 years and yet Bluebird still performs so well. |
V8's benchmarks actually originate from Bluebird's which obviously show how good our promise performance is in various cases that may or may not matter to users :D Doxbee is actually pretty good and isolates promise overhead in a real world servers but if you look at the numbers from 10 years ago the numbers we have today (in both cases) are "so good" it shouldn't matter. Namely - you're right in that bluebird cuts a lot of edges around scheduling and edge cases the V8 promise does not (for pretty funny historical reasons tbh). If native promise performance is still an issue for you (we worked on it with the v8 team quite a bit back then) it can be discussed further (feel free to open an issue at the nodejs/performance repo and I promise to engage and ping v8 people). |
I guess this can be closed since Bluebird won't be removed due to perf reasons |
What does it do?
Removes Bluebird and replaces it with native Promise API. Promises have been in Node since 0.12 so there's no need to use it anymore.
Needs a PR to https://github.com/hexojs/warehouse as well
Pull request tasks