RFR: 8263551: Provide shared lock-free FIFO queue implementation [v3]

Kim Barrett kbarrett at openjdk.java.net
Tue Mar 30 19:11:41 UTC 2021


On Tue, 30 Mar 2021 07:49:00 GMT, Man Cao <manc at openjdk.org> wrote:

>> Hi all,
>> 
>> Could anyone review this change that is mainly code motion? It creates a generalized lock-free queue implementation based on G1DirtyCardQueueSet::Queue, which will be used by JDK-8236485 in the future.
>> 
>> The shared LockFreeQueue is similar to the existing LockFreeStack. The notable difference is that the LockFreeQueue has an additional template parameter for whether to use GlobalCounter::CriticalSection to avoid ABA problem.
>> 
>> -Man
>
> Man Cao has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Changed to try_pop() and eliminated conditional critical section.

src/hotspot/share/gc/g1/g1DirtyCardQueue.cpp line 155:

> 153:         return pop_result.second;
> 154:       case Status::operation_in_progress:
> 155:         // This could happen when a concurrent operation interferes with

I think most of this comment doesn't belong here.  What's here now is largely restating what should be the documentation for `operation_in_progress`.

src/hotspot/share/utilities/lockFreeQueue.hpp line 56:

> 54:   DEFINE_PAD_MINUS_SIZE(1, DEFAULT_CACHE_LINE_SIZE, sizeof(T*));
> 55:   T* volatile _tail;
> 56:   DEFINE_PAD_MINUS_SIZE(2, DEFAULT_CACHE_LINE_SIZE, sizeof(T*));

LockFreeQueue should probably document it's padding.  And maybe should not have the second (trailing) padding, but leave that to the specific usage.  The trailing padding in the original was consistent with the usage there.  Other uses might not need any trailing padding, or might need less.

src/hotspot/share/utilities/lockFreeQueue.hpp line 98:

> 96:   //   The operation succeeded. If pair.second is NULL, the queue is empty;
> 97:   //   otherwise caller can assume ownership of the object pointed by
> 98:   //   pair.second. Note that this case still subjects to ABA behavior;

s/case still subjects/case is still subject/

-------------

PR: https://git.openjdk.java.net/jdk/pull/2986



More information about the hotspot-gc-dev mailing list