[7u12] Request for approval for CR 8000999: backport of JSR 292 to 7u
Christian Thalinger
christian.thalinger at oracle.com
Fri Oct 19 17:07:19 PDT 2012
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.
>
>>
>> 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 <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