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

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Fri Oct 19 19:13:49 UTC 2018

Hi Dean,

Thank you for looking at it and the question! Karen told that it is was a mistake back in 2005 to add non-final private methods into vtable. I don't know anything about compatibility here and related tests. Keren is traveling now and will be able to clarify it next week only.

Thanks, Serguei

On 10/19/18 12:07 PM, dean.long at oracle.com wrote:

It sounds like the vtable slot was there for backwards compatibility, but I don't understand how a vtable slot for a private method ensured compatibility (how does it affect behavior?), so I don't understand how we continue to ensure backwards compatibility without it.  Also, which tests check for the desired backwards compatibility?

dl On 10/19/18 9:43 AM, serguei.spitsyn at oracle.com wrote: Hi Karen and Lois,

I have just one official review from David. May I count your internal reviews as official? Do we need the Compiler team to review this as well? Thanks, Serguei On 10/16/18 18:29, David Holmes wrote: 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::updateinheritedvtable() and klassVtable::needsnewvtableentry(), 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

More information about the hotspot-runtime-dev mailing list