RFR 8062116: JVMTI GetClassMethods is Slow

[serguei.spitsyn at oracle.com]

On 11/5/14 3:40 PM, Coleen Phillimore wrote:
>
> On 11/5/14, 6:13 PM, Jeremy Manson wrote:
>>
>>
>> On Tue, Nov 4, 2014 at 8:56 PM, serguei.spitsyn at oracle.com 
>> <mailto:serguei.spitsyn at oracle.com> <serguei.spitsyn at oracle.com 
>> <mailto:serguei.spitsyn at oracle.com>> wrote:
>>
>>     The fix looks good in general.
>>
>>     src/share/vm/oops/method.cpp
>>
>>     1785   bool contains(Method** m) {
>>     1786     if (m == NULL) return false;
>>     1787     for (JNIMethodBlockNode* b = &_head; b != NULL; b = b->_next) {
>>     1788       if (b->_methods <= m && m < b->_methods + b->_number_of_methods) {
>>     *1789         ptrdiff_t idx = m - b->_methods;**
>>     **1790         if (b->_methods + idx == m) {**
>>     1791           return true;
>>     1792         }*
>>     1793       }
>>     1794     }
>>     1795     return false;  // not found
>>     1796   }
>>
>>
>>     Just noticed that the lines 1789-1792 can be replaced with one liner:
>>     *        return true;*
>>
>>
>> Ah, you have found our crappy workaround for wild pointers to 
>> non-aligned places in the middle of _methods.
>
> Can you explain this?  Why are there wild pointers?
>>
>>     It is because the condition *(b->_methods + idx == m)* is always
>>     true.     :)
>>
>>     Also, should we check the condition: **m != _free_method*** ?
>>     What about the following ?:
>>     *        return (****m != _free_method***);*
>>
>>
>> I don't mind adding this, if Coleen thinks those are the semantics 
>> this needs.  It wasn't there before, of course.
>>
>
> The semantics weren't there before and the way this is called has 
> already checked that *m != _free_method.  Would it be an improvement?  
> I don't think so.  It seems that contains() should just check that the 
> Method** is contained in the methodID table. To be more correct, 
> is_method_id should check that it's not a freed methodID but the 
> caller verifies this already.   So I don't think this should change.

Agreed.
Thank you for the explanation!


>
> BTW, I've run the test sets suggested by Serguei and they all passed.

Nice!


Thanks,
Serguei

>
> Coleen
>
>> Jeremy
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20141105/4e27a8a3/attachment.html>