Re: [13] RFR 8215913: [Test_bug]java/util/Locale/LocaleProvidersRun.java failed on de_DE and ja_JP locale.

[Rachna Goel]

Hi Dora,

Kindly update copyright years in both files and add bug id in 
LocaleProvidersRun.java.


Other than that, it looks good to me.

Thanks,

Rachna


On 1/4/19 7:58 AM, Dora Zhou wrote:

Hello,

Please help review the fix for the test bug 
java/util/Locale/LocaleProvidersRun.java failed on de_DE and ja_JP 
locale.


Bug: https://bugs.openjdk.java.net/browse/JDK-8215913

Webrev: http://cr.openjdk.java.net/~dzhou/8215913/webrev.00/

Thanks,
Dora



--
Thanks,
Rachna

Re: performance degradation in Array::newInstance on -XX:TieredStopAtLevel=1

[Claes Redestad]

Hi,

I've also taken a look at your microbenchmark and seen a few regressions
from 9 through 12, some of which I've identified - and some that might
be (partially) actionable. Mostly related to recent additions of low
overhead heap sampling and allocator/GC changes. All of the blame is in
hotspot, so let's leave the core-libs-devs alone for now. :-)

Some additional comments below...

On 2019-01-04 08:25, Сергей Цыпанов wrote:

Hi Claes,

thanks for the explanation, I suspected something like that.

I've run into this performance effect while investigating creation of Spring's 
ConcurrentReferenceHashMap,
it turned out that it used Array::newInstance to create array of References 
stored in a map's Segment:

private Reference[] createReferenceArray(int size) {
   return Array.newInstance(Reference.class, size);
}

The code above was rewritten into plain array constructor call gaining some 
performance improvement:

private Reference[] createReferenceArray(int size) {
   return new Reference[size];
}


while a point fix, avoiding reflection seems like the right thing to do
when the array type is known statically, anyhow.



This was the reason to go deeper and look how both methods behave.
The actual behaviour is the same on both JDK 8 and JDK 11.

And creation of ConcurrentReferenceHashMap is important on some workloads, in 
my case it's
database access via Spring Data where creation of ConcurrentReferenceHashMap 
takes approximately 1/5
of execution profile.

Talkin about Spring Boot it's possible to run SB application in IntelliJ IDEA 
in certain mode adding
-XX:TieredStopAtLevel=1 and -noverify VM options.

With full compilation the simplest application takes this to start up

   Mode  Cnt Score Error  Units
start-upss  100  2885,493 ± 167,660  ms/op

and with `-XX:TieredStopAtLevel=1 -noverify`

Benchmark Mode  Cnt ScoreError  Units
start-upss  100  1707,342 ± 75,166  ms/op


Thanks! Which JDK version are you using?

-noverify can be used without -XX:TieredStopAtLevel=1 (but don't use
this in production!). You might gain some by enabling CDS (run java
-Xshare:dump once, then add -Xshare:auto to your command lines). There
are a few other tricks to pull that might help startup without
sacrificing peak performance.

/Claes




Hi,

what you're seeing specifically here is likely the native overhead:
Array::newInstance calls into the native method Array::newArray, and C1
(TierStopAtLevel=1) doesn't have an intrinsic for this, while C2 does.

C1 and the interpreter will instead call into
Java_java_lang_reflect_Array_newArray in libjava / Array.c over JNI,
which will add a rather expensive constant overhead..

TieredStopAtLevel=1/C1 performance is expected to be relatively slower
than C2 in general, and often much worse in cases like this there are
optimized intrinsics at play.

Have you seen a regression here compared to some older JDK release?

It would also be very helpful if you could shed more light on the use
case and point out what particular startup issues you're seeing that
prevents you from using full tiered compilation and Spring Boot.

/Claes

On 2019-01-02 22:56, Сергей Цыпанов wrote:


Hello,

-XX:TieredStopAtLevel=1 flag is often used in some applications (e.g. Spring 
Boot based) to reduce start-up time.

With this flag I've spotted huge performance degradation of Array::newInstance 
comparing to plain constructor call.

I've used this benchmark

@State(Scope.Thread)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public class ArrayInstantiationBenchmark {

@Param({"10", "100", "1000"})
private int length;

@Benchmark
public Object newInstance() {
return Array.newInstance(Object.class, length);
}

@Benchmark
public Object constructor() {
return new Object[length];
}

}

On C2 (JDK 11) both methods perform the same:

Benchmark (length) Mode Cnt Score Error Units
ArrayInstantiationBenchmark.constructor 10 avgt 50 11,557 ± 0,316 ns/op
ArrayInstantiationBenchmark.constructor 100 avgt 50 86,944 ± 4,945 ns/op
ArrayInstantiationBenchmark.constructor 1000 avgt 50 520,722 ± 28,068 ns/op

ArrayInstantiationBenchmark.newInstance 10 avgt 50 11,899 ± 0,569 ns/op
ArrayInstantiationBenchmark.newInstance 100 avgt 50 86,805 ± 5,103 ns/op
ArrayInstantiationBenchmark.newInstance 1000 avgt 50 488,647 ± 20,829 ns/op

On C1 however there's a huge difference (approximately 8 times!) for length = 
10:

Benchmark (length) Mode Cnt Score Error Units
ArrayInstantiationBenchmark.constructor 10 avgt 50 11,183 ± 0,168 ns/op
ArrayInstantiationBenchmark.constructor 100 avgt 50 92,215 ± 4,425 ns/op
ArrayInstantiationBenchmark.constructor 1000 avgt 50 838,303 ± 33,161 ns/op

