Hello Will Berkeley, Mike Percy, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9254
to look at the new patch set (#6).
Change subject: KUDU-2291 (part 3): use futex to speed up stack collection
......................................................................
KUDU-2291 (part 3): use futex to speed up stack collection
Previously the thread which requests a stack trace would poll on an
atomic variable waiting for the stack to be collected. Of course
spinning is too CPU-hungry so it would actually sleep for 10ms in
each iteration. This limited the performance of stack collection to
about 100/second, which is problematic for the /stacks web page on a
server that may have thousands of threads.
This switches to using the futex system call to allow the dumper thread
to sleep and the dumpee thread to wake it back up when the results are
ready.
Why do we use the low level futex and not something like a condition
variable or CountdownLatch? The issue is that the dumpee thread is
running in a signal handler context, and most things are not safe to do
in that context. Particularly, mutexes may block, which is very naughty,
and condition variables and latches are currently implemented on top of
mutexes. pthread semaphores are another option, but those don't support
proper timeout semantics, which is a feature we use in this use case.
Whether the futex syscall itself is async-signal-safe is up for some debate.
Clearly the "wait" mode is not, since it may block, but is the "wake" mode
that we use here allowed? Technically I don't think so. In practice,
the pthread sem_post() API _is_ documented to be async-signal-safe and
I verified that in libc source it's implemented using futex to wake the
waiter. So, if we are doing something illegal, at least we're doing
the same illegal thing as libc, so we're very unlikely to ever break.
Obviously futex is not portable to macOS, but this code was already
Linux-specific so we haven't lost much.
This speeds up the benchmark for stack trace collection (without symbolization)
by 585x:
Before:
I0207 19:33:20.960477 30799 debug-util-test.cc:175] Throughput: 99
dumps/second (symbolize=0)
I0207 19:33:21.963946 30799 debug-util-test.cc:175] Throughput: 84
dumps/second (symbolize=1)
After:
I0207 19:38:32.505213 32039 debug-util-test.cc:175] Throughput: 57916
dumps/second (symbolize=0)
I0207 19:38:33.506417 32039 debug-util-test.cc:175] Throughput: 595
dumps/second (symbolize=1)
Change-Id: Icb772f0607275ec340c7f38be05347fc14820cbd
---
M src/kudu/util/debug-util.cc
1 file changed, 70 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/9254/6
--
To view, visit http://gerrit.cloudera.org:8080/9254
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icb772f0607275ec340c7f38be05347fc14820cbd
Gerrit-Change-Number: 9254
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com>
