Agree with Henry and Todd. At least in the past, compilers have been pretty bad at implementing atomics.
In addition, I'd prefer if we don't use std::atomic directly because it uses operator overloading. Code involving atomics is much easier to understand if the code itself shows the atomic operations, rather than relying only on variable declarations. Of course, now that we've switched to C++11, we could implement AtomicInt using std:atomic rather than gutil (if std::atomic proves to be robust). On Thu, Dec 8, 2016 at 5:20 AM, Todd Lipcon <[email protected]> wrote: > We looked at it briefly for Kudu and decided not to use it for > perf-sensitive cases at the time, based on the experience of Chromium. See > https://chromium-cpp.appspot.com/ under the listing for atomics. > > That said, I believe there's been some work in LLVM in later versions to do > some smarter optimizations of atomics that wouldn't be possible given the > volatile asm blocks that Impala and Kudu use today. In particular, volatile > asm prevents reordering of operations in both directions, whereas > Release/Store semantics should only prevent reordering in one. So, it may > be worth a shot, but worth testing carefully. > > -Todd > > On Thu, Dec 8, 2016 at 7:45 PM, Henry Robinson <[email protected]> wrote: > > > Not as far as I know. AtomicInt etc. were added at a time when Impala > > didn't support C++11. > > > > On 8 December 2016 at 03:10, Marcel Kornacker <[email protected]> > wrote: > > > > > We have an AtomicInt in common/atomic.h that's based on some gutil > > > libraries. Is there any reason to keep this instead of switching to > > > std::atomic? > > > > > > > > > > > -- > > Henry Robinson > > Software Engineer > > Cloudera > > 415-994-6679 > > > > > > -- > Todd Lipcon > Software Engineer, Cloudera >
