Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-3136: Use gutil SpinLock ......................................................................
IMPALA-3136: Use gutil SpinLock Impala's SpinLock implementation does a busy wait (it yields, but won't block). This is bad because if the lock holder gets delayed (say, due to extra work in malloc), then a bunch of threads can start spinning (stealing cycles from the lock holder) and we can fall off a cliff. Instead, let's use gutil's SpinLock implementation. That implementation is adaptive -- it'll spin for a little while but then fall back to waiting on a futex. Modify the lock-benchmark to include the no-contention single threaded case, which is an important case (locks should usually be uncontended). (On my machine, the previous SpinLock implementation was actually slower than boost::mutex in this case). The new lock is a bit slower in the contended case, but that's expected since the slow path will be a bit more complicated in order to deal with waiters. The added and updated gutil files come from the kudu master branch (044b2c1) with modifications only to the include file paths (remove 'kudu' directory), with one exception: dynamic_annotations.h has symbol definitions that conflict with some llvm headers (magic symbols used by valgrind). Avoid these conflicts with a slight change to dynamic_annotations.h to prefer the llvm way of doing things (weak symbols) when available. Change-Id: Ib1e5322df1d5936ee9679cdc0031772b5f44b135 Reviewed-on: http://gerrit.cloudera.org:8080/2507 Reviewed-by: Tim Armstrong <[email protected]> Reviewed-by: Dan Hecht <[email protected]> Tested-by: Internal Jenkins --- M be/src/benchmarks/lock-benchmark.cc M be/src/gutil/CMakeLists.txt M be/src/gutil/atomicops-internals-x86.h M be/src/gutil/dynamic_annotations.c M be/src/gutil/dynamic_annotations.h M be/src/gutil/once.cc A be/src/gutil/spinlock.cc A be/src/gutil/spinlock.h A be/src/gutil/spinlock_internal.cc A be/src/gutil/spinlock_internal.h M be/src/gutil/spinlock_linux-inl.h M be/src/gutil/spinlock_posix-inl.h D be/src/gutil/spinlock_wait.cc D be/src/gutil/spinlock_wait.h A be/src/gutil/synchronization_profiling.h A be/src/gutil/sysinfo.cc A be/src/gutil/sysinfo.h A be/src/gutil/valgrind.h M be/src/util/CMakeLists.txt D be/src/util/spinlock.cc M be/src/util/spinlock.h 21 files changed, 5,146 insertions(+), 284 deletions(-) Approvals: Internal Jenkins: Verified Dan Hecht: Looks good to me, approved Tim Armstrong: Looks good to me, but someone else must approve -- To view, visit http://gerrit.cloudera.org:8080/2507 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ib1e5322df1d5936ee9679cdc0031772b5f44b135 Gerrit-PatchSet: 7 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Dan Hecht <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Tim Armstrong <[email protected]>
