On Oct 19, 2012, at 11:26 AM, Christian Thalinger <[email protected]> wrote:
> > On Oct 18, 2012, at 6:48 PM, Seán Coffey <[email protected]> wrote: > >> Christian, >> >> It is quite a big code change for 7u, one worthy of more testing before >> integration perhaps. Doesn't this code introduce new dependencies on the VM >> code ? How are changes progressing for that ? Does hsx24 need to be in >> jdk7u-dev first ? > > hsx24 already has the changes that are required for the new JSR 292 > implementation. In other words: the JDK changes are required otherwise > jdk7u doesn't work when hsx24 is integrated. > >> >> Have the proposed backports been reviewed for jdk7u ? Even though the fixes >> are more or less identical to jdk8, I think it's best to get peer reviews in >> the context of such code fitting into jdk7u. > > I didn't get reviews yet and I'm not sure anyone will (given the size of the > patch). > >> >> Why won't you backport the changes in separate changesets ? This has been >> common practice in 7u and it makes tracing of code changes easier to >> analyze. This change is a bulk change for jdk7u [1] - something I can't >> recall seeing for the jdk repo in the past. > > I tried. A changeset-by-changeset backport was non trivial and the deadline > for PIT testing came closer so we decided to do it as one changeset. > > In the end the code is the same anyway because we need exactly the same > classes (except the small diff I posted) in 7u as we have in 8. > > If we are willing to postpone to next week's PIT I can bring the original > changesets over. And if that means no reviews and quick approvals then it > might be worth it. Change of plans. I spent today to backport the individual changesets. There will be a bulk integration of these JDK changesets along with the HS24 changesets after PIT was successful. These are the backported changes: $ hg out --template "{desc|firstline}\n" comparing with http://hg.openjdk.java.net/jdk7u/jdk7u/jdk searching for changes 6983728: JSR 292 remove argument count limitations 7128512: Javadoc typo in java.lang.invoke.MethodHandle 7058651: JSR 292 unit tests need a refresh 7058630: JSR 292 method handle proxy violates contract for Object methods 7089131: test/java/lang/invoke/InvokeGenericTest.java does not compile 7117167: Misc warnings in java.lang.invoke and sun.invoke.* 7129034: VM crash with a field setter method with a filterArguments 7087658: MethodHandles.Lookup.findVirtual is confused by interface methods that are multiply inherited 7127687: MethodType leaks memory due to interning 7023639: JSR 292 method handle invocation needs a fast path for compiled code 7188911: nightly failures after JSR 292 lazy method handle update (round 2) 7190416: JSR 292: typo in InvokerBytecodeGenerator.getConstantPoolSize 7191102: nightly failures after JSR 292 lazy method handle update (round 3) 7194612: api/java_lang/invoke/MethodHandles/Lookup/index.html#ExceptionsTests[findVirtualNSME] fails w/ -esa 7194662: JSR 292: PermuteArgsTest times out in nightly test runs 8000989: smaller code changes to make future JSR 292 backports easier And here is a webrev of the whole thing (should be almost identical to the previous one): http://cr.openjdk.java.net/~twisti/8000999/ -- Chris > > -- Chris > >> >> I understand some more PIT testing is going to be run - Let's see how test >> results look before proceeding. >> >> regards, >> Sean. >> >> [1] http://openjdk.java.net/projects/jdk7u/bulkchanges.html >> >> On 18/10/2012 12:22, Christian Thalinger wrote: >>> On Oct 18, 2012, at 12:17 PM, Christian Thalinger >>> <[email protected]> wrote: >>> >>>> On Oct 18, 2012, at 12:00 PM, [email protected] wrote: >>>> >>>>> 2012/10/18 11:54 -0700, [email protected]: >>>>>> Webrev: http://cr.openjdk.java.net/~twisti/8000999/ >>>>>> 8000999: backport of JSR 292 to 7u >>>>>> >>>>>> This is an umbrella bug for these changes (which are backported in one >>>>>> changeset): >>>>>> >>>>>> ... >>>>> Um, isn't this an awfully big change for what's supposed to be a low-risk >>>>> update release? >>>> It's big, indeed. But actually it's a bug fix and it only touches JSR 292 >>>> related files and functionality. >>>> >>>> Since PR decided that HS24 will be used for 7u12 (which I applaud to) we >>>> need these changes in 7. Otherwise it doesn't work. >>> I forgot to add the diff between 7 and 8 after this change goes in. There >>> is also a review request on hotspot-dev. >>> >>> -- Chris >>> >>> The backport is just copying over the files from JDK 8. That's why the >>> webrev is so big and pretty useless. The real changes between 8 and 7 are >>> these: >>> >>> diff -Nur jdk8/src/share/classes/java/lang/invoke/MethodHandleStatics.java >>> jdk7u/src/share/classes/java/lang/invoke/MethodHandleStatics.java >>> --- jdk8/src/share/classes/java/lang/invoke/MethodHandleStatics.java >>> 2012-10-15 12:21:52.806052959 -0700 >>> +++ jdk7u/src/share/classes/java/lang/invoke/MethodHandleStatics.java >>> 2012-10-16 10:48:29.728257304 -0700 >>> @@ -94,10 +94,14 @@ >>> >>> // handy shared exception makers (they simplify the common case code) >>> /*non-public*/ static InternalError newInternalError(String message, >>> Throwable cause) { >>> - return new InternalError(message, cause); >>> + InternalError e = new InternalError(message); >>> + e.initCause(cause); >>> + return e; >>> } >>> /*non-public*/ static InternalError newInternalError(Throwable cause) { >>> - return new InternalError(cause); >>> + InternalError e = new InternalError(); >>> + e.initCause(cause); >>> + return e; >>> } >>> /*non-public*/ static RuntimeException newIllegalStateException(String >>> message) { >>> return new IllegalStateException(message); >>> diff -Nur jdk8/src/share/classes/sun/invoke/util/ValueConversions.java >>> jdk7u/src/share/classes/sun/invoke/util/ValueConversions.java >>> --- jdk8/src/share/classes/sun/invoke/util/ValueConversions.java >>> 2012-10-16 10:49:36.081911283 -0700 >>> +++ jdk7u/src/share/classes/sun/invoke/util/ValueConversions.java >>> 2012-10-16 10:48:19.626424849 -0700 >>> @@ -1211,9 +1211,13 @@ >>> >>> // handy shared exception makers (they simplify the common case code) >>> private static InternalError newInternalError(String message, Throwable >>> cause) { >>> - return new InternalError(message, cause); >>> + InternalError e = new InternalError(message); >>> + e.initCause(cause); >>> + return e; >>> } >>> private static InternalError newInternalError(Throwable cause) { >>> - return new InternalError(cause); >>> + InternalError e = new InternalError(); >>> + e.initCause(cause); >>> + return e; >>> } >>> } >>> diff --git a/src/share/classes/sun/misc/Unsafe.java >>> b/src/share/classes/sun/misc/Unsafe.java >>> --- a/src/share/classes/sun/misc/Unsafe.java >>> +++ b/src/share/classes/sun/misc/Unsafe.java >>> @@ -678,6 +678,14 @@ >>> public native Object staticFieldBase(Field f); >>> >>> /** >>> + * Detect if the given class may need to be initialized. This is often >>> + * needed in conjunction with obtaining the static field base of a >>> + * class. >>> + * @return false only if a call to {@code ensureClassInitialized} >>> would have no effect >>> + */ >>> + public native boolean shouldBeInitialized(Class c); >>> + >>> + /** >>> * Ensure the given class has been initialized. This is often >>> * needed in conjunction with obtaining the static field base of a >>> * class. >>> >> >
