Re: RFR: 8164525: Re-examine zero form link time pre-generation

[Vladimir Ivanov]

Reviewed.

Best regards,
Vladimir Ivanov

On 8/21/16 12:59 PM, Claes Redestad wrote:

Hi,

the previous attempt to pre-generate zero LambdaForms[1] failed
due to the current implementation surprisingly emitting placeholder
constants to link to the associated identity function (which we can't
properly handle when pre-generating forms) rather than simply emitting
the corresponding bytecode directly (iconst_0, etc).

It turns out the setup of Zero and Identity functions aren't being made
intrinsic, which appears to be an oversight. Fixing this allows for more
optimal bytecode[1].

While this fix could have positive performance implications in general,
it also makes the startup optimization from JDK-8164451 re-apply
without issue (no test failures running RBT --test jdk/test/:tier1).

Bug: https://bugs.openjdk.java.net/browse/JDK-8164525
Webrev: http://cr.openjdk.java.net/~redestad/8164525/webrev.01/

Thanks!

/Claes

[1] https://bugs.openjdk.java.net/browse/JDK-8164451
[2] Before:

  static java.lang.Object zero_000_L(java.lang.Object);
descriptor: (Ljava/lang/Object;)Ljava/lang/Object;
flags: ACC_STATIC
Code:
  stack=2, locals=1, args_size=1
 0: ldc   #14 // String
CONSTANT_PLACEHOLDER_1 <<(Object)Object :
identity_000_L=Lambda(a0:L,a1:L)=>{a1:L}\n& Class=SimpleMethodHandle>>
 2: checkcast #16 // class
java/lang/invoke/MethodHandle
 5: aconst_null
 6: invokevirtual #19 // Method
java/lang/invoke/MethodHandle.invokeBasic:(Ljava/lang/Object;)Ljava/lang/Object;

 9: areturn
RuntimeVisibleAnnotations:
  0: #8()
  1: #9()
  2: #10()

After:

  static java.lang.Object zero_L(java.lang.Object);
descriptor: (Ljava/lang/Object;)Ljava/lang/Object;
flags: ACC_STATIC
Code:
  stack=1, locals=1, args_size=1
 0: aconst_null
 1: areturn
RuntimeVisibleAnnotations:
  0: #8()
  1: #9()
  2: #10()

RFR: JDK-8164547: Make java.lang.reflect.ClassLoaderValue public for internal use

[Peter Levart]

Hi,

Several potential internal usages of currently package-private 
java.lang.reflect.ClassLoaderValue have been identified. Here is a 
proposal to move it to jdk.internal.loader package and make it public...


Task: https://bugs.openjdk.java.net/browse/JDK-8164547
Webrev: 
http://cr.openjdk.java.net/~plevart/jdk9-dev/ClassLoaderValue.public/webrev.01/


Regards, Peter

Re: RFR: JDK-8164547: Make java.lang.reflect.ClassLoaderValue public for internal use

[Aleksey Shipilev]

On 08/22/2016 01:15 PM, Peter Levart wrote:
> Webrev:
> http://cr.openjdk.java.net/~plevart/jdk9-dev/ClassLoaderValue.public/webrev.01/

Looks good to me.

AFAIU, making the classes public under the non-exported
jdk.internal.loader package is fine.

Thanks,
-Aleksey

Re: RFR: JDK-8164547: Make java.lang.reflect.ClassLoaderValue public for internal use

[Alan Bateman]

On 22/08/2016 11:15, Peter Levart wrote:

Hi,

Several potential internal usages of currently package-private 
java.lang.reflect.ClassLoaderValue have been identified. Here is a 
proposal to move it to jdk.internal.loader package and make it public...


Task: https://bugs.openjdk.java.net/browse/JDK-8164547
Webrev: 
http://cr.openjdk.java.net/~plevart/jdk9-dev/ClassLoaderValue.public/webrev.01/

jdk.internal.loader looks right, the change looks good to me.

-Alan

Re: RFR: JDK-8164547: Make java.lang.reflect.ClassLoaderValue public for internal use

[Peter Levart]

Hi Aleksey, Alan,

