RFR: 8212826: Make PtrQueue free list lock-free

[Thomas Schatzl]

Hi Kim,

On Wed, 2019-01-09 at 16:48 -0500, Kim Barrett wrote:
> Please review this change to BufferNode::Allocator to eliminate the
> Mutex protecting the free-list, instead using a lock-free free-list.
> 
> This eliminates the need for the (access rank) free list lock for the
> SATB and DirtyCard buffer allocator.
> 
> Part of this change is the introduction of a LockFreeStack utility
> class template.  This class doesn't attempt to solve the ABA problem,
> instead leaving that to the client.  The Allocator uses GlobalCounter
> to solve the problem.  (Other existing potential uses cases have
> separate phases for push and pop, don't recycle objects, or have
> other solutions.)  Popping a buffer from the free list is done within
> a read critical section.  A buffer is added to the free list only
> after an associated write synchronization is performed.

That's really nice, just what I needed :) (The allocator and the stack
class).

> 
> Unfortunately, the API for LockFreeStack<> isn't quite what we would
> prefer.  We would like this class's signature to be
> 
>   template<typename T, T* volatile T::*next> class LockFreeStack;
> 
> but it is instead
> 
>   template<typename T, T* volatile* (*next_ptr)(T&)> class
> LockFreeStack;
> 
> This is due to a bug in Solaris Studio's handling of pointer to data
> member references within the class declaration (where the class is
> incomplete).  (Visual Studio 2013 (not before or after) had a similar
> bug.)
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8212826
> 
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8212826/open.00/
> 

- ptrQueue.cpp:121:

size_zero seems to be unused, and size_one can be avoided completely by
using Atomic::inc/dec instead of add/sub which I would prefer.

Even if you kept add/sub, hardcoding (size_t)1 would be less code than
the constant declaration as there are not many uses.

- test_ptrQueueBufferAllocator.cpp copyright should be "2018, 2019,"
instead of just 2019.

- the gtest adds a small API for testing and comparison should be part
of the test. It honestly seems to add nothing to the final test other
than adding additional code, particularly because the other variants
mentioned in the comment are not there. So I would prefer to flatten
the FreeListPtrQueueBufferAllocator/FreeListPtrQueueBufferCompletedList
classes.

Looks good otherwise.

Thanks,
  Thomas