You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@impala.apache.org by "Dan Hecht (Code Review)" <ge...@cloudera.org> on 2016/03/10 00:36:43 UTC

[Impala-CR](cdh5-trunk) IMPALA-3136: Use gutil SpinLock

Dan Hecht has uploaded a new patch set (#4).

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
defintions 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
---
M be/src/benchmarks/lock-benchmark.cc
M be/src/gutil/CMakeLists.txt
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
20 files changed, 5,145 insertions(+), 283 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/07/2507/4
-- 
To view, visit http://gerrit.cloudera.org:8080/2507
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib1e5322df1d5936ee9679cdc0031772b5f44b135
Gerrit-PatchSet: 4
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>