RFR [jdk11]: 8216541: CompiledICHolders of VM locked unloaded nmethods are released too late

[Doerr, Martin]

Hi Matthias,

it's very cumbersome to figure out what you've done without any description.
I had trouble matching the changes in nmethod.cpp and sweeper.cpp.


nmethod.cpp:
clear_ic_callsites() introduced once in your version, twice in jdk12 version (one time with CompiledICLocker).

I guess it is not in make_not_entrant_or_zombie before post_compiled_method_unload() because JDK-8215491 is not in 11u.


jdk12 code looks like this:
{
    // Clear ICStubs and release any CompiledICHolders.
    CompiledICLocker ml(this);
    clear_ic_callsites();
  }
CompiledICLocker was introduced in JDK-8212681, which is not in 11u. I believe it's correct to omit the lock in 11u, because we're at safepoint and there's not concurrent GC thread outside of a safepoint at this place.


sweeper.cpp:
jdk12 version contains:
-    if (cm->is_alive() && !cm->is_unloading()) {
+    if (cm->is_alive()) {
This is already implemented this way in 11u because JDK-8213755 was not backported. So this part of your change seems to be fine.


So these changes look reasonable. The rest looks good, too.
Maybe you can double-check what I've written.

Best regards,
Martin


> -----Original Message-----
> From: Baesken, Matthias <matthias.baesken at sap.com>
> Sent: Freitag, 22. Mai 2020 13:28
> To: Doerr, Martin <martin.doerr at sap.com>; jdk-updates-
> dev at openjdk.java.net
> Subject: RE: RFR [jdk11]: 8216541: CompiledICHolders of VM locked unloaded
> nmethods are released too late
> 
> Hi Martin , there is no CompiledICLocker  in jdk11 .
> 
> Regarding "cm->is_alive()"  , this is at the same place were it was before :
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8216541_jdk11_0/src/hots
> pot/share/runtime/sweeper.cpp.frames.html
> 
> I think there was no need to change anything at that place .
> 
> Best regards , Matthias
> 
> 
> -----Original Message-----
> From: Doerr, Martin <martin.doerr at sap.com>
> Sent: Freitag, 22. Mai 2020 12:58
> To: Baesken, Matthias <matthias.baesken at sap.com>; jdk-updates-
> dev at openjdk.java.net
> Subject: RE: RFR [jdk11]: 8216541: CompiledICHolders of VM locked unloaded
> nmethods are released too late
> 
> Hi Matthias,
> 
> thanks for backporting it. Makes sense.
> 
> I'd be really helpful to get a few remarks about what you have resolved
> manually.
> 
> - I can see that the original change uses NoSafepointVerifier _nsv, but you
> have skipped it because JDK-8214338 is not in 11u. Looks ok.
> 
> - You had to introduce "CompiledICLocker ml(this);" and integrate "cm-
> >is_alive()" at a different place?
> Please describe your changes shortly.
> 
> Thanks and best regards,
> Martin
> 
> 
> > -----Original Message-----
> > From: jdk-updates-dev <jdk-updates-dev-bounces at openjdk.java.net> On
> > Behalf Of Baesken, Matthias
> > Sent: Freitag, 22. Mai 2020 11:26
> > To: jdk-updates-dev at openjdk.java.net
> > Subject: RFR [jdk11]: 8216541: CompiledICHolders of VM locked
> > unloaded nmethods are released too late
> >
> > Hello , please review  the jdk11  backport  of  8216541: CompiledICHolders
> of
> > VM locked unloaded nmethods are released too late  .
> > I had to do a few small adjustments because  the patch does not apply
> > cleanly .
> >
> >
> > Bug/webrev :
> >
> > https://bugs.openjdk.java.net/browse/JDK-8216541
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8216541_jdk11_0/
> >
> >
> > change from jdk12 was :
> >
> > http://hg.openjdk.java.net/jdk/jdk12/rev/0331b08811ad
> >
> >
> > Thanks, Matthias