ArrayInstantiationBenchmark.newInstance 10 avgt 50 86,696 ± 1,297 ns/op
ArrayInstantiationBenchmark.newInstance 100 avgt 50 106,751 ± 2,796 ns/op
ArrayInstantiationBenchmark.newInstance 1000 avgt 50 840,582 ± 24,745 ns/op

Pay attention that performance for length = {100, 1000} is almost the same.

I 

Re: [12] RFR: 8215303: Allowing additional currency code points from later Unicode updates

[Rachna Goel]

Hi Naoto,

Your fix looks good to me.

Thanks,

Rachna

On 1/3/19 10:26 PM, Naoto Sato wrote:

Hello,

Please review the fix to the following issue (and its approved CSR):

https://bugs.openjdk.java.net/browse/JDK-8215303
https://bugs.openjdk.java.net/browse/JDK-8215305

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8215303/webrev.00/

This is a spec only modification that will allow future code point 
additions to the Unicode currency symbols block without any spec 
changes. This is especially important for update releases (such as 8u) 
without releasing a Maintenance Release.


Naoto


--
Thanks,
Rachna

Re: [12] RFR: 8215303: Allowing additional currency code points from later Unicode updates

[Rachna Goel]

Hi Naoto,

just one nit, copyright year need to be updated in Character.java.

Thanks,

Rachna


On 1/3/19 10:26 PM, Naoto Sato wrote:

Hello,

Please review the fix to the following issue (and its approved CSR):

https://bugs.openjdk.java.net/browse/JDK-8215303
https://bugs.openjdk.java.net/browse/JDK-8215305

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8215303/webrev.00/

This is a spec only modification that will allow future code point 
additions to the Unicode currency symbols block without any spec 
changes. This is especially important for update releases (such as 8u) 
without releasing a Maintenance Release.


Naoto


--
Thanks,
Rachna

Re: [12] RFR: 8215303: Allowing additional currency code points from later Unicode updates

[Nishit Jain]

Changes looks fine to me.

Regards,
Nishit Jain
On 03-01-2019 22:26, Naoto Sato wrote:

Hello,

Please review the fix to the following issue (and its approved CSR):

https://bugs.openjdk.java.net/browse/JDK-8215303
https://bugs.openjdk.java.net/browse/JDK-8215305

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8215303/webrev.00/

This is a spec only modification that will allow future code point 
additions to the Unicode currency symbols block without any spec 
changes. This is especially important for update releases (such as 8u) 
without releasing a Maintenance Release.


Naoto

Re: 8216134 (process) ProcessBuilder startPipeline does not hide piped streams

[Steve Groeger]

Roger, 

Looks OK. 

One small point, I know it is only a test but do you need the extra 
System.out.printf statements, they look as though they might have been 
just for debugging. 
No issues if they stay in there just wondering if it was a oversight in 
removing them.

Also, need to have the copyright dates changed as we are now in 2019.

Thanks
Steve Groeger
IBM Runtime Technologies
Hursley, Winchester
Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
Fax (44) 1962 816800
Lotus Notes: Steve Groeger/UK/IBM
Internet: groe...@uk.ibm.com

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU



From:   Roger Riggs 
To: core-libs-dev 
Date:   03/01/2019 20:48
Subject:8216134 (process) ProcessBuilder startPipeline does not 
hide piped streams
Sent by:"core-libs-dev" 



Please review a bug fix for the ProcessBuilder startPipeline test and 
Windows implementation.
The test failed to check that Process.getInputStream returned the null 
stream
for all but the last process in the pipeline.  When the test was fixed 
it failed on Windows.
The Windows ProcessImpl did not ensure that getInputStream returned a 
null stream.

The same issue was found and fixed in the AIX implementation (JDK-8211844)
which prompted investigation of the test.

Webrev:
  
http://cr.openjdk.java.net/~rriggs/webrev-pipeline-8211844/


Happy New Year!  Roger





Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

Re: [12] RFR: 8215303: Allowing additional currency code points from later Unicode updates

[Chris Hegarty]

On 1/3/19 10:26 PM, Naoto Sato wrote:
> Hello,
> 
> Please review the fix to the following issue (and its approved CSR):
> 
> https://bugs.openjdk.java.net/browse/JDK-8215303
> https://bugs.openjdk.java.net/browse/JDK-8215305
> 
> The proposed changeset is located at:
> 
> http://cr.openjdk.java.net/~naoto/8215303/webrev.00/

I think this is a positive change, but for clarity ( since I cannot find it
in the CSR ), will this change have an impact on the set of allowable
characters that can be used as identifiers, i.e. isJavaIdentifierStart,
isJavaIdentifierPart?

-Chris.

Re: 8216134 (process) ProcessBuilder startPipeline does not hide piped streams

[Roger Riggs]

Hi Brent, Steve,

Thanks for the review and corrections for copyrights, removing debugging 
info, and

input file contents.

Updated Webrev:
  http://cr.openjdk.java.net/~rriggs/webrev-pipeline-8211844-2/

Regards, Roger

On 01/04/2019 06:03 AM, Steve Groeger wrote:

Roger,

Looks OK.

One small point, I know it is only a test but do you need the extra 
System.out.printf statements, they look as though they might have been 
just for debugging.
No issues if they stay in there just wondering if it was a oversight 
in removing them.


Also, need to have the copyright dates changed as we are now in 2019.

Thanks
Steve Groeger
IBM Runtime Technologies
Hursley, Winchester
Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
Fax (44) 1962 816800
Lotus Notes: Steve Groeger/UK/IBM
Internet: groe...@uk.ibm.com

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with 
number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
3AU




From: Roger Riggs 
To: core-libs-dev 
Date: 03/01/2019 20:48
Subject: 8216134 (process) ProcessBuilder startPipeline does not hide 
piped streams

