On Tue, 24 Nov 2020 02:59:50 GMT, Kim Barrett <[email protected]> wrote:
>> Looks good. Just want to request that you also remove the following comment
>> in zReferenceProcessor.cpp, as it's no longer true.
>>
>> --- a/src/hotspot/share/gc/z/zReferenceProcessor.cpp
>> +++ b/src/hotspot/share/gc/z/zReferenceProcessor.cpp
>> @@ -184,12 +184,6 @@ bool ZReferenceProcessor::should_discover(oop
>> reference, ReferenceType type) con
>> }
>>
>> bool ZReferenceProcessor::should_drop(oop reference, ReferenceType type)
>> const {
>> - // This check is racing with a call to Reference.clear() from the
>> application.
>> - // If the application clears the reference after this check it will still
>> end
>> - // up on the pending list, and there's nothing we can do about that
>> without
>> - // changing the Reference.clear() API. This check is also racing with a
>> call
>> - // to Reference.enqueue() from the application, which is unproblematic,
>> since
>> - // the application wants the reference to be enqueued anyway.
>> const oop referent = reference_referent(reference);
>> if (referent == NULL) {
>> // Reference has been cleared, by a call to Reference.enqueue()
>
>> Looks good. Just want to request that you also remove the following comment
>> in zReferenceProcessor.cpp, as it's no longer true.
>>
>> ```
>> --- a/src/hotspot/share/gc/z/zReferenceProcessor.cpp
>> +++ b/src/hotspot/share/gc/z/zReferenceProcessor.cpp
>> @@ -184,12 +184,6 @@ bool ZReferenceProcessor::should_discover(oop
>> reference, ReferenceType type) con
>> }
>>
>> bool ZReferenceProcessor::should_drop(oop reference, ReferenceType type)
>> const {
>> - // This check is racing with a call to Reference.clear() from the
>> application.
>> - // If the application clears the reference after this check it will still
>> end
>> - // up on the pending list, and there's nothing we can do about that
>> without
>> - // changing the Reference.clear() API. This check is also racing with a
>> call
>> - // to Reference.enqueue() from the application, which is unproblematic,
>> since
>> - // the application wants the reference to be enqueued anyway.
>> const oop referent = reference_referent(reference);
>> if (referent == NULL) {
>> // Reference has been cleared, by a call to Reference.enqueue()
>> ```
>
> Done.
I realized there was a theoretical problem with the new tests; they
should also be ensuring the Reference objects are in oldgen. That's
fixed in the latest push.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1376
