Skip to content
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

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

Open
antonellil opened this issue Dec 17, 2019 · 6 comments

Comments

@antonellil
Copy link

antonellil commented Dec 17, 2019

(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

var stack = new Error().stack;
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?

@antonellil antonellil changed the title Using bluebird.couroutine for async generator via babel stores tons of unnecessary error stack traces in closure Using bluebird.couroutine for async generator via babel stores tons of error stack traces in closure Dec 17, 2019
@benjamingr
Copy link
Collaborator

benjamingr commented Dec 18, 2019

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
Copy link
Author

antonellil commented Dec 18, 2019

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:

bluebird/src/generators.js

Lines 191 to 209 in 49da1ac

Promise.coroutine = function (generatorFunction, options) {
//Throw synchronously because Promise.coroutine is semantically
//something you call at "compile time" to annotate static functions
if (typeof generatorFunction !== "function") {
throw new TypeError(NOT_GENERATOR_ERROR);
}
var yieldHandler = Object(options).yieldHandler;
var PromiseSpawn$ = PromiseSpawn;
var stack = new Error().stack;
return function () {
var generator = generatorFunction.apply(this, arguments);
var spawn = new PromiseSpawn$(undefined, undefined, yieldHandler,
stack);
var ret = spawn.promise();
spawn._generator = generator;
spawn._promiseFulfilled(undefined);
return ret;
};
};

It doesn't appear to take the value of longStackTraces into account here and it is indiscriminately creating them in the coroutine function.

@benjamingr
Copy link
Collaborator

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
Copy link
Author

@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
Copy link
Author

@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
Copy link
Collaborator

@petkaantonov wdyt? ^ This sounds rather reasonable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants