RFR: 8292077: G1 nmethod entry barriers don't keep oops alive [v3]

[Thomas Schatzl]

On Wed, 10 Aug 2022 14:54:17 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:

>> The intention is that when G1 uses nmethod entry barriers (currently with --enable-preview to support Loom, but soon also with https://bugs.openjdk.org/browse/JDK-8290025), it keeps nmethod oops alive the first time an nmethod becomes on-stack during concurrent marking. The current code fails to do that, even though it intended to. That should be fixed.
>> 
>> There was code from loom to keep the oops alive by performing a phantom load. However, there is a quirk with the access API; if you don't assign the result of the load to something, the load won't be performed, and hence the oop won't be kept alive. I changed the code to use CollectedHeap::keep_alive explicitly instead of relying on side effects of a dummy load.
>> I also found that we assume G1 concurrent marking isn't aborted more than once. But that can happen. I made the assumption more conservative - surely if there are INT_MAX - 1 interrupted concurrent GCs in a row... something is very very wrong.
>
> Erik Österlund has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Stefan comments v1

Still good.

src/hotspot/share/gc/shared/barrierSetNMethod.cpp line 66:

> 64:   class OopKeepAliveClosure : public OopClosure {
> 65:   public:
> 66:     OopKeepAliveClosure() {}

The constructor could be removed as well now. Still looks good.

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

Marked as reviewed by tschatzl (Reviewer).

PR: https://git.openjdk.org/jdk/pull/9817