petkaantonov / bluebird

:bird: :zap: Bluebird is a full featured promise library with unmatched performance.
http://bluebirdjs.com
MIT License
20.45k stars 2.33k forks source link

Using bluebird.couroutine for async generator via babel stores tons of error stack traces in closure #1632

Open antonellil opened 4 years ago

antonellil commented 4 years ago

(This issue tracker is only for bug reports or feature requests, if this is neither, please choose appropriate channel from http://bluebirdjs.com/docs/support.html)

Please answer the questions the best you can:

1) What version of bluebird is the issue happening on? All versions

2) What platform and version? (For example Node.js 0.12 or Google Chrome 32) All platforms and versions

(Write description of your issue here, stack traces from errors and code that reproduces the issue are helpful)

Was digging into some memory profiling, and noticed that bluebird's coroutine creates a stack trace https://github.com/petkaantonov/bluebird/blob/49da1ac256c7ee0fb1e07679791399f24648b933/src/generators.js#L199 that is taking up a lot of memory during runtime image

In our case, we are leveraging the @babel/plugin-transform-async-to-generator plugin and have thousands of these error messages generated during runtime from the source code line above. This is amounting to dozens of extra megabytes on the JS heap that seem like they can be avoided. Can open a PR or otherwise resolve, but wanted to check first the reasoning for generating the error stack at this point, and if it can be behind a flag or otherwise how to prevent this memory issue?

benjamingr commented 4 years ago

but wanted to check first the reasoning for generating the error stack at this point, and if it can be behind a flag or otherwise how to prevent this memory issue?

It's there as a debugging tool in order to provide long stack traces in production when an exception is thrown. You can disable it by building bluebird with stack traces false or setting Promise.config({ longStackTraces: false }).

antonellil commented 4 years ago

thanks for the response. @benjamingr we have that value set to false and it is still generating these error traces. looking at this line of code where the problem is occuring on line 199 below: https://github.com/petkaantonov/bluebird/blob/49da1ac256c7ee0fb1e07679791399f24648b933/src/generators.js#L191-L209 It doesn't appear to take the value of longStackTraces into account here and it is indiscriminately creating them in the coroutine function.

benjamingr commented 4 years ago

Are you generating a lot of Promise.coroutines during runtime and retaining them?

You are expected to call Promise.coroutine on your methods (on the prototype) and then calling those methods - this leads to a low amount of captured stack traces (and low overhead).

You can use Promise.spawn for dynamic functions in the meantime (it's deprecated). If the use case is compelling enough we might un-deprecate it.

antonellil commented 4 years ago

@benjamingr okay interesting... so this might actually be an issue with this babel plugin https://babeljs.io/docs/en/babel-plugin-transform-async-to-generator

We're seeing it call this Promise.coroutine at runtime for every instance of an async function in our application, which is thousands and pretty significant.

antonellil commented 4 years ago

@benjamingr we've forked this in the meantime to have that stack trace behind the longStackTraces flag https://github.com/TeamGuilded/bluebird/blob/master/src/generators.js#L199

wondering if that would be a worthwhile addition to the bluebird itself?

benjamingr commented 4 years ago

@petkaantonov wdyt? ^ This sounds rather reasonable.