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

[David Holmes]

Bizarre!

Looks good - officially Reviewed again!

Thanks for fixing Serguei!

David

On 17/10/2018 4:34 AM, serguei.spitsyn at oracle.com wrote:
> Re-sending my RFR.
> Behive managed not to deliver it to the target mailing lists.
> David got it because he was on the cc-list.
> 
> 
> 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
>