hexojs / hexo

A fast, simple & powerful blog framework, powered by Node.js.
https://hexo.io
MIT License
39.54k stars 4.86k forks source link

cleanup(wip): remove bluebird #5511

Closed talentlessguy closed 2 months ago

talentlessguy commented 4 months ago

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

github-actions[bot] commented 4 months ago

How to test

git clone -b remove-bluebird https://github.com/talentlessguy/hexo.git
cd hexo
npm install
npm test
SukkaW commented 4 months ago

https://github.com/hexojs/hexo/issues/5215#issuecomment-1564116657

talentlessguy commented 4 months ago

Ah crap, didn't see this. I'll switch to nativebird instead. Or close the PR?

SukkaW commented 4 months ago

Ah crap, didn't see this. I'll switch to nativebird instead. Or close the PR?

You can give nativebird a shot, and we can compare the performance. If there is no performance regression, we can continue!

talentlessguy commented 4 months ago

@SukkaW oh awesome, I'll revert some of my changes then and use nativebird

talentlessguy commented 4 months ago

Temporarily blocked by https://github.com/doodlewind/nativebird/pull/13

And a few other methods which implementations of I'll add to nativebird later

benjamingr commented 4 months ago

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

SukkaW commented 4 months ago

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.

benjamingr commented 4 months ago

@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 :]

SukkaW commented 4 months ago

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).

What I am really curious about is the bluebird performance. We've tried to replace bluebird 4 years ago: https://github.com/hexojs/hexo/pull/3939, https://github.com/hexojs/hexo/pull/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:

image

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 Promise.all is still 40% faster than the native Promise.all and consumes only 10% of the memory. That's why I always hesitated about replacing Bluebird.

And I know native Promise.all will always be slower since the native Promise.all is bound to the ECMAScript spec, while the Bluebird is only bound to the Promise A+ spec without strict runtime checks.

@benjamingr IMHO v8 could put more effort into improving Promise's performance. It has been 5 years and yet Bluebird still performs so well.

benjamingr commented 4 months ago

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).

talentlessguy commented 2 months ago

I guess this can be closed since Bluebird won't be removed due to perf reasons