hs25 review request: 8008511 JSR 292: MemberName vmtarget refs to methods must be updated at class redefinition

[Coleen Phillimore]

Hi Serguei,

Sorry for the delay in reviewing this.

In instanceKlass.cpp line 2730, can you make that mtClass since it's 
part of class metadata?

In methodHandles.cpp, I think the whole member name table mechanism 
could be protected by #if INCLUDE_JVMTI since it's not needed unless you 
have jvmti built in the vm.   And you don't need to call 
add_method_handle() or just have it do JVMTI_RETURN or something for the 
minimal vm.

Also, I think you should consider a global table rather than one per 
class.  I think we discussed this and may have decided that speed of 
adding these is important, but depending on how many are actually used 
per class, there's one pointer per class.

Maybe they could be organized as a hashtable instead like the method 
handle intrinsics?

The code itself looks good everywhere, except for the concern about 
footprint.

Thanks,
Coleen

Can you make this mtClass on line
On 03/26/2013 04:37 AM, serguei.spitsyn at oracle.com wrote:
>
> Please, review the following fix.
> The fix was preliminarily reviewed by Coleen, Christian
> and John but still a final and open jdk review is needed.
> So that, everyone is welcome to review the fix!
>
> This is open webrev:
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/8008511-JVMTI-JSR292.1/
>
> The CR is:
> 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 Rose) is to lazily create and keep 
> up-to-date a MemberNameTable
> which plays a role of MemberName cache assosiated with the InstanceKlass.
> Then, at the 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.
>
> The following No_Safepoint_Verifier has been disabled:
> *  src/share/vm/prims/methodHandles.cpp*:
>     799 int MethodHandles::find_MemberNames(Klass* k,
>     . . .
>     803 DEBUG_ONLY(No_Safepoint_Verifier nsv);
>     It is probably Ok, but, please, let me know if it is not.
>     The assert related to locking is fired if it is not disabled.
>
> Test coverage: vm.mlvm, nsk.jvmti, nsk.jdi tests on multiple platforms 
> (32 vs 64-bit too).
> The testing looks good so far.
>
>
> Thanks,
> Serguei
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20130401/a1cc9075/attachment.html