Thanks for reviewing. Pushed.

Regards, Peter

On 08/22/2016 12:27 PM, Alan Bateman wrote:



On 22/08/2016 11:15, Peter Levart wrote:

Hi,

Several potential internal usages of currently package-private 
java.lang.reflect.ClassLoaderValue have been identified. Here is a 
proposal to move it to jdk.internal.loader package and make it public...


Task: https://bugs.openjdk.java.net/browse/JDK-8164547
Webrev: 
http://cr.openjdk.java.net/~plevart/jdk9-dev/ClassLoaderValue.public/webrev.01/

jdk.internal.loader looks right, the change looks good to me.

-Alan

Re: [jdk9] RFR: 8164366: ZoneOffset.ofHoursMinutesSeconds() does not reject invalid input

[Seán Coffey]

Looks fine Ivan. Reviewed.

Regards,
Sean.

On 19/08/16 15:59, Ivan Gerasimov wrote:

On 19.08.2016 17:32, Stephen Colebourne wrote:

I'm less comfortable with the compareTo change because it may make it
slower and that may have knock on effects. I think a comment saying
that both are bounded would be enough in compareTo()


Okay.  I reverted that change back then and added a comment.
The webrev is updated:
http://cr.openjdk.java.net/~igerasim/8164366/02/webrev/

With kind regards,
Ivan


Stephen

On 19 August 2016 at 13:52, Ivan Gerasimov 
 wrote:

Thanks Nadeesh.  It's a good catch!

Here's the updated webrev:
http://cr.openjdk.java.net/~igerasim/8164366/01/webrev/

I also slightly modified comareTo(), not because there was an error 
in it,
but just to avoid thinking too much about possible overflow in 
subtraction

(of course, there can be no overflow here, as totalSeconds is bounded.)

Now we just need official blessing from the Reviewer.

With kind regards,
Ivan



On 19.08.2016 9:01, nadeesh tv wrote:

Hi Ivan,

ZoneOffset.ofTotalSeconds(Integer.MIN_VALUE) have also the same 
issue. It'

not throwing the expected DTE.

May be you can correct this also as part of this.

Please update the copyright year also.

Rest looks good.

Re: JDK 9 RFR of JDK-8164524: Correct inconsistencies in floating-point abs spec

[Brian Burkhalter]

Hi Joe,

This doc-only patch appears reasonable to me. Approved.

Brian

On Aug 20, 2016, at 11:55 AM, joe darcy  wrote:

> Please review the doc-only patch below to address
> 
>JDK-8164524: Correct inconsistencies in floating-point abs spec
> 
> In brief, Martin noted in JDK-8164199 that a close reading of the 
> specification of the Math and StrictMath floating-point abs methods reveals 
> some inconsistencies in the text of the specification versus the operational 
> semantics of the sample code in term of NaN handling.
> 
> To resolve this, the sample code is slightly modified and demoted to 
> informative rather than normative text.
> 
> The core of the edit is changing
> 
>Float.intBitsToFloat(0x7fff & Float.floatToIntBits(a))
> 
> to
> 
>Float.intBitsToFloat(0x7fff & Float.floatToRawIntBits(a))
> 
> that is the "raw" floating-point => integral conversion rather than the 
> "cooked" one which has tighter behavioral requirements around different NaN 
> values, analogous changes for the double cases.

Re: JDK 9 RFR of JDK-8164524: Correct inconsistencies in floating-point abs spec

[Joseph D. Darcy]

Hello,

I plan to push with a slightly different wording. Rather than

... but with a guaranteed positive sign bit:

using

...but with a guaranteed zero sign bit of a positive value:

I think the latter is clearer.

Thanks,

-Joe

On 8/22/2016 11:41 AM, Brian Burkhalter wrote:

Hi Joe,

This doc-only patch appears reasonable to me. Approved.

Brian

On Aug 20, 2016, at 11:55 AM, joe darcy > wrote:



Please review the doc-only patch below to address

   JDK-8164524: Correct inconsistencies in floating-point abs spec

