Joe McDonnell has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/19185 )
Change subject: PROTOTYPE IMPALA-11694: Convert gutil atomic ops to C++ atomics ...................................................................... PROTOTYPE IMPALA-11694: Convert gutil atomic ops to C++ atomics Impala uses the gutil's implementation of atomic ops, which has several port-specific versions. As new processors come out and make improvements, these can require maintenance over time. When testing on ARM, we noticed that a test for a spinlock was dramatically slower (> 500x) and this is likely due to improper atomics implementations. The Chromium codebase switched to using the C++ atomics package for these implementations a few years ago in https://github.com/chromium/chromium/commit/57a4e4a50c673c25e9cdaab53e32f6e53aa0b574 This seems like a much more maintainable approach. On ARM, this fixes the spinlock issue. This is a prototype of adapting this change to the Impala codebase. This still needs more work, but it has passed Impala tests in the past. Known remaining work: 1. This will be taken over to the Kudu codebase's gutil as well, so we need to double-check that all APIs they use are covered. 2. Kudu has support for OS X, so this will need to be tested with x86_64 and the M1 chips. 3. It would be good to factor out the PauseCPU() function to its own header so it isn't mixed in with atomics. 4. This has diverged quite a bit from the Chromium change, so doing another pass going through why things became different would be good. Change-Id: I3d1e3f4e71988a6c464071c1cd8bdebce622e4b8 --- M be/src/common/init.cc M be/src/gutil/CMakeLists.txt M be/src/gutil/atomicops-internals-arm64.h D be/src/gutil/atomicops-internals-macosx.h A be/src/gutil/atomicops-internals-portable.h D be/src/gutil/atomicops-internals-powerpc.h D be/src/gutil/atomicops-internals-tsan.h D be/src/gutil/atomicops-internals-x86.cc M be/src/gutil/atomicops-internals-x86.h M be/src/gutil/atomicops.h D be/src/gutil/auxiliary/atomicops-internals-arm-generic.h D be/src/gutil/auxiliary/atomicops-internals-arm-v6plus.h D be/src/gutil/auxiliary/atomicops-internals-windows.h 13 files changed, 322 insertions(+), 3,300 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/85/19185/6 -- To view, visit http://gerrit.cloudera.org:8080/19185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3d1e3f4e71988a6c464071c1cd8bdebce622e4b8 Gerrit-Change-Number: 19185 Gerrit-PatchSet: 6 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]>
