hs25 review request: 8008511 JSR 292: MemberName vmtarget refs to methods must be updated at class redefinition
coleen.phillimore at oracle.com
Mon Apr 1 15:51:12 PDT 2013
On 04/01/2013 06:02 PM, serguei.spitsyn at oracle.com wrote:
> 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 Christian first.
Definitely would like to hear from John and Christian. We did discuss
it but I wasn't totally convinced this field is more important than the
others that were going to be moved out of Klass* to make it smaller.
Some statistics for some sample applications like eclipse would be great.
>> 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
>>> 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
>>> 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