Sent by: "core-libs-dev" 




Please review a bug fix for the ProcessBuilder startPipeline test and
Windows implementation.
The test failed to check that Process.getInputStream returned the null
stream
for all but the last process in the pipeline.  When the test was fixed
it failed on Windows.
The Windows ProcessImpl did not ensure that getInputStream returned a
null stream.

The same issue was found and fixed in the AIX implementation (JDK-8211844)
which prompted investigation of the test.

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-pipeline-8211844/ 



Happy New Year!  Roger





Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with 
number 741598.

Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

Re: [13] RFR 8215913: [Test_bug]java/util/Locale/LocaleProvidersRun.java failed on de_DE and ja_JP locale.

[Roger Riggs]

+1

On 01/04/2019 04:44 AM, Rachna Goel wrote:

Hi Dora,

Kindly update copyright years in both files and add bug id in 
LocaleProvidersRun.java.


Other than that, it looks good to me.

Thanks,

Rachna


On 1/4/19 7:58 AM, Dora Zhou wrote:

Hello,

Please help review the fix for the test bug 
java/util/Locale/LocaleProvidersRun.java failed on de_DE and ja_JP 
locale.


Bug: https://bugs.openjdk.java.net/browse/JDK-8215913

Webrev: http://cr.openjdk.java.net/~dzhou/8215913/webrev.00/

Thanks,
Dora

Re: RFR: 8215412: Optimize PrintStream.println methods

[Roger Riggs]

Hi Claes,

The logic looks fine, but the 'internal' field name doesn't mean anything
and there's no description of the optimization.

The field could be renamed:  noOverride or notOverridden or noDoubleSync 
or ...

or add a comment to the field describing its purpose.

Update copyright date on PrintStream.java.

Thanks, Roger

On 01/02/2019 03:44 AM, Claes Redestad wrote:

Hi,

new webrev:

 http://cr.openjdk.java.net/~redestad/8215412/jdk.02/

- Adds a forgotten String.valueOf in println(String), making sure all
methods are semantically the same for
- Remove the isInternal method and use only exact match with
PrintStream.class to determine whether to use the optimized paths.

As before:

On 2018-12-14 21:22, Claes Redestad wrote:

Same performance characteristics in the simple tests I've used to
verify this, and no measurable regression (but no speed-up) for classes
overriding PrintStream.


I've withdrawn the related CSR, since it's not relevant for this
implementation as it doesn't observably alter behavior of types
inheriting from PrintStream.

Thanks!

/Claes

Re: RFR: 8215412: Optimize PrintStream.println methods

[Remi Forax]

Hi Roger,
the field has disappear in the latest webrev.

Rémi

- Mail original -
> De: "Roger Riggs" 
> À: "Claes Redestad" , "core-libs-dev" 
> 
> Envoyé: Vendredi 4 Janvier 2019 16:38:06
> Objet: Re: RFR: 8215412: Optimize PrintStream.println methods

> Hi Claes,
> 
> The logic looks fine, but the 'internal' field name doesn't mean anything
> and there's no description of the optimization.
> 
> The field could be renamed:  noOverride or notOverridden or noDoubleSync
> or ...
> or add a comment to the field describing its purpose.
> 
> Update copyright date on PrintStream.java.
> 
> Thanks, Roger
> 
> On 01/02/2019 03:44 AM, Claes Redestad wrote:
>> Hi,
>>
>> new webrev:
>>
>>  http://cr.openjdk.java.net/~redestad/8215412/jdk.02/
>>
>> - Adds a forgotten String.valueOf in println(String), making sure all
>> methods are semantically the same for
>> - Remove the isInternal method and use only exact match with
>> PrintStream.class to determine whether to use the optimized paths.
>>
>> As before:
>>
>> On 2018-12-14 21:22, Claes Redestad wrote:
>>> Same performance characteristics in the simple tests I've used to
>>> verify this, and no measurable regression (but no speed-up) for classes
>>> overriding PrintStream.
>>
>> I've withdrawn the related CSR, since it's not relevant for this
>> implementation as it doesn't observably alter behavior of types
>> inheriting from PrintStream.
>>
>> Thanks!
>>
> > /Claes

Re: RFR: 8215412: Optimize PrintStream.println methods

[forax]

The perf diff is more that what i was expected :)

Looks good to me !

Rémi 

- Mail original -
> De: "Claes Redestad" 
> À: "Remi Forax" 
> Cc: "core-libs-dev" 
> Envoyé: Mercredi 2 Janvier 2019 21:32:09
> Objet: Re: RFR: 8215412: Optimize PrintStream.println methods

> Hi again,
> 
> as expected not much of a difference on the test I was using (a small
> cost on the startup/warmup numbers due replacing a boolean check with a
> method call), but on a test that adds in a few extended PrintStreams
> and mixes those with use of System.out I can get a ~3-4% statistical
> improvement with your idea - so I think it's better to peel off
> the PrintStream.class check like this:
> 
> http://cr.openjdk.java.net/~redestad/8215412/jdk.03/
> 
> Thanks!
> 
> /Claes
> 
> On 2019-01-02 13:29, Claes Redestad wrote:
>> Hi Rémi,
>> 
>> 
>> On 2019-01-02 13:05, Remi Forax wrote:
>>> Hi Claes,
>>> did you try instead of having a field 'internal' to inline the class
>>> check (this.getClass() == PrintStream.class) at every call site you
>>> are reading that field ?
>>>
>>> for a method like println(), the VM has to do a class check (if CHA is
>>> defeated) before entering the method println so the JIT might be able
>>> to remove the check this.getClass() == PrintStream.class because it
>>> already know at that point that the current class is PrintStream.
>> 
>> interesting idea - I'll try it out. I don't expect it to make much of a
>> difference in my current (trivial) tests, but perhaps in a slightly more
>> contrived setup.
>> 
>> happy new year!
>> 
> > /Claes

Re: RFR: 8215412: Optimize PrintStream.println methods

[Roger Riggs]

Oops, missed a revision.

Looks good, though the reader may puzzle a while about why.

Thanks, Roger

On 01/04/2019 10:44 AM, Remi Forax wrote:

Hi Roger,
the field has disappear in the latest webrev.

Rémi

- Mail original -

De: "Roger Riggs" 
À: "Claes Redestad" , "core-libs-dev" 

Envoyé: Vendredi 4 Janvier 2019 16:38:06
Objet: Re: RFR: 8215412: Optimize PrintStream.println methods
Hi Claes,

The logic looks fine, but the 'internal' field name doesn't mean anything
and there's no description of the optimization.

The field could be renamed:  noOverride or notOverridden or noDoubleSync
or ...
or add a comment to the field describing its purpose.

Update copyright date on PrintStream.java.

Thanks, Roger

On 01/02/2019 03:44 AM, Claes Redestad wrote:

Hi,

new webrev:

  http://cr.openjdk.java.net/~redestad/8215412/jdk.02/

- Adds a forgotten String.valueOf in println(String), making sure all
methods are semantically the same for
- Remove the isInternal method and use only exact match with
PrintStream.class to determine whether to use the optimized paths.

As before:

On 2018-12-14 21:22, Claes Redestad wrote:

Same performance characteristics in the simple tests I've used to
verify this, and no measurable regression (but no speed-up) for classes
overriding PrintStream.

I've withdrawn the related CSR, since it's not relevant for this
implementation as it doesn't observably alter behavior of types
inheriting from PrintStream.

Thanks!

/Claes

Re: Class.getDeclaredMethods() is returning inherited methods

[Roger Riggs]

Hi David,

Its worth considering and so I reset the issue so it would be re-triaged and
assigned for an appropriate person to consider.

Roger


On 01/03/2019 05:58 PM, David Holmes wrote:

Hi Roger,

On 4/01/2019 12:22 am, Roger Riggs wrote:

Hi,

With a link to the explanation added to the issue, I think it can be 
closed as not-an-issue.


Do you think the Class.getDeclared* method specs should be updated to 
reflect (pardon the pun) that synthetic methods/constructors will be 
included?


Cheers,
David

8215798: [javadoc] Use {@systemProperty} for org.openjdk.java.util.stream.tripwire system property

[Roger Riggs]

Please review using @systemProperty to refer to the 
org.openjdk.java.util.stream.tripwire

system property.


diff --git a/src/java.base/share/classes/java/util/Spliterator.java 
b/src/java.base/share/classes/java/util/Spliterator.java

--- a/src/java.base/share/classes/java/util/Spliterator.java
+++ b/src/java.base/share/classes/java/util/Spliterator.java
@@ -284,7 +284,7 @@ import java.util.function.LongConsumer;
  * }}
  *
  * @implNote
- * If the boolean system property {@code 
org.openjdk.java.util.stream.tripwire}
+ * If the boolean system property {@systemProperty 
org.openjdk.java.util.stream.tripwire}
  * is set to {@code true} then diagnostic warnings are reported if 
boxing of
  * primitive values occur when operating on primitive subtype 
specializations.

  *

Thanks, Roger

Re: RFR: 8215412: Optimize PrintStream.println methods

[Claes Redestad]

On 2019-01-04 16:46, fo...@univ-mlv.fr wrote:

The perf diff is more that what i was expected :)

Looks good to me !


Thanks!

/Claes

Re: RFR: 8215412: Optimize PrintStream.println methods

[Claes Redestad]

On 2019-01-04 16:55, Roger Riggs wrote:

Oops, missed a revision.

Looks good, 


Thanks!

> though the reader may puzzle a while about why.

I'll mull a bit about a succinct comment, but at least the version
history will point to this RFE..

/Claes

Re: 8215798: [javadoc] Use {@systemProperty} for org.openjdk.java.util.stream.tripwire system property

[Lance Andersen]

+1
> On Jan 4, 2019, at 11:18 AM, Roger Riggs  wrote:
> 
> Please review using @systemProperty to refer to the 
> org.openjdk.java.util.stream.tripwire
> system property.
> 
> 
> diff --git a/src/java.base/share/classes/java/util/Spliterator.java 
> b/src/java.base/share/classes/java/util/Spliterator.java
> --- a/src/java.base/share/classes/java/util/Spliterator.java
> +++ b/src/java.base/share/classes/java/util/Spliterator.java
> @@ -284,7 +284,7 @@ import java.util.function.LongConsumer;
>   * }}
>   *
>   * @implNote
> - * If the boolean system property {@code 
> org.openjdk.java.util.stream.tripwire}
> + * If the boolean system property {@systemProperty 
> org.openjdk.java.util.stream.tripwire}
>   * is set to {@code true} then diagnostic warnings are reported if boxing of
>   * primitive values occur when operating on primitive subtype 
> specializations.
>   *
> 
> Thanks, Roger

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 

Re: [12] RFR: 8215303: Allowing additional currency code points from later Unicode updates

[naoto . sato]

Hi Chris,

Yes, it will affect the behavior of those methods. This has been 
discussed within the JLS folks, and their understanding was that the 
risk is minimal and OK to proceed. I was not involved in the discussion, 
but here are the reasons I can think of.


