Using C++11+ in hotspot
Martin Buchholz suggested the topic and Mikael signed up to lead the session. Martin gave an introduction. He had observed some issues recently (“impossible null pointer exceptions”) which after investigation turned out to be caused by a toolchain upgrade and in turn revealed the fact that some code in hotspot requires atomicity but does not make this requirement very explicit and in the end assumes that the C++ compiler will produce suitable code to guarantee atomicity. Martin also observed that (on linux) hotspot is compiled targeting the c++98 standard, which is old enough to not even mention the concept of threads. Mikael also added that for extra fun the story is different on different platforms and toolchains. A quick poll in the room showed no immediate resistance to upgrading to a more recent C++ standard. It was noted that upgrading/changing the target standard is separate from actively starting to use new functionality, and that not all functionality may be well suited for, or even compatible with Java’s memory model. There are caveats with using toolchain intrinsics, C++ atomics, etc. More investigations and discussions are needed to decide on what subset of the C++ functionality to pick up and use. Andrew Haley noted that the race conditions Martin had observed is an example of the more broad use/reliance on “undefined behavior” which exists in the hotspot code, acknowledging that some things are unlikely to ever change (the use of no-strict-aliasing was mentioned). He polled the room for thoughts on removing/rewriting code relying on undefined behavior, and the poll indicated that people were open to such improvements. Mikael mentioned that he has done some initial work to make hotspot build successfully with C++14 on linux, mac, and windows (all x64). One change[1] to remove uses of the register storage class specifier was recently pushed to jdk/jdk (jdk12), and a relatively small and so far mostly untested webrev[2] is available which has the changes needed to enable C++14. It was noted that if new C++ functionality is used in shared code all platforms are affected which in turn requires all relevant toolchains to support the new functionality. For example, the xlc compiler (AIX) does not yet support C++14 (unless this has changed recently?) so maintainers of that port would need to figure out how to move forward. Finally, Mikael also noted that there is other native code in the JDK (apart from hotspot) for which an upgrade should also be considered. Cheers, Mikael [1] http://hg.openjdk.java.net/jdk/jdk/rev/96ea37459ca7 [2] http://cr.openjdk.java.net/~mikael/webrevs/c++14/webrev.08/open/webrev/
On Fri, Aug 3, 2018 at 3:14 PM, Mikael Vidstedt <mikael.vidstedt@oracle.com> wrote:
Martin Buchholz suggested the topic and Mikael signed up to lead the session. Martin gave an introduction. He had observed some issues recently (“impossible null pointer exceptions”) which after investigation turned out to be caused by a toolchain upgrade and in turn revealed the fact that some code in hotspot requires atomicity but does not make this requirement very explicit and in the end assumes that the C++ compiler will produce suitable code to guarantee atomicity. Martin also observed that (on linux) hotspot is compiled targeting the c++98 standard, which is old enough to not even mention the concept of threads. Mikael also added that for extra fun the story is different on different platforms and toolchains.
I was surprised to hear that IBM AIX xlc compilers might not support C++11 - it's not the IBM way. But from reading the tea leaves at https://www-01.ibm.com/support/docview.wss?uid=swg27007322&aid=1 I concluded that IBM is a Linux company now! Even for IBM, AIX is a niche legacy platform and they just couldn't keep up with the evolution of C++ (who can blame them?). Meanwhile gcc is available for AIX and can/should be used to build openjdk. Has anyone tried? Here's one example of code that actually did go wrong with Google's latest internal toolchain, because the copy was not in fact word-atomic. Thanks to whoever added the comment long ago. static inline void copy_table(address* from, address* to, int size) { // Copy non-overlapping tables. The copy has to occur word wise for MT safety. while (size-- > 0) *to++ = *from++; } Recommendation: target C++11 for jdk12; use gcc to build openjdk on AIX.
On 08/05/2018 04:30 PM, Martin Buchholz wrote:
Here's one example of code that actually did go wrong with Google's latest internal toolchain, because the copy was not in fact word-atomic. Thanks to whoever added the comment long ago.
static inline void copy_table(address* from, address* to, int size) { // Copy non-overlapping tables. The copy has to occur word wise for MT safety. while (size-- > 0) *to++ = *from++; }
Recommendation: target C++11 for jdk12; I don't think that helps. There's no legal way AFAICS to force an atomic access to non-atomic types in C++11.
-- Andrew Haley Java Platform Lead Engineer Red Hat UK Ltd. <https://www.redhat.com> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
On 6/08/2018 6:16 PM, Andrew Haley wrote:
On 08/05/2018 04:30 PM, Martin Buchholz wrote:
Here's one example of code that actually did go wrong with Google's latest internal toolchain, because the copy was not in fact word-atomic. Thanks to whoever added the comment long ago.
static inline void copy_table(address* from, address* to, int size) { // Copy non-overlapping tables. The copy has to occur word wise for MT safety. while (size-- > 0) *to++ = *from++; }
Recommendation: target C++11 for jdk12; I don't think that helps. There's no legal way AFAICS to force an atomic access to non-atomic types in C++11.
I would agree. We implicitly rely on compilers doing the obvious/natural thing as long as the variables are suitable aligned. We're outside the language here with regards to "atomic access".** David
On Mon, Aug 6, 2018 at 1:16 AM, Andrew Haley <aph@redhat.com> wrote:
On 08/05/2018 04:30 PM, Martin Buchholz wrote:
Here's one example of code that actually did go wrong with Google's latest internal toolchain, because the copy was not in fact word-atomic. Thanks to whoever added the comment long ago.
static inline void copy_table(address* from, address* to, int size) { // Copy non-overlapping tables. The copy has to occur word wise for MT safety. while (size-- > 0) *to++ = *from++; }
Recommendation: target C++11 for jdk12; I don't think that helps. There's no legal way AFAICS to force an atomic access to non-atomic types in C++11.
Ohh... perhaps that's the idea behind atomic_ref https://en.cppreference.com/w/cpp/atomic/atomic_ref we only have to wait one more decade for that to become available. I don't know what is actually being copied here, but can't the underlying type be atomic<address*> ?
On Aug 6, 2018, at 9:54 PM, Martin Buchholz <martinrb@google.com> wrote:
I don't know what is actually being copied here, but can't the underlying type be atomic<address*> ?
Yes, if we are allowed to cast some random sequence of metadata words to atomic<address*>[]. If that's the magic incantation to get to the hardware's atomicity primitives, OK. I suspect a more direct technique may be needed, such as assembly code stubs. — John
On 08/07/2018 06:48 AM, John Rose wrote:
On Aug 6, 2018, at 9:54 PM, Martin Buchholz <martinrb@google.com> wrote:
I don't know what is actually being copied here, but can't the underlying type be atomic<address*> ?
Yes, if we are allowed to cast some random sequence of metadata words to atomic<address*>[].
We're not. Well, we sort-of are because we use -fno-strict-aliasing, but that's not standard C++11. Do we care? :-) GCC builtins do what we need when we're using GCC, but then we don't need C++11. -- Andrew Haley Java Platform Lead Engineer Red Hat UK Ltd. <https://www.redhat.com> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
Hi Andrew, This question is very important and deserves highlighting. I have been thinking for some time that there ought to be a document describing ”allowed C++ sins in HotSpot”. And I would put aliasing rules at the top of said document. Today we rely on compilers being tamed not to be tempted to exploit aliasing rules (e.g. -fno-strict-aliasing). The reliance on this in our code base goes so deep that we will arguably never be able to stop relying on it. And why would we want to, more than to find more interesting ways of tormenting ourselves? This is something we should embrace. By embracing this and putting it in a document that this is allowed, we would have the following benefits: 1) All reoccuring discussions whether we should or should not care about aliasing would come to quick ends, and decisions would not have to be taken (inconsistently) over and over again on a case by case basis. 2) Porters would know what requirements HotSpot has on compiler taming to safely run HotSpot. If they can not tame the compiler to ignore aliasing rules, then they can not use HotSpot. 3) By embracing aliasing violations as an allowed C++ sin, time will be saved for everyone involved, not having to invent complicated solutions circumventing it. Spending time honoring these rules seems like a waste of time and resources unless we fully commit to removing all such behaviour and hence can flip the compiler switches and hence remove our reliance on this. And we will never be able to do that. Nor should we if we could. 4) It seems like the problem being discussed in this thread before I hijacked it would have simple solutions. So basically, my answer to your question is: no we do not and should not care. And that message ought to be documented somewhere to remove all uncertainty and inconsistency around that reoccuring question. Thanks, /Erik
On 7 Aug 2018, at 16:00, Andrew Haley <aph@redhat.com> wrote:
On 08/07/2018 06:48 AM, John Rose wrote:
On Aug 6, 2018, at 9:54 PM, Martin Buchholz <martinrb@google.com> wrote:
I don't know what is actually being copied here, but can't the underlying type be atomic<address*> ?
Yes, if we are allowed to cast some random sequence of metadata words to atomic<address*>[].
We're not. Well, we sort-of are because we use -fno-strict-aliasing, but that's not standard C++11. Do we care? :-)
GCC builtins do what we need when we're using GCC, but then we don't need C++11.
-- Andrew Haley Java Platform Lead Engineer Red Hat UK Ltd. <https://www.redhat.com> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
On 08/08/2018 07:51 AM, Erik Osterlund wrote:
So basically, my answer to your question is: no we do not and should not care. And that message ought to be documented somewhere to remove all uncertainty and inconsistency around that reoccuring question.
That sounds sensible. I guess that if we use -fno-strict-aliasing then we can cast *T to *atomic<T>. I can ask on gcc@ to be sure. Having said that, I wouldn't. gcc builtins do what we need anyway, at least for platform-specific code. -- Andrew Haley Java Platform Lead Engineer Red Hat UK Ltd. <https://www.redhat.com> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
On Wed, Aug 8, 2018 at 7:02 AM, Andrew Haley <aph@redhat.com> wrote:
On 08/08/2018 07:51 AM, Erik Osterlund wrote:
So basically, my answer to your question is: no we do not and should not care. And that message ought to be documented somewhere to remove all uncertainty and inconsistency around that reoccuring question.
That sounds sensible. I guess that if we use -fno-strict-aliasing then we can cast *T to *atomic<T>. I can ask on gcc@ to be sure.
A difficulty might arise if the representation of atomic<T> is different from T, as might happen if the arch has no atomic instructions for a type of that size and so a lock must be allocated somewhere. I don't know how gcc's atomic builtins deal with that problem.
On Aug 8, 2018, at 1:05 PM, Martin Buchholz <martinrb@google.com> wrote:
That sounds sensible. I guess that if we use -fno-strict-aliasing then we can cast *T to *atomic<T>. I can ask on gcc@ to be sure.
A difficulty might arise if the representation of atomic<T> is different from T, as might happen if the arch has no atomic instructions for a type of that size and so a lock must be allocated somewhere. I don't know how gcc's atomic builtins deal with that problem.
A CPU/memory architecture which requires STM to for atomic storage of machine words would be a prime example of such a platform. It's also a prime example of a platform which HotSpot would not be portable to without deep refactoring along the lines of the access API but for all data. I think the two properties would be correlated, in practice. HotSpot makes pervasive assumptions that machine word data is routinely atomic (non-tearable, with valid race-winners in all cases), and it will be hard to break it of those assumptions in all cases. We've started to do that with things like the atomic, copy, and access APIs, but there's lots more to do, if we need to go down that road. — John
I agree getting to a hotspot without C++ undefined behavior is very hard. Organizations like Google like to have control over all their software, including the toolchain, and want both high performance and standards compliance, enforced via tools like ubsan https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html. Full employment for engineers like me who try to bridge the 2 worlds. I'd like to see hotspot stop using the -fno-strict-aliasing curtch, replacing it with union and/or may_alias, but this is a serious investment. Here are my type-punning notes: #---------------------------------------------------------------- # -fstrict-aliasing, union, memcpy, C99, C++. #---------------------------------------------------------------- https://blog.regehr.org/archives/959 http://dbp-consulting.com/tutorials/StrictAliasing.html C99 allows type punning via members of a union, and all known C++ compilers allow it, but strictly speaking not permitted by the C++ standard. 85. If the member used to access the contents of a union object is not the same as the member last used to store a value in the object, the appropriate part of the object representation of the value is reinterpreted as an object representation in the new type as described in 6.2.6 (a process sometimes called type punning). This might be a trap representation. Doing type punning via memcpy really goes against low-level programmer instinct - we really have to trust the compiler to optimize away the memcpy library call! gcc (what about clang?) has the may_alias attribute https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html
On 08/08/2018 08:51 PM, Martin Buchholz wrote:
I'd like to see hotspot stop using the -fno-strict-aliasing curtch, replacing it with union and/or may_alias, but this is a serious investment.
It's a serious investment, and we'd risk breaking stuff. At least the entire heap would have to be an array with elements which are a union of all of the possible types, and there'd be a byte array overlaid on top of that. It would not be pretty. -- Andrew Haley Java Platform Lead Engineer Red Hat UK Ltd. <https://www.redhat.com> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
OK, it looks like -fno-strict-aliasing is here to stay. Casting freely between pointers to different types is pervasive in the source code, and there's insufficient discipline in the culture to try to fix it. And I'm not volunteering. static void pd_arrayof_conjoint_jlongs(const HeapWord* from, HeapWord* to, size_t count) { #ifdef AMD64 _Copy_arrayof_conjoint_jlongs(from, to, count); #else pd_conjoint_jlongs_atomic((const jlong*)from, (jlong*)to, count); #endif // AMD64 } static void pd_arrayof_conjoint_oops(const HeapWord* from, HeapWord* to, size_t count) { #ifdef AMD64 assert(BytesPerLong == BytesPerOop, "jlongs and oops must be the same size"); _Copy_arrayof_conjoint_jlongs(from, to, count); #else pd_conjoint_oops_atomic((const oop*)from, (oop*)to, count); #endif // AMD64 } On Fri, Aug 10, 2018 at 5:59 AM, Andrew Haley <aph@redhat.com> wrote:
On 08/08/2018 08:51 PM, Martin Buchholz wrote:
I'd like to see hotspot stop using the -fno-strict-aliasing curtch, replacing it with union and/or may_alias, but this is a serious investment.
It's a serious investment, and we'd risk breaking stuff. At least the entire heap would have to be an array with elements which are a union of all of the possible types, and there'd be a byte array overlaid on top of that. It would not be pretty.
On Aug 5, 2018, at 8:30 AM, Martin Buchholz <martinrb@google.com> wrote:
Thanks to whoever added the comment long ago.
FTR I think it was Steffen Grarup. We were just learning about MT safety at the time. The copy conjoint/disjoint APIs were not yet in existence. I think they came around 2003, and Paul Hohensee's name is all over the SCCS history there. s 00008/00002/00762 d D 1.147 99/02/17 10:14:36 steffen 235 233 … I 235 static inline void copy_table(address* from, address* to, int size) { // Copy non-overlapping tables. The copy has to occur word wise for MT safety. while (size-- > 0) *to++ = *from++; } Today, that loop should be recoded to use copy, and copy in turn needs to do whatever magic is required to force word-atomic access on non-atomic data. — John
On Mon, Aug 6, 2018 at 11:12 AM, John Rose <john.r.rose@oracle.com> wrote:
On Aug 5, 2018, at 8:30 AM, Martin Buchholz <martinrb@google.com> wrote:
Thanks to whoever added the comment long ago.
FTR I think it was Steffen Grarup. We were just learning about MT safety at the time. The copy conjoint/disjoint APIs were not yet in existence. I think they came around 2003, and Paul Hohensee's name is all over the SCCS history there.
s 00008/00002/00762 d D 1.147 99/02/17 10:14:36 steffen 235 233 … I 235 static inline void copy_table(address* from, address* to, int size) { // Copy non-overlapping tables. The copy has to occur word wise for MT safety. while (size-- > 0) *to++ = *from++; }
Today, that loop should be recoded to use copy, and copy in turn needs to do whatever magic is required to force word-atomic access on non-atomic data.
That loop copies address*, while pd_disjoint_words_atomic copies HeapWord, so these are not compatible out of the box. We could have atomic relaxed copies like below. Using compiler builtins also avoids the problem of the underlying type not being declared atomic<>, and is ISA-independent. OTOH maybe we always want that loop compiled to REP MOVSQ on x64. template <typename T> static ALWAYSINLINE void copy_atomic_relaxed(const T* from, T* to) { T val; __atomic_load(from, &val, __ATOMIC_RELAXED); __atomic_store(to, &val, __ATOMIC_RELAXED); } static void pd_disjoint_words_atomic(const HeapWord* from, HeapWord* to, size_t count) { #ifdef AMD64 switch (count) { case 8: copy_atomic_relaxed(from + 7, to + 7); case 7: copy_atomic_relaxed(from + 6, to + 6); case 6: copy_atomic_relaxed(from + 5, to + 5); case 5: copy_atomic_relaxed(from + 4, to + 4); case 4: copy_atomic_relaxed(from + 3, to + 3); case 3: copy_atomic_relaxed(from + 2, to + 2); case 2: copy_atomic_relaxed(from + 1, to + 1); case 1: copy_atomic_relaxed(from + 0, to + 0); case 0: break; default: while (count-- > 0) { copy_atomic_relaxed(from++, to++); } break; }
In utilities/copy.[ch]pp there’s Copy::conjoint_copy and its close friends which does support different element sizes, and which promises to not tear the words/elements (if the underlying implementation doesn’t do the right thing it needs to be fixed). It doesn’t currently allow for configuring/customizing memory ordering requirements though, and If “extreme” performance is required there may well be some additional specialization needed as well. Cheers, Mikael
On Aug 6, 2018, at 11:26 PM, Martin Buchholz <martinrb@google.com> wrote:
On Mon, Aug 6, 2018 at 11:12 AM, John Rose <john.r.rose@oracle.com <mailto:john.r.rose@oracle.com>> wrote: On Aug 5, 2018, at 8:30 AM, Martin Buchholz <martinrb@google.com <mailto:martinrb@google.com>> wrote:
Thanks to whoever added the comment long ago.
FTR I think it was Steffen Grarup. We were just learning about MT safety at the time. The copy conjoint/disjoint APIs were not yet in existence. I think they came around 2003, and Paul Hohensee's name is all over the SCCS history there.
s 00008/00002/00762 d D 1.147 99/02/17 10:14:36 steffen 235 233 … I 235 static inline void copy_table(address* from, address* to, int size) { // Copy non-overlapping tables. The copy has to occur word wise for MT safety. while (size-- > 0) *to++ = *from++; }
Today, that loop should be recoded to use copy, and copy in turn needs to do whatever magic is required to force word-atomic access on non-atomic data.
That loop copies address*, while pd_disjoint_words_atomic copies HeapWord, so these are not compatible out of the box.
We could have atomic relaxed copies like below. Using compiler builtins also avoids the problem of the underlying type not being declared atomic<>, and is ISA-independent. OTOH maybe we always want that loop compiled to REP MOVSQ on x64.
template <typename T> static ALWAYSINLINE void copy_atomic_relaxed(const T* from, T* to) { T val; __atomic_load(from, &val, __ATOMIC_RELAXED); __atomic_store(to, &val, __ATOMIC_RELAXED); }
static void pd_disjoint_words_atomic(const HeapWord* from, HeapWord* to, size_t count) { #ifdef AMD64 switch (count) { case 8: copy_atomic_relaxed(from + 7, to + 7); case 7: copy_atomic_relaxed(from + 6, to + 6); case 6: copy_atomic_relaxed(from + 5, to + 5); case 5: copy_atomic_relaxed(from + 4, to + 4); case 4: copy_atomic_relaxed(from + 3, to + 3); case 3: copy_atomic_relaxed(from + 2, to + 2); case 2: copy_atomic_relaxed(from + 1, to + 1); case 1: copy_atomic_relaxed(from + 0, to + 0); case 0: break; default: while (count-- > 0) { copy_atomic_relaxed(from++, to++); } break; }
On Mon, Aug 6, 2018 at 11:12 AM, John Rose <john.r.rose@oracle.com> wrote:
On Aug 5, 2018, at 8:30 AM, Martin Buchholz <martinrb@google.com> wrote:
static inline void copy_table(address* from, address* to, int size) { // Copy non-overlapping tables. The copy has to occur word wise for MT safety. while (size-- > 0) *to++ = *from++; }
Today, that loop should be recoded to use copy, and copy in turn needs to do whatever magic is required to force word-atomic access on non-atomic data.
I now see the many variants of copy in share/utilities/copy.hpp but there is none that makes copies of the type "address". Maybe you could make an atomic copy template that takes any type T with sizeof(T) <= 8 ? --- At the type system level, HeapWord is a struct, so C++ will not be happy with our other traditional trick of reading a pointer to a volatile HeapWord to "force" atomicity.
participants (6)
-
Andrew Haley
-
David Holmes
-
Erik Osterlund
-
John Rose
-
Martin Buchholz
-
Mikael Vidstedt