----- Original Message ----- > > On Oct 18, 2012, at 6:48 PM, Seán Coffey <sean.cof...@oracle.com> > 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. >
Please delay. I don't see the need for the rush and I'd rather all of us didn't have to deal with this humongous changeset. > -- 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 > >> <christian.thalin...@oracle.com> wrote: > >> > >>> On Oct 18, 2012, at 12:00 PM, mark.reinh...@oracle.com wrote: > >>> > >>>> 2012/10/18 11:54 -0700, christian.thalin...@oracle.com: > >>>>> 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. > >> > > > > -- Andrew :) Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) PGP Key: 248BDC07 (https://keys.indymedia.org/) Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07
