Long call stack trace causes performance issue

[bigbobzhang]

Sequence.js
line 15:
this._callSite = (new Error).stack.replace(/.+\n/, '');

This line slow down every query. Without this line, I can get 3 to 4k rps, but with this i can only get 1k rps.

[tomasikp]

Interesting, is this necessary?

[felixge]

Seems to be slow indeed: https://gist.github.com/felixge/955659d1f1d8a530f2ff

The slowness seems to come from accessing the .stack property on the error, as v8 seems to go through considerable trouble in order to convert it from its internal representation to the string version it uses to report errors.

IMO the best fix for this would be to change the line to this._callSite = (new Error), and only access the .stack property if an error needs to be reported. If somebody was to send a patch for this, that'd be awesome!

[Acconut]

Maybe something like this:

Object.defineProperty(this, "_callSite", {
    get: function() {
         return (new Error).stack.replace(/.+\n/, '');
   }
});

[felixge]

@Acconut that could do the trick, but IMO a simple if statement in the right place would be more appropriate than getter/setter voodoo. That being said, I'd be happy to pull in any patch solving this issue : )

[Acconut]

If _callSite is only used in Sequence.js, I think I have a solution. Or is it used anywhere else?

[tomasikp]

@Acconut the only references to _callSite that are found are in Sequence.js

• Line 15
• Line 44

[dresende]

Merged :)

[bigbobzhang]

Great. i tried the patch, i got 2k-3.5k rps. If i just set ._callSite=''; I can get 4k more rps. since it's just for error report, an option to disable it may be better.

[felixge]

@bigbobzhang you're saying you get 2k-3.5k with this patch and 4k with disabling the _callSite stuff all together? If that's the case, there is probably still room for improvement here.

[Acconut]

Maybe if you would define the error in _addLongStackTrace and not in the constructor. Or would this interfere with the stack?

[bigbobzhang]

@felixge yes, it seems the error object works in a low performance way in v8. I understand the _callSite is used to capture the stack where the query was called，but itˊs too waste to run the code in a deployment env. my opinion is enable it in dev env, after debug &test, disable it in production env. Thanks for the great lib node mysql.

[dresende]

Perhaps only available when debug option is on? Or by looking at an environment variable... or both..

[Acconut]

What's the problem if we construct the error in line 44:

err.stack += delimiter + (new Error).stack.replace(/.+\n/, '');

Is then the stack value wrong and not helpful?