RFR: 8061630: G1 iterates over JNIHandles two times

[Mikael Gerdin]

Hi Erik,

On 2014-10-21 16:47, Erik Helin wrote:
> Hi all,
>
> this patch removes a redundant call to JNIHandles::weak_oops_do. The
> call to JNIHandles::weak_oops_do can be removed because is it also
> called by ReferenceProcessor::process_discovered_references via
> G1CollectedHeap::process_discovered_references, see
> ReferenceProcessor::process_phaseJNI.
>
> Both calls to JNIHandles::weak_oops_do uses the same kind of closure for
> checking if an oop alive, G1STWIsAliveClosure. However,
> JNIHandles::weak_oops_do in g1CollectedHeap.cpp gets passed an
> G1KeepAliveClosure, whereas the call to JNIHandles::weak_oops_do in
> referenceProcessor.cpp gets passed an G1CopyingKeepAliveClosure.
>
> Looking at JNIHandleBlock::weak_oops_do, one can see that the
> `keep_alive` closure will only be called on the root if the `is_alive`
> closure returns true for the value of the handle. If the
> G1STWIsAliveClosure returns true, that means the value is either outside
> the collection set or the value has been forwarded. Since the root
> (JNIHandle) is outside the G1 heap, G1CopyingKeepAliveClosure will use a
> G1ParCopyClosure (see _copy_non_heap_obj_cl in
> G1CopyingKeepAliveClosure). Therefore, if we want to remove the
> G1KeepAliveClosure phase, we must ensure ourselves that G1ParCopyClosure
> and G1KeepAliveClosure do the same thing for a JNIHandle root with a
> value that is either outside the collection set or
> already forwarded.
>
> If the value is outside the collection set and not humongous, then both
> G1ParCopyClosure and G1STWIsAliveClosure do nothing. If the value is
> humongous, then both closures calls
> G1CollectedHeap::set_humongous_is_live. If the value is in the
> collection set, then G1STWIsAliveClosure assumes it has been forwarded
> and updates the root. G1ParCopyClosure will also update the root, since
> the value already has been forwarded.
>
> Due to the above, G1ParCopyClosure already does everything
> G1KeepAliveClosure does for JNIHandles, so there is no need to iterate
> over them again using G1KeepAliveClosure.
>
> Webrev:
> http://cr.openjdk.java.net/~ehelin/8061630/webrev.00/

The change looks good.
/Mikael

>
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8061630
>
> Testing:
> JPRT
>
> Thanks,
> Erik