- The Currency Symbols range is very limited (U+20A0-U+20CF)
- The change is to allow the code point, not the way around, so existing 
identifiers are guaranteed to work.
- Apart from this CSR, this kind of behavior change is common when a 
Unicode upgrade is done.


Naoto

On 1/4/19 6:13 AM, Chris Hegarty wrote:



On 1/3/19 10:26 PM, Naoto Sato wrote:

Hello,

Please review the fix to the following issue (and its approved CSR):

https://bugs.openjdk.java.net/browse/JDK-8215303
https://bugs.openjdk.java.net/browse/JDK-8215305

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8215303/webrev.00/


I think this is a positive change, but for clarity ( since I cannot find it
in the CSR ), will this change have an impact on the set of allowable
characters that can be used as identifiers, i.e. isJavaIdentifierStart,
isJavaIdentifierPart?

-Chris.

Re: [13] RFR 8215913: [Test_bug]java/util/Locale/LocaleProvidersRun.java failed on de_DE and ja_JP locale.

[naoto . sato]

+1

Naoto

On 1/4/19 7:21 AM, Roger Riggs wrote:

+1

On 01/04/2019 04:44 AM, Rachna Goel wrote:

Hi Dora,

Kindly update copyright years in both files and add bug id in 
LocaleProvidersRun.java.


Other than that, it looks good to me.

Thanks,

Rachna


On 1/4/19 7:58 AM, Dora Zhou wrote:

Hello,

Please help review the fix for the test bug 
java/util/Locale/LocaleProvidersRun.java failed on de_DE and ja_JP 
locale.


Bug: https://bugs.openjdk.java.net/browse/JDK-8215913

Webrev: http://cr.openjdk.java.net/~dzhou/8215913/webrev.00/

Thanks,
Dora

Re: [12] RFR: 8215303: Allowing additional currency code points from later Unicode updates

[Chris Hegarty]

Thanks Naoto.

> On 4 Jan 2019, at 17:10, naoto.s...@oracle.com wrote:
> 
> Hi Chris,
> 
> Yes, it will affect the behavior of those methods. This has been discussed 
> within the JLS folks, and their understanding was that the risk is minimal 
> and OK to proceed. I was not involved in the discussion, but here are the 
> reasons I can think of.

Ok. Should the CSR be updated to indicate this?

> - The Currency Symbols range is very limited (U+20A0-U+20CF)
> - The change is to allow the code point, not the way around, so existing 
> identifiers are guaranteed to work.
> - Apart from this CSR, this kind of behavior change is common when a Unicode 
> upgrade is done.

Sure, all good and valid reasons.

Will compilations with `--release N-1` wind back the set of allowable 
identifiers?  It possibly should, if so how does one do similar when/if
the set of currency characters is expanded in an update release?

-Chris.

> Naoto
> 
> On 1/4/19 6:13 AM, Chris Hegarty wrote:
>> On 1/3/19 10:26 PM, Naoto Sato wrote:
>>> Hello,
>>> 
>>> Please review the fix to the following issue (and its approved CSR):
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8215303
>>> https://bugs.openjdk.java.net/browse/JDK-8215305
>>> 
>>> The proposed changeset is located at:
>>> 
>>> http://cr.openjdk.java.net/~naoto/8215303/webrev.00/
>> I think this is a positive change, but for clarity ( since I cannot find it
>> in the CSR ), will this change have an impact on the set of allowable
>> characters that can be used as identifiers, i.e. isJavaIdentifierStart,
>> isJavaIdentifierPart?
>> -Chris.

Re: RFR: 8182992 Typo in DatagramPacket constructor API doc

[Roger Calnan]

>> - * suitable for retrieving large{@code LONGVARCHAR}values.  The JDBC 
>> driver will
>> + * suitable for retrieving large{@code LONGVARCHAR} values.  The JDBC 
>> driver will
> Needs a space after large

indeed, I looked for other examples and found a couple more.  I’ll file 
a bug to get them fixed,

Thanks,

Roger