In brief, Martin noted in JDK-8164199 that a close reading of the 
specification of the Math and StrictMath floating-point abs methods 
reveals some inconsistencies in the text of the specification versus 
the operational semantics of the sample code in term of NaN handling.


To resolve this, the sample code is slightly modified and demoted to 
informative rather than normative text.


The core of the edit is changing

   Float.intBitsToFloat(0x7fff & Float.floatToIntBits(a))

to

   Float.intBitsToFloat(0x7fff & Float.floatToRawIntBits(a))

that is the "raw" floating-point => integral conversion rather than 
the "cooked" one which has tighter behavioral requirements around 
different NaN values, analogous changes for the double cases.

Re: JDK 9 RFR of JDK-8164524: Correct inconsistencies in floating-point abs spec

[Hans Boehm]

The new version seems less clear to me. Could it be misread as only
applying if the value is positive? s/of/indicating a/ ?


On Mon, Aug 22, 2016 at 1:47 PM, Joseph D. Darcy 
wrote:

> Hello,
>
> I plan to push with a slightly different wording. Rather than
>
> ... but with a guaranteed positive sign bit:
>
> using
>
> ...but with a guaranteed zero sign bit of a positive value:
>
> I think the latter is clearer.
>
> Thanks,
>
> -Joe
>
> On 8/22/2016 11:41 AM, Brian Burkhalter wrote:
>
>> Hi Joe,
>>
>> This doc-only patch appears reasonable to me. Approved.
>>
>> Brian
>>
>> On Aug 20, 2016, at 11:55 AM, joe darcy > joe.da...@oracle.com>> wrote:
>>
>> Please review the doc-only patch below to address
>>>
>>>JDK-8164524: Correct inconsistencies in floating-point abs spec
>>>
>>> In brief, Martin noted in JDK-8164199 that a close reading of the
>>> specification of the Math and StrictMath floating-point abs methods reveals
>>> some inconsistencies in the text of the specification versus the
>>> operational semantics of the sample code in term of NaN handling.
>>>
>>> To resolve this, the sample code is slightly modified and demoted to
>>> informative rather than normative text.
>>>
>>> The core of the edit is changing
>>>
>>>Float.intBitsToFloat(0x7fff & Float.floatToIntBits(a))
>>>
>>> to
>>>
>>>Float.intBitsToFloat(0x7fff & Float.floatToRawIntBits(a))
>>>
>>> that is the "raw" floating-point => integral conversion rather than the
>>> "cooked" one which has tighter behavioral requirements around different NaN
>>> values, analogous changes for the double cases.
>>>
>>
>>
>

Re: JDK 9 RFR of JDK-8164524: Correct inconsistencies in floating-point abs spec

[Brian Burkhalter]

I think that sounds good.

Brian

On Aug 22, 2016, at 2:44 PM, Hans Boehm  wrote:

> The new version seems less clear to me. Could it be misread as only applying 
> if the value is positive? s/of/indicating a/ ?
> 
> 
> On Mon, Aug 22, 2016 at 1:47 PM, Joseph D. Darcy  wrote:
> Hello,
> 
> I plan to push with a slightly different wording. Rather than
> 
> ... but with a guaranteed positive sign bit:
> 
> using
> 
> ...but with a guaranteed zero sign bit of a positive value:
> 
> I think the latter is clearer.
> 
> Thanks,
> 
> -Joe
> 
> On 8/22/2016 11:41 AM, Brian Burkhalter wrote:
> Hi Joe,
> 
> This doc-only patch appears reasonable to me. Approved.
> 
> Brian
> 
> On Aug 20, 2016, at 11:55 AM, joe darcy  > wrote:
> 
> Please review the doc-only patch below to address
> 
>JDK-8164524: Correct inconsistencies in floating-point abs spec
> 
> In brief, Martin noted in JDK-8164199 that a close reading of the 
> specification of the Math and StrictMath floating-point abs methods reveals 
> some inconsistencies in the text of the specification versus the operational 
> semantics of the sample code in term of NaN handling.
> 
> To resolve this, the sample code is slightly modified and demoted to 
> informative rather than normative text.
> 
> The core of the edit is changing
> 
>Float.intBitsToFloat(0x7fff & Float.floatToIntBits(a))
> 
> to
> 
>Float.intBitsToFloat(0x7fff & Float.floatToRawIntBits(a))
> 
> that is the "raw" floating-point => integral conversion rather than the 
> "cooked" one which has tighter behavioral requirements around different NaN 
> values, analogous changes for the double cases.

