RFR(S): 8024368: private methods are allocated vtable slots

[Lois Foltan]

On 10/24/2018 1:33 PM, serguei.spitsyn at oracle.com wrote:

> Hi Karen,
>
> It is great you had a chance to review it officially - thanks a lot!
> Probably, I have to add Lois to the list of reviewers before pushing it.

Please do!  I did review and looks good.
Lois

>
> Thanks,
> Serguei
>
>
> On 10/24/18 09:36, Karen Kinnear wrote:
>> Serguei,
>>
>> Fix looks good. Officially reviewed.
>>
>> Many thanks for the tests and the -Xlog output. Many thanks for 
>> fixing this.
>>
>>
>> thanks,
>> Karen
>>
>>> On Oct 16, 2018, at 1:03 AM, serguei.spitsyn at oracle.com 
>>> <mailto:serguei.spitsyn at oracle.com> wrote:
>>>
>>> Please, review a fix for:
>>> https://bugs.openjdk.java.net/browse/JDK-8024368
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8024368-priv-vtable.2/
>>>
>>>
>>> Summary:
>>>
>>>   Private method should not allocate vtable indeces.
>>>   The fix tweaks several conditions in the functions:
>>>   klassVtable::update_inherited_vtable() and 
>>> klassVtable::needs_new_vtable_entry(),
>>>   adds a couple of asserts and makes several comments up-to-date.
>>>
>>>   The fix was preliminary reviewed by Karen, David H. and Lois.
>>>
>>> Testing:
>>>
>>>   The fix was verified with 
>>> runtime/{appcds+Nestmates+RedefineTests+SelectionResolution}
>>>   and vmTestbase/vm/runtime/defmeth tests.
>>>   Also, I ran a mach5 job with the flags: 
>>> tier1,tier2,hs-tier3,hs-tier4,hs-tier5.
>>>
>>>   By suggestion from David, it was also tested with the -Xlog output 
>>> for vtables before and after the fix.
>>>   I've done this comparison (awk was used to get rid of the first 
>>> column with the time stamps).
>>>   It shows that private methods were included into vtable before the 
>>> fix but not included after.
>>>   Please, see one fragment as an example:
>>>
>>>    166c166
>>>    <  copy vtable from java.lang.Object to java.lang.StackFrameInfo 
>>> size 17
>>>    ---
>>>    >  copy vtable from java.lang.Object to java.lang.StackFrameInfo 
>>> size 16
>>>    180d179
>>>    <  adding java.lang.StackFrameInfo.ensureRetainClassRefEnabled()V 
>>> at index 16, flags: private
>>>    182c181
>>>    <  copy vtable from java.lang.StackFrameInfo to 
>>> java.lang.LiveStackFrameInfo size 17
>>>    ---
>>>    >  copy vtable from java.lang.StackFrameInfo to 
>>> java.lang.LiveStackFrameInfo size 16
>>>    188,201c187,191
>>>    <  copy vtable from java.lang.Object to 
>>> java.lang.StackStreamFactory$AbstractStackWalker size 18
>>>    <  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.close()V at index 
>>> 5, flags: private
>>>    <  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.toStackWalkMode(Ljava/lang/StackWalker;I)I 
>>> at index 6, flags: private
>>>    <  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.consumeFrames()Ljava/lang/Object; 
>>> at index 7, flags: protected abstract
>>>    <  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.initFrameBuffer()V 
>>> at index 8, flags: protected abstract
>>>    <  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.batchSize(I)I at 
>>> index 9, flags: protected abstract
>>>    <  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.getNextBatchSize()I 
>>> at index 10, flags: protected
>>>    <  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.skipReflectionFrames()Z 
>>> at index 11, flags: private
>>>    <  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.doStackWalk(JIIII)Ljava/lang/Object; 
>>> at index 12, flags: private
>>>    <  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.getNextBatch()I at 
>>> index 13, flags: private
>>>    <  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.beginStackWalk()Ljava/lang/Object; 
>>> at index 14, flags: private
>>>    <  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.fetchStackFrames(I)I 
>>> at index 15, flags: private
>>>    <  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.fetchStackFrames(JJII[Ljava/lang/Object;)I 
>>> at index 16, flags: private native
>>>    <  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.callStackWalk(JIII[Ljava/lang/Object;)Ljava/lang/Object; 
>>> at index 17, flags: private native
>>>    ---
>>>    >  copy vtable from java.lang.Object to 
>>> java.lang.StackStreamFactory$AbstractStackWalker size 9
>>>    >  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.consumeFrames()Ljava/lang/Object; 
>>> at index 5, flags: protected abstract
>>>    >  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.initFrameBuffer()V 
>>> at index 6, flags: protected abstract
>>>    >  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.batchSize(I)I at 
>>> index 7, flags: protected abstract
>>>    >  adding 
>>> java.lang.StackStreamFactory$AbstractStackWalker.getNextBatchSize()I 
>>> at index 8, flags: protected
>>>
>>>
>>> Thanks,
>>> Serguei
>>>
>>
>