> From: Lance Andersen 
> Subject: Re: RFR: 8182992 Typo in DatagramPacket constructor API doc
> Date: January 3, 2019 at 9:53:07 AM PST
> To: Roger Calnan 
> Cc: core-libs-dev@openjdk.java.net
> 
> 
> Hi Roger,
> 
> Please see below.  I am not sure how much review the javadocs have gotten  
> for the com/sun/rowset/internal classes
> 
> 
>> 
>> 
>> 
>> JDK-8215911 Various Typos in SQL Method Documentation
>> https://bugs.openjdk.java.net/browse/JDK-8215911 
>>  
>> > >https://hg.openjdk.java.net/jdk/jdk/file/abe21b82ff7c/src/java.sql.rowset/share/classes/com/sun/rowset/internal/SyncResolverImpl.java#l1067
>>  
>> 
>>  
>> >  
>> >
>> 
>> diff -r 3d0f6ef91216 
>> src/java.sql.rowset/share/classes/com/sun/rowset/internal/SyncResolverImpl.java
>> --- 
>> a/src/java.sql.rowset/share/classes/com/sun/rowset/internal/SyncResolverImpl.java
>> Wed Jan 02 13:37:55 2019 -0500
>> +++ 
>> b/src/java.sql.rowset/share/classes/com/sun/rowset/internal/SyncResolverImpl.java
>> Wed Jan 02 13:19:38 2019 -0800
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2004, 2013, Oracle and/or its affiliates. All rights 
>> reserved.
>> + * Copyright (c) 2004, 2019, Oracle and/or its affiliates. All rights 
>> reserved.
>>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>  *
>>  * This code is free software; you can redistribute it and/or modify it
>> @@ -1064,7 +1064,7 @@
>> 
>> /**
>>  * Returns the insert row or the current row of this
>> - * {@code CachedRowSetImpl}object.
>> + * {@code CachedRowSetImpl} object.
>>  *
>>  * @return the {@code Row} object on which this {@code CachedRowSetImpl}
>>  * objects's cursor is positioned
>> @@ -4326,7 +4326,7 @@
>>  *
>>  * @param columnName a {@code String} object that must match the
>>  *SQL name of a column in this rowset, ignoring case
>> - * @param c the new column {@code Clob}value
>> + * @param c the new column {@code Clob} value
>>  * @throws SQLException if (1) the given column name does not match the
>>  *name of a column in this rowset, (2) the cursor is not on
>>  *one of this rowset's rows or its insert row, or (3) this
>> 
>> https://hg.openjdk.java.net/jdk/jdk/file/abe21b82ff7c/src/java.sql.rowset/share/classes/com/sun/rowset/JdbcRowSetImpl.java#l1136
>>  
>> 
>>  
>> >  
>> >
>> https://hg.openjdk.java.net/jdk/jdk/file/abe21b82ff7c/src/java.sql.rowset/share/classes/com/sun/rowset/JdbcRowSetImpl.java#l6191
>>  
>> 
>>  
>> >  
>> >
>> 
>> diff -r abe21b82ff7c 
>> src/java.sql.rowset/share/classes/com/sun/rowset/JdbcRowSetImpl.java
>> diff -r 3d0f6ef91216 
>> src/java.sql.rowset/share/classes/com/sun/rowset/JdbcRowSetImpl.java
>> --- a/src/java.sql.rowset/share/classes/com/sun/rowset/JdbcRowSetImpl.java   
>> Wed Jan 02 13:37:55 2019 -0500
>> +++ b/src/java.sql.rowset/share/classes/com/sun/rowset/JdbcRowSetImpl.java   
>> Wed Jan 02 13:19:38 2019 -0800
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights 
>> reserved.
>> + * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights 
>> reserved.
>>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>  *
>>  * This code is free software; you can redistribute 

Re: [12] RFR: 8215303: Allowing additional currency code points from later Unicode updates

[naoto . sato]

Hi Rachna,

Updated: http://cr.openjdk.java.net/~naoto/8215303/webrev.01/

Naoto

On 1/4/19 2:24 AM, Rachna Goel wrote:

Hi Naoto,

just one nit, copyright year need to be updated in Character.java.

Thanks,

Rachna


On 1/3/19 10:26 PM, Naoto Sato wrote:

Hello,

Please review the fix to the following issue (and its approved CSR):

https://bugs.openjdk.java.net/browse/JDK-8215303
https://bugs.openjdk.java.net/browse/JDK-8215305

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8215303/webrev.00/

This is a spec only modification that will allow future code point 
additions to the Unicode currency symbols block without any spec 
changes. This is especially important for update releases (such as 8u) 
without releasing a Maintenance Release.


Naoto


--
Thanks,
Rachna

Re: [12] RFR: 8215303: Allowing additional currency code points from later Unicode updates

[naoto . sato]

Hi Chris,

Yes. I just updated the CSR, adding the description in the compatibility 
risk:


https://bugs.openjdk.java.net/browse/JDK-8215305

Naoto

On 1/4/19 9:18 AM, Chris Hegarty wrote:

Thanks Naoto.


On 4 Jan 2019, at 17:10, naoto.s...@oracle.com wrote:

Hi Chris,

Yes, it will affect the behavior of those methods. This has been discussed 
within the JLS folks, and their understanding was that the risk is minimal and 
OK to proceed. I was not involved in the discussion, but here are the reasons I 
can think of.


Ok. Should the CSR be updated to indicate this?


- The Currency Symbols range is very limited (U+20A0-U+20CF)
- The change is to allow the code point, not the way around, so existing 
identifiers are guaranteed to work.
- Apart from this CSR, this kind of behavior change is common when a Unicode 
upgrade is done.


Sure, all good and valid reasons.

Will compilations with `--release N-1` wind back the set of allowable
identifiers?  It possibly should, if so how does one do similar when/if
the set of currency characters is expanded in an update release?

-Chris.


Naoto

On 1/4/19 6:13 AM, Chris Hegarty wrote:

On 1/3/19 10:26 PM, Naoto Sato wrote:

Hello,

Please review the fix to the following issue (and its approved CSR):

https://bugs.openjdk.java.net/browse/JDK-8215303
https://bugs.openjdk.java.net/browse/JDK-8215305

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8215303/webrev.00/

I think this is a positive change, but for clarity ( since I cannot find it
in the CSR ), will this change have an impact on the set of allowable
characters that can be used as identifiers, i.e. isJavaIdentifierStart,
isJavaIdentifierPart?
-Chris.

Re: 8216134 (process) ProcessBuilder startPipeline does not hide piped streams

[Brent Christian]

Looks good.
If you wanted to break up L278 in the test before pushing, I wouldn't 
complain. :)


-Brent

On 1/4/19 6:52 AM, Roger Riggs wrote:

Hi Brent, Steve,

Thanks for the review and corrections for copyrights, removing debugging 
info, and

input file contents.

Updated Webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-pipeline-8211844-2/

Regards, Roger

On 01/04/2019 06:03 AM, Steve Groeger wrote:

Roger,

Looks OK.

One small point, I know it is only a test but do you need the extra 
System.out.printf statements, they look as though they might have been 
just for debugging.
No issues if they stay in there just wondering if it was a oversight 
in removing them.


Also, need to have the copyright dates changed as we are now in 2019.

Thanks
Steve Groeger
IBM Runtime Technologies
Hursley, Winchester
Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
Fax (44) 1962 816800
Lotus Notes: Steve Groeger/UK/IBM
Internet: groe...@uk.ibm.com

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with 
number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
3AU




From: Roger Riggs 
To: core-libs-dev 
Date: 03/01/2019 20:48
Subject: 8216134 (process) ProcessBuilder startPipeline does not hide 
piped streams

Sent by: "core-libs-dev" 




Please review a bug fix for the ProcessBuilder startPipeline test and
Windows implementation.
The test failed to check that Process.getInputStream returned the null
stream
for all but the last process in the pipeline.  When the test was fixed
it failed on Windows.
The Windows ProcessImpl did not ensure that getInputStream returned a
null stream.

The same issue was found and fixed in the AIX implementation 
(JDK-8211844)

which prompted investigation of the test.

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-pipeline-8211844/ 



Happy New Year!  Roger





Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with 
number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
3AU

Re: 8216134 (process) ProcessBuilder startPipeline does not hide piped streams

[Roger Riggs]

Thanks Brent, I will re-wrap L278 before pushing.

On 01/04/2019 12:59 PM, Brent Christian wrote:

Looks good.
If you wanted to break up L278 in the test before pushing, I wouldn't 
complain. :)


-Brent

On 1/4/19 6:52 AM, Roger Riggs wrote:

Hi Brent, Steve,

Thanks for the review and corrections for copyrights, removing 
debugging info, and

input file contents.

Updated Webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-pipeline-8211844-2/

Regards, Roger

On 01/04/2019 06:03 AM, Steve Groeger wrote:

Roger,

Looks OK.

One small point, I know it is only a test but do you need the extra 
System.out.printf statements, they look as though they might have 
been just for debugging.
No issues if they stay in there just wondering if it was a oversight 
in removing them.


Also, need to have the copyright dates changed as we are now in 2019.

Thanks
Steve Groeger
IBM Runtime Technologies
Hursley, Winchester
Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
Fax (44) 1962 816800
Lotus Notes: Steve Groeger/UK/IBM
Internet: groe...@uk.ibm.com

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with 
number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire 
PO6 3AU




From: Roger Riggs 
To: core-libs-dev 
Date: 03/01/2019 20:48
Subject: 8216134 (process) ProcessBuilder startPipeline does not 
hide piped streams

Sent by: "core-libs-dev" 
 





Please review a bug fix for the ProcessBuilder startPipeline test and
Windows implementation.
The test failed to check that Process.getInputStream returned the null
stream
for all but the last process in the pipeline.  When the test was fixed
it failed on Windows.
The Windows ProcessImpl did not ensure that getInputStream returned a
null stream.

The same issue was found and fixed in the AIX implementation 
(JDK-8211844)

which prompted investigation of the test.

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-pipeline-8211844/ 



Happy New Year!  Roger





Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with 
number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire 
PO6 3AU

Re: performance degradation in Array::newInstance on -XX:TieredStopAtLevel=1

[Martin Buchholz]

On Thu, Jan 3, 2019 at 11:26 PM Сергей Цыпанов 
wrote:

>
> I've run into this performance effect while investigating creation of
> Spring's ConcurrentReferenceHashMap,
> it turned out that it used Array::newInstance to create array of
> References stored in a map's Segment:
>

Slight tangent - ConcurrentHashMap used to use Segments, but no longer
does.  Probably  ConcurrentReferenceHashMap is based on an old version of
ConcurrentHashMap and would benefit from a re-port.

Re: RFR(JDK 13/java.xml) 8215330: javax.xml.catalog.CatalogResolverImpl: GroupEntry.matchURI fails to match

[Joe Wang]

Thanks Lance!  The change is in now.

-Joe

On 1/3/19, 3:48 PM, Lance Andersen wrote:

Hi Joe,

The changes and test seem fine!

Happy New Year

On Jan 3, 2019, at 6:25 PM, Joe Wang > wrote:


Hi,

Please review a fix to the impl for the Catalog. The reporter was 
right, in the Group case, we failed to update the currently longest 
match when a match is found.


JBS: https://bugs.openjdk.java.net/browse/JDK-8215330
webrevs: http://cr.openjdk.java.net/~joehw/jdk13/8215330/webrev/ 



Thanks,
Joe





Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 

Re: RFR: 8215412: Optimize PrintStream.println methods

[Claes Redestad]

On 2019-01-04 17:39, Claes Redestad wrote:


I'll mull a bit about a succinct comment, but at least the version
history will point to this RFE..


Lest I hear any objections, I'll push with this comment added to the
two new writeln methods:

 // Used to optimize away back-to-back flushing and synchronization when
 // using println, but since subclasses could exist which depend on
 // observing a call to print followed by newLine we only use this if
 // getClass() == PrintStream.class to avoid compatibility issues.

/Claes

Re: RFR: 8215412: Optimize PrintStream.println methods

[Roger Riggs]

+1

Thanks

On 01/04/2019 02:40 PM, Claes Redestad wrote:



On 2019-01-04 17:39, Claes Redestad wrote:


I'll mull a bit about a succinct comment, but at least the version
history will point to this RFE..


Lest I hear any objections, I'll push with this comment added to the
two new writeln methods:

 // Used to optimize away back-to-back flushing and synchronization when
 // using println, but since subclasses could exist which depend on
 // observing a call to print followed by newLine we only use this if
 // getClass() == PrintStream.class to avoid compatibility issues.

/Claes

Feature suggestion: Allow generic wildcard in class literal expression

[some-java-user-99206970363698485155]

JDK-6184881 describes that Object.getClass() should ideally not return classes 
of raw types anymore but instead use wildcards. The problem is that, as noted 
in the comments, this breaks compability with previously written code and 
therefore this change is not very likely (at the moment?).

However, the report does not cover the class literal expression 
(https://docs.oracle.com/javase/specs/jls/se11/html/jls-15.html#jls-15.8.2).

It would be good to allow the usage of the wildcard (`?`) for this expression:
List.class => Class>

Since it is currently not allowed to provide type parameters in the class 
literal expression there should not be any compatibility issues.
Additionally if at some point JDK-6184881 is fixed for class literals as well, 
then the expression List.class could just be a verbose variant of List.class 
without causing any problems either.


This would make writing methods or constructors which determine a generic type 
based on a given class easier for generic classes, e.g.:

public class MyContainer {
private final Class valueClass;

public MyContainer(Class valueClass) {
this.valueClass = valueClass;
}

public static void main(String[] args) {
MyContainer stringContainer = new MyContainer<>(String.class);
// Have to use raw type (or verbose casting of class)
MyContainer listContainer = new MyContainer<>(List.class);
 
// With suggested change:
MyContainer> listContainer = new MyContainer<>(List.class);
 }
}

Feature suggestion: Add static equals methods to Float and Double

[some-java-user-99206970363698485155]

To test whether primitive float or double values are equal according to 
`Float.equals` and `Double.equals` you either have to create wrapper instances 
for them (possible performance decrease), use the respective static `compareTo` 
(verbose) or have to use the appropriate methods (`floatToIntBits` / 
`doubleToLongBits`) (verbose and error-prone since you could confuse them with 
the other conversion methods).

It would be good to provide static methods for testing for equality of the 
primitive values:

// In Float.java

public static boolean equals(float a, float b) {
return Float.floatToIntBits(a) == Float.floatToIntBits(b);
}

// In Double.java

public static boolean equals(double a, double b) {
return Double.doubleToLongBits(a) == Double.doubleToLongBits(b);
}

This would be very convenient for developers and prevent them from writing 
(possibly faulty) code for this themselves.

Feature suggestion: Provide efficient way for nullable value lookup in Map

[some-java-user-99206970363698485155]

The methods currently provided by the Map interface 
(https://docs.oracle.com/javase/8/docs/api/java/util/Map.html) do not provide 
an efficient way to look up nullable values. This problem would be solved if 
JDK-6552529 was implemented, but that will likely not be happening since the 
interface cannot be changed without breaking compatibility.

Ways to currently look up nullable values are:

* Combined ussage of `get()` and `containsKey()` -> inefficient
* Usage of `getOrDefault()` with private no-value constant, e.g.:
private static final Object NO_VALUE = new Object();
// ...
V value = map.getOrDefault(key, (V) NO_VALUE);
if (value == NO_VALUE) {
 // No value
}
else {
 // Value exists
}

Both solutions are not really that great, therefore it would be good to provide 
an easier and (if overridden) more efficient method: default 
OptionalNullable getOptional(Object key) {
V value = get(key);
   
if (value != null || containsKey(key)) {
return OptionalNullable.of(value);
}
else {
return OptionalNullable.empty();
}
}

Where `OptionalNullable` is a new class similar to `Optional` except that null 
is considered a value.

Maybe a `void ifExists(K key, Consumer consumer)`, which makes the consumer 
consume the value if it exists (= nullable), would be good as well to avoid 
creation of `OptionalNullable` objects, though that method might (at least for 
performance reasons) become obsolute when `OptionalNullable` becomes a value 
type.

Re: Feature suggestion: Add static equals methods to Float and Double

[Remi Forax]

Hi,
it's not obvious to me that this is a source backward compatible change.

if you have something like:
  interface Fun { boolean eq(Double a, double b); }
  ...
  Fun fun = Double::equals; 

it's not clear to me which variant will be selected ?

regards,
Rémi

- Mail original -
> De: some-java-user-99206970363698485...@vodafonemail.de
> À: "core-libs-dev" 
> Envoyé: Samedi 5 Janvier 2019 00:11:09
> Objet: Feature suggestion: Add static equals methods to Float and Double

> To test whether primitive float or double values are equal according to
> `Float.equals` and `Double.equals` you either have to create wrapper instances
> for them (possible performance decrease), use the respective static 
> `compareTo`
> (verbose) or have to use the appropriate methods (`floatToIntBits` /
> `doubleToLongBits`) (verbose and error-prone since you could confuse them with
> the other conversion methods).
> 
> It would be good to provide static methods for testing for equality of the
> primitive values:
> 
> // In Float.java
> 
> public static boolean equals(float a, float b) {
> return Float.floatToIntBits(a) == Float.floatToIntBits(b);
> }
> 
> // In Double.java
> 
> public static boolean equals(double a, double b) {
> return Double.doubleToLongBits(a) == Double.doubleToLongBits(b);
> }
> 
> This would be very convenient for developers and prevent them from writing
> (possibly faulty) code for this themselves.

Re: Feature suggestion: Add static equals methods to Float and Double

[some-java-user-99206970363698485155]

Hello Remi,
You are right, the proposed method names would prevent backwards compatibility, 
I forgot to think 
about that. Sorry for the trouble.
Maybe a different, currently not used name, such as `areEqual`, 
`primitiveEquals` or similar would 
be better.
What do You think about the addition of such a method?

Kind regards

> Remi Forax  hat am 5. Januar 2019 um 00:50 geschrieben:
> 
> 
> Hi,
> it's not obvious to me that this is a source backward compatible change.
> 
> if you have something like:
>   interface Fun { boolean eq(Double a, double b); }
>   ...
>   Fun fun = Double::equals; 
> 
> it's not clear to me which variant will be selected ?
> 
> regards,
> Rémi