RFR: 8212826: Make PtrQueue free list lock-free
sangheon.kim at oracle.com
sangheon.kim at oracle.com
Sat Jan 19 07:26:23 UTC 2019
Hi Kim,
On 1/15/19 2:54 PM, Kim Barrett wrote:
>> On Jan 15, 2019, at 9:13 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>>
>> Hi Kim,
>>
>> On Wed, 2019-01-09 at 16:48 -0500, Kim Barrett wrote:
>>> 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.
> Can’t use inc/dec, because the incremented values are used, and those functions
> don’t return anything. But it seems that I forgot that Erik and I managed to slip in
> a bit of argument canonicalization here, so that an exact type match isn’t required;
> just using ‘1u’ works.
>
>> - test_ptrQueueBufferAllocator.cpp copyright should be "2018, 2019,"
>> instead of just 2019.
> Oops. Fixed.
>
>> - 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.
> Done, with a bit more cleanup.
>
>> Looks good otherwise.
> Thanks.
>
> Note that I’ve filed a bug against Solaris Studio for the problem with
> using pointers to data members. I’ll file a JDK RFE to change
> LockFreeStack to the preferred form once the Solaris Studio bug is
> fixed and we’re no longer supporting versions with the bug.
>
> New webrevs:
> full: http://cr.openjdk.java.net/~kbarrett/8212826/open.01/
> incr: http://cr.openjdk.java.net/~kbarrett/8212826/open.01.inc/
01 looks good.
FYI, 4 files failed(ptrQueue.hpp, logTag.hpp padded.hpp mutexLocker.hpp)
to apply because the copyright year is already updated.
Actual codes were applied cleanly. I don't need a webrev for this change.
Thanks,
Sangheon
>
>
More information about the hotspot-gc-dev
mailing list