Dan Hecht has posted comments on this change. Change subject: Remove AMD Opteron Rev E workaround from atomicops ......................................................................
Patch Set 1: > Just passing by. Re: "make out Atomic class use these functions" - > wouldn't we prefer to depend on std library atomic operations once > C++11 allows us to do so? Are these implementations superior? I don't think we should use C++11 atomics directly, because they provide operator overloads and I think this makes code that relies on atomic operations too difficult to read (this is one problem with our current AtomicInt class which I plan to address). One option will be to eventually base our new AtomicInt class on C++11, but I don't take it for granted that compiler writers get atomics correct/performant. There's been a lot of issues with the various compiler support for atomics in the past. The nice thing about gutil's atomic is it's easy to see what assembly code you get. But, we can evaluate using C++11 atomics under the covers when we have C++11 support. Let's discuss more in that patch (which I haven't yet posted). -- To view, visit http://gerrit.cloudera.org:8080/2516 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3639dcf86c14778967c0079b8dbc222a4516cf05 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Dan Hecht <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: No
