Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9254 )
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 Reviewed-on: http://gerrit.cloudera.org:8080/9254 Reviewed-by: Mike Percy <mpe...@apache.org> Tested-by: Kudu Jenkins --- M src/kudu/util/debug-util.cc 1 file changed, 70 insertions(+), 16 deletions(-) Approvals: Mike Percy: Looks good to me, approved Kudu Jenkins: Verified -- 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: merged Gerrit-Change-Id: Icb772f0607275ec340c7f38be05347fc14820cbd Gerrit-Change-Number: 9254 Gerrit-PatchSet: 7 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>