hs25 review request: 8008511 JSR 292: MemberName vmtarget refs to methods must be updated at class redefinition
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Mon Apr 1 15:02:35 PDT 2013
On 4/1/13 1:09 PM, Coleen Phillimore wrote:
> 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?
Thank you for the catch!
> 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.
My understanding was that John is considering to use the
MemberNameTable in the future for the compilation purposes.
However, I can temporarily place it under #if INCLUDE_JVMTI
until the compiler team decides what to do with it.
> 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.
I thought we've reached a consensus and decided to add MNT pointer
per class as it is performance sensitive for indy calls.
Anyway, before changing it to a hashtable I'd want to hear from John and
> 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
I very appreciate you found a time to review it!
There are not many people having an expertize in this area.
> 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:
>> The CR is:
>> 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
>> It is released in the InstanceKlass::release_C_heap_structures().
>> A global week reference to member name oop is stored in the
>> 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.
More information about the hotspot-dev