[7u12] Request for approval for CR 8000999: backport of JSR 292 to 7u
John Coomes
John.Coomes at oracle.com
Sat Oct 20 01:20:13 PDT 2012
Christian Thalinger (christian.thalinger at oracle.com) wrote:
>
> On Oct 19, 2012, at 11:26 AM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>
> >
> > On Oct 18, 2012, at 6:48 PM, Seán Coffey <sean.coffey at 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.
To clarify a bit: the integration of hotspot hs24 and Christian's jdk
changes have to be done simultaneously. They are interdependent, and
jsr292 functionality will not work unless both hotspot and the jdk are
matched.
Binaries for PIT (pre-integration testing) are being built as I write
this. Once the builds are done, we'll submit a bulk integration
request for both hs24 and these jdk changes.
-John
> >> 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/
PIT
> >> 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.thalinger at oracle.com> wrote:
> >>>
> >>>> On Oct 18, 2012, at 12:00 PM, mark.reinhold at oracle.com wrote:
> >>>>
> >>>>> 2012/10/18 11:54 -0700, christian.thalinger at 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.
> >>>
> >>
> >
>
More information about the jdk7u-dev
mailing list