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

[Thomas Schatzl]

On Wed, 10 Aug 2022 09:01:59 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.

Changes requested by tschatzl (Reviewer).

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

> 42: 
> 43: class OopKeepaliveClosure : public OopClosure {
> 44: private:

Suggestion:


This line can be dropped.

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

> 122:   // seriously wrong.
> 123:   ++_current_phase;
> 124:   if (_current_phase == INT_MAX) {

I would really prefer to deal with this in a separate issue. It does not have anything to do with keeping oops alive in the nmethod barrier.

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

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