8164585: JarFile::isMultiRelease does not return true in all cases where it should return true

[Steve Drach]

Hi,

Please review this simple change to JarFile

issue: https://bugs.openjdk.java.net/browse/JDK-8164585 

webrev: http://cr.openjdk.java.net/~sdrach/8164585/webrev.00/ 


Thanks,
Steve

Re: JDK 9 RFR of JDK-8164524: Correct inconsistencies in floating-point abs spec

[Joseph D. Darcy]

Pushed with suggested wording change; thanks,

-Joe

On 8/22/2016 3:24 PM, Brian Burkhalter wrote:

I think that sounds good.

Brian

On Aug 22, 2016, at 2:44 PM, Hans Boehm > wrote:


The new version seems less clear to me. Could it be misread as only 
applying if the value is positive? s/of/indicating a/ ?



On Mon, Aug 22, 2016 at 1:47 PM, Joseph D. Darcy>wrote:


Hello,

I plan to push with a slightly different wording. Rather than

... but with a guaranteed positive sign bit:

using

...but with a guaranteed zero sign bit of a positive value:

I think the latter is clearer.

Thanks,

-Joe

On 8/22/2016 11:41 AM, Brian Burkhalter wrote:

Hi Joe,

This doc-only patch appears reasonable to me. Approved.

Brian

On Aug 20, 2016, at 11:55 AM, joe darcy mailto:joe.da...@oracle.com>>> wrote:

Please review the doc-only patch below to address

   JDK-8164524: Correct inconsistencies in floating-point
abs spec

In brief, Martin noted in JDK-8164199 that a close
reading of the specification of the Math and StrictMath
floating-point abs methods reveals some inconsistencies
in the text of the specification versus the operational
semantics of the sample code in term of NaN handling.

To resolve this, the sample code is slightly modified and
demoted to informative rather than normative text.

The core of the edit is changing

   Float.intBitsToFloat(0x7fff & Float.floatToIntBits(a))

to

   Float.intBitsToFloat(0x7fff &
Float.floatToRawIntBits(a))

that is the "raw" floating-point => integral conversion
rather than the "cooked" one which has tighter behavioral
requirements around different NaN values, analogous
changes for the double cases.

RFR: 8148859 Fix module dependences for java/time tests

[Alexandre (Shura) Iline]

Hi.

Please review a quick fix for the java/time test module dependencies.
http://cr.openjdk.java.net/~shurailine/8148859/webrev.00/

It is using new syntax added in JTReg, which allows to only build certain 
classes from a test library for TestNG tests:
https://bugs.openjdk.java.net/browse/CODETOOLS-7901667

Shura

Re: RFR 8156073 : 2-slot LiveStackFrame locals (long and double) are incorrect

[Mandy Chung]

Hi Brent,


> On Aug 17, 2016, at 1:05 PM, Brent Christian  
> wrote:
> 
> Hi,
> 
> Please review this StackWalker fix in hotspot for 8156073, "2-slot 
> LiveStackFrame locals (long and double) are incorrect"
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8156073
> Webrev: http://cr.openjdk.java.net/~bchristi/8156073/webrev.01/

This looks a good stop-gap fix that will allow further experiments of using 
LiveStackFrame. The new test has long lines that should be wrapped.


> On 08/17/2016 02:28 PM, Frederic Parain wrote:
>> Another question: is it guaranteed that an unused slot from
>> a pair-slot is always zero? This is not written in the JVM spec,
>> and I don't remember that the JVM zeroes unused slots.
> 
> Interesting.  I've only seen zeros in my testing of locally-declared longs.  
> (How does other VM code account for unused local slots?).
> 

We need to follow up this issue to understand what the interpreter and compiler 
do for this unused slot and whether it’s always zero out.

We should file JBS issue to follow it up separately.

Mandy