Use long stack traces only when debug is true

[sidorares]

new Error is extremely slow ( 200k per sec on my mb air ) and callSite is saved in every sequence ( https://github.com/felixge/node-mysql/blob/master/lib/protocol/sequences/Sequence.js#L15 ). Maybe it makes sense to only save callSite when connection debug is set to true. I'll post benchmarks with/without callSite here later.

[felixge]

I recall benchmarking this and not being worried about performance. That being said, my memory is fuzzy, and it's always possible that v8 performance has changed.

200k/sec doesn't sound great, but OTOH which use case would be limited by this? Nobody sane would do bulk inserts with one query per insert. IMO the lack of long stack traces is a huge problem when developing node apps, and this feature has allowed many users to debug their own issues rather than show up here with unintelligible bug reports.

So generally speaking I'd really like to keep this on by default, but I can be convinced otherwise.

[sidorares]

200k/sec is not 200k queries/sec performance - it's 200k new Error per sec, when cpu is 100% busy. Each one adds 0.5 ms to latency which adds when under load

[dougwilson]

I agree that it is slow to generate the stack trace, but it is also extremely valuable. I don't think we should only enable it with debug, though, as that also prints a lot of information about the packets. How about adding an option to disable the long stack traces, but having it false by default? This way people who do nothing (and would arguably benefit more from them) would have long stack traces, but people could make a decision to disable them to eek out some more performance.

I'm sure generating these doesn't really add much overhead to query workloads, but it does take away bits of time doing other activities, so a web server doing MySQL queries wouldn't see queries returning faster, but would see higher concurrency for request processing.

[jdduncan]

In our performance testing, a full 8% to 9% of the total run time of the benchmark application is spent here in this Error constructor.

[papajdo]

I'd be happy with a separate flag (not debug) to enable/disable creating the Error object on every sequence. I understand the value but I'd hate to have folks naively test performance and not realize that 8% is chopped right off the top. Maybe a "trace" flag would do the trick.

[felixge]

I'm ok with a flag being added that allows people to disable long stack traces.

@jdduncan can you provide more information about your benchmark?

200k/sec is not 200k queries/sec performance - it's 200k new Error per sec, when cpu is 100% busy. Each one adds 0.5 ms to latency which adds when under load

I didn't have coffee yet, so bear with me here. Isn't 1s = 1000ms, and 1000ms / 200000 = 0.005ms?

[sidorares]

you are right (my "0.5 ms" was posted before coffee :) ) 1ms delay = 1k ops/s, so it's more like 0.005 ms

[jdduncan]

@felixge From https://github.com/mysql/mysql-js at perftest/jscrund.js we ran the "find" test in "indy" mode with -m (use mysql adapter) and -r -1 (run forever). Then we used Joyent's flamegraph toolchain to profile it for 60 seconds.

[papajdo]

Excellent. Thanks, all.

Craig

On Jan 16, 2014, at 10:34 PM, Andrey Sidorov wrote:

Closed #710 via #713.

—
Reply to this email directly or view it on GitHub.

Craig L Russell
Architect, Oracle
http://db.apache.org/jdo
408 276-5638 mailto:[email protected]
P.S. A good JDO? O, Gasp!