hs25 review request (2nd round): 8008511 JSR 292: MemberName vmtarget refs to methods must be updated at class redefinition

[David Holmes]

Hi Serguei,

Not a full review ...

In instanceKlass.cpp:

+   MemberNameTable* mnt = member_names();
+   if (mnt != NULL) {
+     delete mnt;
+   }

a) do we need to call set_member_names(null) ?
b) do we need to use the MemberNameTable_lock to guard this?

---

I'm still concerned about the removal of the No_Safepoint_verifier. Has 
whomever added it indicated that removing it is indeed okay?

Thanks,
David

On 13/04/2013 12:13 PM, serguei.spitsyn at oracle.com wrote:
>
> Please, review the following fix.
>
> This is the 2nd version that has a review fixes suggested by Coleen and
> John in the 1st review round.
>
> Now the mtClass is used in allocation of MemberNameTable since it's part
> of class metadata.
> All call sites in the methodHandles.cpp that recursively invoke the
> add_member_name()
> are made safepoint-safe by replacing oop's with Handle's.
> This allowed to get rid of the No_Safepoint_Verifier in the
> find_MemberNames.
> The MemberName's are added to MNT not only for methods but also for fields.
>
> Coleen,
> You also suggested to put all new non-jvmti code under #if INCLUDE_JVMTI
> condition.
> I've not done it because now the MemberName's associated with the fields
> are
> also registered in the MNT which is not needed for JVMTI purposes.
> Just have some doubt if it is a right thing to do.
> I'm Ok to add this modification, but could you, confirm that it is still
> important.
>
>
> Webrev:
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/8008511-JVMTI-JSR292.2/
>
>
> Bug:
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8008511
> https://jbs.oracle.com/bugs/browse/JDK-8008511
>
>
> The problem is that the old version of the bootstrap method is re-invoked
> after a popframe from the bootstrap method execution.
> It is because the MemberName keeps a stale reference to the old method
> version.
>
> The solution (suggested by John) is to lazily create and keep up-to-date
> a MemberNameTable
> which plays a role of MemberName cache assosiated with the InstanceKlass.
> Then, at class redefinition, this cache is used to replace the old method
> references in the MemberName's with the new method references.
>
> The MemberNameTable is based on the GrowableArray<jweek>.
> A C_HEAP array is allocated at the first call to
> InstanceKlass::add_member_name().
> It is released in the InstanceKlass::release_C_heap_structures().
>
> A global week reference to member name oop is stored in the
> MemberNameTable.
> It allowed to avoid having the oops_do() in the MemberNameTable.
> Also, the MemberNameTable won't hold member name oops in memory.
>
> The MemberNameTable_lock mutex is added to serialize MemberNameTable's
> updates.
>
> Test coverage: vm.mlvm, nsk.jvmti, nsk.jdi tests on multiple platforms
> (32 vs 64-bit too).
> The testing looks good.
>
>
> Thanks,
> Serguei