Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9835 )

Change subject: KUDU-2384. Don't collect thread stacks when running under 
debugger
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9835/1/src/kudu/util/debug-util.cc
File src/kudu/util/debug-util.cc:

http://gerrit.cloudera.org:8080/#/c/9835/1/src/kudu/util/debug-util.cc@728
PS1, Line 728:   if (IsBeingDebugged()) {
> Why do this here and not inside StackTraceCollector::TriggerAsync? Between
TriggerAsync is called for each thread. Even though reading /proc/self/status 
is quick, I'd rather not have to re-read it hundreds of times in a row.

For reference, on el6 I got:
In [1]: %timeit file("/proc/self/status").read()
100000 loops, best of 3: 12.6 µs per loop

On el7 I got:
In [3]: %timeit file("/proc/self/status").read()
10000 loops, best of 3: 28.7 µs per loop

On my Ubuntu laptop I got:
In [2]: %timeit file("/proc/self/status").read()
10000 loops, best of 3: 38.8 µs per loop

(it appears to be getting slower as more info gets added to it in new kernel 
versions).

Anyway, with 1000 threads, 38us/loop makes 38ms of overhead, which is a 
significant enough "skid" for use cases like /stacks that makes it less useful 
(eg more likely to see 'impossible" situations where two threads are holding 
the same lock due to the skew between the time when the stacks were collected)


http://gerrit.cloudera.org:8080/#/c/9835/1/src/kudu/util/os-util.cc
File src/kudu/util/os-util.cc:

http://gerrit.cloudera.org:8080/#/c/9835/1/src/kudu/util/os-util.cc@166
PS1, Line 166:  = -1
> Nit: this initialization is unnecessary, right?
yea but it seems that gcc likes to give warnings about use of uninitialized 
variables in the case that a var is initialized by out-param as is the case 
here.



--
To view, visit http://gerrit.cloudera.org:8080/9835
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib17ffb9514e9b440b8d97843374d2a60913a398b
Gerrit-Change-Number: 9835
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Wed, 28 Mar 2018 21:03:52 +0000
Gerrit-HasComments: Yes

Reply via email to