Review Request (S) 8014288: perf regression in nashorn JDK-8008448.js test after 8008511 changes

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Thu May 23 17:37:19 PDT 2013


Hi David,

On 5/23/13 5:15 PM, David Holmes wrote:
> Hi Serguei,
>
> I have two queries:
>
> In MemberNameTable::add_member_name why do you not have 
> assert_locked_or_safepoint(MemberNameTable_lock) ?

Nice catch, thanks.
It has to be there but I accidentally removed it.
Will fix it.

>
> In MethodHandles::init_field_MemberName you deleted:
>
> 304 InstanceKlass::cast(field_holder())->add_member_name(mname);
>
> apart from this seeming to be unrelated to the rest of the fix, does 
> its removal make the previous 8 lines of comment unnecessary? (I 
> confess I don't understand the relationship between the comment and 
> the code anyway.)

It is not related to the removed line.
I think, it is for the return value.
The line was removed because we decided to skip caching the field
MemberName's for now as they are not used yet.

Thanks,
Serguei

>
> Thanks,
> David
>
> On 24/05/2013 9:19 AM, serguei.spitsyn at oracle.com wrote:
>> Please, review the fix for:
>>    bug: http://bugs.sun.com/view_bug.do?bug_id=8014288
>>    jbs: https://jbs.oracle.com/bugs/browse/JDK-8014288
>>
>> Open webrev:
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/8014288-JVMTI-JSR292.1/ 
>>
>>
>>
>> Summary:
>>    The fix of the 8008511 introduced a performance regression for the
>> Nashorn tests.
>>    The fix is to use method_idnum() for direct indexing into the
>> MemberNameTable
>>    that replaces a linear search caused the regression.
>>    No new regression test is needed as the Nashorn tests show the
>> performance is back to normal.
>>
>> Testing:
>>    The vm/mlvm and Nashorn tests
>>
>> Thanks,
>> Serguei



More information about the hotspot-dev mailing list