RFR (M) 8195099: Concurrent safe-memory-reclamation mechanism (original) (raw)

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Tue Apr 10 22:40:36 UTC 2018

On 4/10/18 11:53 AM, Kim Barrett wrote:

On Apr 10, 2018, at 10:42 AM, Robbin Ehn <robbin.ehn at oracle.com> wrote:

Full: http://cr.openjdk.java.net/~rehn/8195099/v2/webrev/ Inc : http://cr.openjdk.java.net/~rehn/8195099/v2/inc/webrev/ ------------------------------------------------------------------------------ GlobalCounter describes its implementation, but is not the least bit evocative of its purpose. I don't have a good name suggestion though.

I like the name because it's simple, unique in the vm, and there isn't a better name that comes to mind. GlobalCounter ?  I think we should keep this as is until we find a better name, which might take a while.

------------------------------------------------------------------------------ This is being put in utilities. Similarly we put SpinYield in utilities. But I'm not sure either of those are properly placed there. These are synchronization building blocks, and all other synchronization stuff is in runtime.

The intended use is for a concurrent hashtable in utilities that will be used by the StringTable (in classfile directory), so that's why utilities seems a good choice (esp given SpinYield there).  I think it would get confused with synchronizer.cpp (for Java level locks) if it were in runtime.

------------------------------------------------------------------------------ src/hotspot/share/utilities/globalCounter.hpp Consider just forward declaring Thread (at namespace scope). Consider just forward declaring CounterThreadCheck at class scope, with the definition in the .cpp file. With those changes, don't need to #include thread.hpp in this header. Although globalCounter.inline.hpp still needs to #include thread.hpp, so maybe this doesn't matter so much?

I had this comment too.

Coleen

------------------------------------------------------------------------------ src/hotspot/share/utilities/globalCounter.hpp

Although CriticalSection is fully defined in this header, I think it can't be instantiated without including globalCounter.inline.hpp. Consider just forward declaring in the GlobalCounter definition, with the definition in the .inline.hpp. ------------------------------------------------------------------------------ src/hotspot/share/utilities/globalCounter.inline.hpp Missing #include globalCounter.hpp. ------------------------------------------------------------------------------ src/hotspot/share/utilities/globalCounter.inline.hpp 36 OrderAccess::releasestorefence(thread->getrcucounter(), gblcnt + 1); "gblcnt + 1" => "gblcnt | COUNTERACTIVE". ------------------------------------------------------------------------------ src/hotspot/share/utilities/globalCounter.hpp 74 // Must be called after finished accessing the data. 75 // Do not provide fence, allows load/stores moving into the critical section. 76 static void criticalsectionend(Thread *thread); Both begin and end should have descriptions of their ordering behavior. I'm thinking something similar to the descriptions we have for Atomic operations. ------------------------------------------------------------------------------

More information about the hotspot-dev mailing list