Re: Floating Point and Arithmetic Changes for Java SE Lnguage

2018-03-02 Thread Bernd Eckenfels
Hello Anon,

It is impossible to Change the Java behavior for already existing Features like 
the simple Floating Point types. I would not expect much in this area. There 
was a JSR to add a new mode, but it seems to be abandoned: 
https://jcp.org/en/jsr/detail?id=84

I think there was some discussion on Underflow/Overflow handlers, cant remeber 
where I have seen it. It seems to be not on the current JEP list: 
http://openjdk.java.net/jeps/0


BTW: please reconsider your communication strategy (especially on Mailing 
lists). Marking mails as urgent should only be done if you are sure they are 
urgent to all (thousand) receivers, not only to you.

Gruss
Bernd
-- 
http://bernd.eckenfels.net

Von: A Z
Gesendet: Samstag, 3. März 2018 04:25
An: core-libs-dev@openjdk.java.net
Betreff: Floating Point and Arithmetic Changes for Java SE Lnguage
Wichtigkeit: Hoch

I have found that double and float types in java are heirs to arithmetic 
underflow and overflow at any use.

I have found that presently, floating point is an arithmetic approximation.  My 
problem is that

the java language needs to be changed here, so that one may have arithmetic 
accuracy with

floats and doubles.



There is also a trigonometric shortfall when it comes to BigDecimal.



I have attempted to, and have more carefully described these problems, via the 
java bugs database:



JDK-8190947

JDK-8197995

JDK-8190991

JDK-8190946



-These types, as things are, must be computationally discarded, used only in 
terms of push and pull,

and be programmed around using BigDecimal, which will be a waste of memory,

program execution speed, and a total confusion due to the lack of any operator

usage options on BigDecimals.



-It is the case that set, get methods, constructors and mutability methods are 
all based

around float and double, not BigDecimal, which is part of the previous problem.



-It is the case that setting up BigDecimals can be and is a circumstantial 
waste of memory

with very many tasks, combined with the fact that the fact that having to use 
BigDecimal

method calls is nowhere near as efficient or legible to developers or 
mathematics and enginner

programmers (and useful with their time) as



+, -, *, /, %, +=,-+,*=,/=,%=, ++, --



.This is a syntax argument largely, but also an instruction argument

since BigDecimals have to be set up or used with an extra, thereby second, call.



-It is the case that every other major language includes both floating point 
and accuracy mode

options with these two types and or Objects, either as a source code 
instruction or as a

compiler switch option.  These languages at least provide both options for 
floating

point and mathematical accuracy mode.



-It is so that the present arithmetic underflow and underflow are total 
disadvantages,

that need only be changed in place, for preconditions and postconditions.

This is in regard to programs that have been compiled and built already.



-It is also the case that the state of Java floating point for the last 10 
years or so is not really

any argument, given that things can be changed in place remaining reverse 
compatible,

along with how clear and necessary the problem is.



-My Oracle Support technical reviews seem not to recognise these real problems 
whatsoever, or

only interpret matters in terms of the Java Language Specification on these 
issues,

which of itself possesses inadequecies in these places.



Can someone please reply to me on these things?

Can Oracle update the Java SE language?




Re: RFR 8198970: jnu_util.c compilation error on Solaris

2018-03-02 Thread David Holmes

On 3/03/2018 8:56 AM, Roger Riggs wrote:
Please review a correction to the jni_util.c native code that does not 
compile on Solaris.

Declarations must precede assignments.


Wow! I didn't think Solaris Studio compiler was subject to such 
anachronisms! We must be compiling in a really old mode. I'm pretty darn 
certain we're not limited this way when compiling hotspot ..


David

Issue: 8198970 jnu_util.c compilation error on Solaris 



diff --git a/src/java.base/share/native/libjava/jni_util.c 
b/src/java.base/share/native/libjava/jni_util.c

--- a/src/java.base/share/native/libjava/jni_util.c
+++ b/src/java.base/share/native/libjava/jni_util.c
@@ -803,10 +803,10 @@ InitializeEncoding(JNIEnv *env, const ch
  (strcmp(encname, "ISO-8859-1") == 0)) {
  fastEncoding = FAST_8859_1;
  } else if (strcmp(encname, "UTF-8") == 0) {
-    fastEncoding = FAST_UTF_8;
  jstring enc = (*env)->NewStringUTF(env, encname);
  if (enc == NULL)
  return;
+    fastEncoding = FAST_UTF_8;
  jnuEncoding = (jstring)(*env)->NewGlobalRef(env, enc);
  (*env)->DeleteLocalRef(env, enc);
  } else if (strcmp(encname, "ISO646-US") == 0) {
@@ -818,10 +818,10 @@ InitializeEncoding(JNIEnv *env, const ch
  strcmp(encname, "utf-16le") == 0) {
  fastEncoding = FAST_CP1252;
  } else {
-    fastEncoding = NO_FAST_ENCODING;
  jstring enc = (*env)->NewStringUTF(env, encname);
  if (enc == NULL)
  return;
+    fastEncoding = NO_FAST_ENCODING;
  jnuEncoding = (jstring)(*env)->NewGlobalRef(env, enc);
  (*env)->DeleteLocalRef(env, enc);
  }


Thanks, Roger




Re: RFR: JDK-8197594: String#repeat

2018-03-02 Thread Stuart Marks



On 2/28/18 6:34 PM, James Laskey wrote:

Thanks Stuart. RE apinote, I was trying to follow the pattern of other older
method comments (Roman-style.) Comments/Javadoc in most of these older
classes are a mix of styles. Question: if you update/clean-up a method in an
older class, should you bring the comment/Javadoc up-to-date as well?
This is always a tough one. It's like when you're refactoring code... sometimes 
you just want to clean up the next thing, and the next, and the next, until 
you're off in the weeds.


On the one hand, there's the prevailing rule of remaining consistent with other 
stuff in the same file. On the other hand, if there are problems with other 
stuff in the file, it often doesn't make sense to propagate the same error to 
the new stuff you're adding, merely in order to remain consistent.


Sometimes it's reasonable just to include the other cleanups along with the 
current changeset, if they're small enough and prove not to be too much of a 
distraction.


At other times, the cleanups will tend to overwhelm the main goal of this 
changeset. In those cases, I'd suggest proceeding the "right way" for the stuff 
being added, even if it means a hopefully temporary inconsistency with the rest 
of the file. Then do the cleanup and consistency changes in a separate pass.


This requires a separate bug report, but that's not a big deal. Often, if it's 
just javadoc stuff, it won't require regression tests. If the doc changes are 
merely editorial (not specification) it doesn't even require a CSR. So pure 
cleanup changes can proceed relatively quickly.


s'marks


Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

2018-03-02 Thread Martin Buchholz
Thanks - looks good.


On Fri, Mar 2, 2018 at 3:37 PM,  wrote:

> Thanks for comments, Martin, Roger. Updated the fix as follows:
>
> http://cr.openjdk.java.net/~naoto/4993841/webrev.04/
>
> Naoto
>
>
> On 3/1/18 6:47 PM, naoto.s...@oracle.com wrote:
>
>> Hi,
>>
>> Please review the fix to the following issue:
>>
>> https://bugs.openjdk.java.net/browse/JDK-4993841
>>
>> The proposed changeset is located at:
>>
>> http://cr.openjdk.java.net/~naoto/4993841/webrev.03/
>>
>> This stems from the recent discussion regarding String.repeat().[1] The
>> corresponding CSR has already been approved.
>>
>> Naoto
>>
>> --
>> [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-Fe
>> bruary/051568.html
>>
>


Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

2018-03-02 Thread Stuart Marks

Looks good.

I note that other codepoint-consuming methods, such as

Character.UnicodeBlock.of(cp)
Character.UnicodeScript.of(cp)
Character.toChars(cp, char[], int)
Character.toChars(cp)
Character.getName(cp)

all throw IAE with no message. It would be nice to add messages to them. It 
would be even nicer to print out the offending value, possibly even in hex. 
Indeed, there are several other places in Character.java where exceptions are 
thrown that lack diagnostic information. Maybe as part of a separate cleanup pass?


s'marks

On 3/2/18 3:37 PM, naoto.s...@oracle.com wrote:

Thanks for comments, Martin, Roger. Updated the fix as follows:

http://cr.openjdk.java.net/~naoto/4993841/webrev.04/

Naoto

On 3/1/18 6:47 PM, naoto.s...@oracle.com wrote:

Hi,

Please review the fix to the following issue:

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

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/4993841/webrev.03/

This stems from the recent discussion regarding String.repeat().[1] The 
corresponding CSR has already been approved.


Naoto

--
[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html


Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

2018-03-02 Thread Stuart Marks

On 3/2/18 4:42 PM, Remi Forax wrote:

Just to be sure, it now means that a code like this will work

   String s = "hello".chars()
 .mapToObj(Character::toString)
 .collect(Collectors.joining());


Yes, this will work, as Naoto mentioned, but I don't recommend it -- this will 
split up surrogate pairs. Simplying joining them back together will work in this 
case, but if any intermediate processing is done, it could be lossy.


I think the more important case is something like this:

String s = "hello\ud83d\ude1d".codePoints()
.mapToObj(Character::toString)
.collect(joining());

Previously, you had to do something like

String s = "hello\ud83d\ude1d".codePoints()
.mapToObj(cp -> new String(Character.toChars(cp)))
.collect(joining());

which is a mouthful and which also creates an extra array.

s'marks






Rémi

- Mail original -

De: "naoto sato" 
À: "Stuart Marks" , "Xueming Shen" , 
"core-libs-dev"

Envoyé: Vendredi 2 Mars 2018 03:47:51
Objet: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) 
method



Hi,

Please review the fix to the following issue:

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

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/4993841/webrev.03/

This stems from the recent discussion regarding String.repeat().[1] The
corresponding CSR has already been approved.

Naoto

--
[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html


Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

2018-03-02 Thread Jonathan Bluett-Duncan
Hi Rémi,

I'm sure I've misunderstood something, but AFAIK the code example you gave
is already valid code and was valid as far back as Java 8 (I don't have an
IDE at hand to confirm this).

Did you perhaps mean to ask instead that the code snippet below would work
with the resolution of
https://bugs.openjdk.java.net/browse/JDK-4993841 (difference
being that it uses .codePoints() instead of .chars())? Or have I completely
lost the plot?

String s = "hello".codePoints()
.mapToObj(Character::toString)
.collect(Collectors.joining());

Cheers,
Jonathan

On 3 March 2018 at 00:42, Remi Forax  wrote:

> Just to be sure, it now means that a code like this will work
>
>   String s = "hello".chars()
> .mapToObj(Character::toString)
> .collect(Collectors.joining());
>
> Rémi
>
> - Mail original -
> > De: "naoto sato" 
> > À: "Stuart Marks" , "Xueming Shen" <
> xueming.s...@gmail.com>, "core-libs-dev"
> > 
> > Envoyé: Vendredi 2 Mars 2018 03:47:51
> > Objet: [11] RFR: 4993841: (str) java.lang.Character should have a
> toString(int) method
>
> > Hi,
> >
> > Please review the fix to the following issue:
> >
> > https://bugs.openjdk.java.net/browse/JDK-4993841
> >
> > The proposed changeset is located at:
> >
> > http://cr.openjdk.java.net/~naoto/4993841/webrev.03/
> >
> > This stems from the recent discussion regarding String.repeat().[1] The
> > corresponding CSR has already been approved.
> >
> > Naoto
> >
> > --
> > [1]
> > http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-
> February/051568.html
>


Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

2018-03-02 Thread Xueming Shen

+1

On 3/2/18, 3:37 PM, naoto.s...@oracle.com wrote:

Thanks for comments, Martin, Roger. Updated the fix as follows:

http://cr.openjdk.java.net/~naoto/4993841/webrev.04/

Naoto

On 3/1/18 6:47 PM, naoto.s...@oracle.com wrote:

Hi,

Please review the fix to the following issue:

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

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/4993841/webrev.03/

This stems from the recent discussion regarding String.repeat().[1] 
The corresponding CSR has already been approved.


Naoto

--
[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html 





Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

2018-03-02 Thread Naoto Sato

Yes. With a private build:

jshell> 
"hello".chars().mapToObj(Character::toString).collect(Collectors.joining())

$1 ==> "hello"

Naoto

On 03/02/2018 04:42 PM, Remi Forax wrote:

Just to be sure, it now means that a code like this will work

  String s = "hello".chars()
.mapToObj(Character::toString)
.collect(Collectors.joining());

Rémi

- Mail original -

De: "naoto sato" 
À: "Stuart Marks" , "Xueming Shen" , 
"core-libs-dev"

Envoyé: Vendredi 2 Mars 2018 03:47:51
Objet: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) 
method



Hi,

Please review the fix to the following issue:

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

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/4993841/webrev.03/

This stems from the recent discussion regarding String.repeat().[1] The
corresponding CSR has already been approved.

Naoto

--
[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html




Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

2018-03-02 Thread Remi Forax
Just to be sure, it now means that a code like this will work

  String s = "hello".chars()
.mapToObj(Character::toString)
.collect(Collectors.joining());

Rémi

- Mail original -
> De: "naoto sato" 
> À: "Stuart Marks" , "Xueming Shen" 
> , "core-libs-dev"
> 
> Envoyé: Vendredi 2 Mars 2018 03:47:51
> Objet: [11] RFR: 4993841: (str) java.lang.Character should have a 
> toString(int) method

> Hi,
> 
> Please review the fix to the following issue:
> 
> https://bugs.openjdk.java.net/browse/JDK-4993841
> 
> The proposed changeset is located at:
> 
> http://cr.openjdk.java.net/~naoto/4993841/webrev.03/
> 
> This stems from the recent discussion regarding String.repeat().[1] The
> corresponding CSR has already been approved.
> 
> Naoto
> 
> --
> [1]
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html


Floating Point and Arithmetic Changes for Java SE Lnguage

2018-03-02 Thread A Z
I have found that double and float types in java are heirs to arithmetic 
underflow and overflow at any use.

I have found that presently, floating point is an arithmetic approximation.  My 
problem is that

the java language needs to be changed here, so that one may have arithmetic 
accuracy with

floats and doubles.



There is also a trigonometric shortfall when it comes to BigDecimal.



I have attempted to, and have more carefully described these problems, via the 
java bugs database:



JDK-8190947

JDK-8197995

JDK-8190991

JDK-8190946



-These types, as things are, must be computationally discarded, used only in 
terms of push and pull,

and be programmed around using BigDecimal, which will be a waste of memory,

program execution speed, and a total confusion due to the lack of any operator

usage options on BigDecimals.



-It is the case that set, get methods, constructors and mutability methods are 
all based

around float and double, not BigDecimal, which is part of the previous problem.



-It is the case that setting up BigDecimals can be and is a circumstantial 
waste of memory

with very many tasks, combined with the fact that the fact that having to use 
BigDecimal

method calls is nowhere near as efficient or legible to developers or 
mathematics and enginner

programmers (and useful with their time) as



+, -, *, /, %, +=,-+,*=,/=,%=, ++, --



.This is a syntax argument largely, but also an instruction argument

since BigDecimals have to be set up or used with an extra, thereby second, call.



-It is the case that every other major language includes both floating point 
and accuracy mode

options with these two types and or Objects, either as a source code 
instruction or as a

compiler switch option.  These languages at least provide both options for 
floating

point and mathematical accuracy mode.



-It is so that the present arithmetic underflow and underflow are total 
disadvantages,

that need only be changed in place, for preconditions and postconditions.

This is in regard to programs that have been compiled and built already.



-It is also the case that the state of Java floating point for the last 10 
years or so is not really

any argument, given that things can be changed in place remaining reverse 
compatible,

along with how clear and necessary the problem is.



-My Oracle Support technical reviews seem not to recognise these real problems 
whatsoever, or

only interpret matters in terms of the Java Language Specification on these 
issues,

which of itself possesses inadequecies in these places.



Can someone please reply to me on these things?

Can Oracle update the Java SE language?



Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

2018-03-02 Thread naoto . sato

Thanks for comments, Martin, Roger. Updated the fix as follows:

http://cr.openjdk.java.net/~naoto/4993841/webrev.04/

Naoto

On 3/1/18 6:47 PM, naoto.s...@oracle.com wrote:

Hi,

Please review the fix to the following issue:

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

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/4993841/webrev.03/

This stems from the recent discussion regarding String.repeat().[1] The 
corresponding CSR has already been approved.


Naoto

--
[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html 



Re: RFR 8198970: jnu_util.c compilation error on Solaris

2018-03-02 Thread Roger Riggs

Thanks, Mandy, Claes,

On 3/2/2018 6:02 PM, mandy chung wrote:

+1

Mandy

On 3/2/18 2:56 PM, Roger Riggs wrote:
Please review a correction to the jni_util.c native code that does 
not compile on Solaris.

Declarations must precede assignments.

Issue: 8198970 jnu_util.c compilation error on Solaris 



diff --git a/src/java.base/share/native/libjava/jni_util.c 
b/src/java.base/share/native/libjava/jni_util.c

--- a/src/java.base/share/native/libjava/jni_util.c
+++ b/src/java.base/share/native/libjava/jni_util.c
@@ -803,10 +803,10 @@ InitializeEncoding(JNIEnv *env, const ch
 (strcmp(encname, "ISO-8859-1") == 0)) {
 fastEncoding = FAST_8859_1;
 } else if (strcmp(encname, "UTF-8") == 0) {
-    fastEncoding = FAST_UTF_8;
 jstring enc = (*env)->NewStringUTF(env, encname);
 if (enc == NULL)
 return;
+    fastEncoding = FAST_UTF_8;
 jnuEncoding = (jstring)(*env)->NewGlobalRef(env, enc);
 (*env)->DeleteLocalRef(env, enc);
 } else if (strcmp(encname, "ISO646-US") == 0) {
@@ -818,10 +818,10 @@ InitializeEncoding(JNIEnv *env, const ch
 strcmp(encname, "utf-16le") == 0) {
 fastEncoding = FAST_CP1252;
 } else {
-    fastEncoding = NO_FAST_ENCODING;
 jstring enc = (*env)->NewStringUTF(env, encname);
 if (enc == NULL)
 return;
+    fastEncoding = NO_FAST_ENCODING;
 jnuEncoding = (jstring)(*env)->NewGlobalRef(env, enc);
 (*env)->DeleteLocalRef(env, enc);
 }


Thanks, Roger








Re: RFR 8198970: jnu_util.c compilation error on Solaris

2018-03-02 Thread Claes Redestad

Looks ok to me.

/Claes

On 2018-03-02 23:56, Roger Riggs wrote:
Please review a correction to the jni_util.c native code that does not 
compile on Solaris.

Declarations must precede assignments.

Issue: 8198970 jnu_util.c compilation error on Solaris 



diff --git a/src/java.base/share/native/libjava/jni_util.c 
b/src/java.base/share/native/libjava/jni_util.c

--- a/src/java.base/share/native/libjava/jni_util.c
+++ b/src/java.base/share/native/libjava/jni_util.c
@@ -803,10 +803,10 @@ InitializeEncoding(JNIEnv *env, const ch
 (strcmp(encname, "ISO-8859-1") == 0)) {
 fastEncoding = FAST_8859_1;
 } else if (strcmp(encname, "UTF-8") == 0) {
-    fastEncoding = FAST_UTF_8;
 jstring enc = (*env)->NewStringUTF(env, encname);
 if (enc == NULL)
 return;
+    fastEncoding = FAST_UTF_8;
 jnuEncoding = (jstring)(*env)->NewGlobalRef(env, enc);
 (*env)->DeleteLocalRef(env, enc);
 } else if (strcmp(encname, "ISO646-US") == 0) {
@@ -818,10 +818,10 @@ InitializeEncoding(JNIEnv *env, const ch
 strcmp(encname, "utf-16le") == 0) {
 fastEncoding = FAST_CP1252;
 } else {
-    fastEncoding = NO_FAST_ENCODING;
 jstring enc = (*env)->NewStringUTF(env, encname);
 if (enc == NULL)
 return;
+    fastEncoding = NO_FAST_ENCODING;
 jnuEncoding = (jstring)(*env)->NewGlobalRef(env, enc);
 (*env)->DeleteLocalRef(env, enc);
 }


Thanks, Roger






Re: RFR 8198970: jnu_util.c compilation error on Solaris

2018-03-02 Thread mandy chung

+1

Mandy

On 3/2/18 2:56 PM, Roger Riggs wrote:
Please review a correction to the jni_util.c native code that does not 
compile on Solaris.

Declarations must precede assignments.

Issue: 8198970 jnu_util.c compilation error on Solaris 



diff --git a/src/java.base/share/native/libjava/jni_util.c 
b/src/java.base/share/native/libjava/jni_util.c

--- a/src/java.base/share/native/libjava/jni_util.c
+++ b/src/java.base/share/native/libjava/jni_util.c
@@ -803,10 +803,10 @@ InitializeEncoding(JNIEnv *env, const ch
 (strcmp(encname, "ISO-8859-1") == 0)) {
 fastEncoding = FAST_8859_1;
 } else if (strcmp(encname, "UTF-8") == 0) {
-    fastEncoding = FAST_UTF_8;
 jstring enc = (*env)->NewStringUTF(env, encname);
 if (enc == NULL)
 return;
+    fastEncoding = FAST_UTF_8;
 jnuEncoding = (jstring)(*env)->NewGlobalRef(env, enc);
 (*env)->DeleteLocalRef(env, enc);
 } else if (strcmp(encname, "ISO646-US") == 0) {
@@ -818,10 +818,10 @@ InitializeEncoding(JNIEnv *env, const ch
 strcmp(encname, "utf-16le") == 0) {
 fastEncoding = FAST_CP1252;
 } else {
-    fastEncoding = NO_FAST_ENCODING;
 jstring enc = (*env)->NewStringUTF(env, encname);
 if (enc == NULL)
 return;
+    fastEncoding = NO_FAST_ENCODING;
 jnuEncoding = (jstring)(*env)->NewGlobalRef(env, enc);
 (*env)->DeleteLocalRef(env, enc);
 }


Thanks, Roger






Re: RFR 8198697: Simplify platform encoding initialization tweak

2018-03-02 Thread Claes Redestad



On 2018-03-02 20:21, Roger Riggs wrote:
I reordered as the suggested the checks for encoding type to put 
utf-8, most common first

in JNU_NewStringPlatform() and JNU_GetStringPlatformChars().

Webrev updated in place:
http://cr.openjdk.java.net/~rriggs/webrev-simplify-jnu-8198697/


Thanks, looks good to me!

/Claes


RFR 8198970: jnu_util.c compilation error on Solaris

2018-03-02 Thread Roger Riggs
Please review a correction to the jni_util.c native code that does not 
compile on Solaris.

Declarations must precede assignments.

Issue: 8198970 jnu_util.c compilation error on Solaris 



diff --git a/src/java.base/share/native/libjava/jni_util.c 
b/src/java.base/share/native/libjava/jni_util.c

--- a/src/java.base/share/native/libjava/jni_util.c
+++ b/src/java.base/share/native/libjava/jni_util.c
@@ -803,10 +803,10 @@ InitializeEncoding(JNIEnv *env, const ch
 (strcmp(encname, "ISO-8859-1") == 0)) {
 fastEncoding = FAST_8859_1;
 } else if (strcmp(encname, "UTF-8") == 0) {
-    fastEncoding = FAST_UTF_8;
 jstring enc = (*env)->NewStringUTF(env, encname);
 if (enc == NULL)
 return;
+    fastEncoding = FAST_UTF_8;
 jnuEncoding = (jstring)(*env)->NewGlobalRef(env, enc);
 (*env)->DeleteLocalRef(env, enc);
 } else if (strcmp(encname, "ISO646-US") == 0) {
@@ -818,10 +818,10 @@ InitializeEncoding(JNIEnv *env, const ch
 strcmp(encname, "utf-16le") == 0) {
 fastEncoding = FAST_CP1252;
 } else {
-    fastEncoding = NO_FAST_ENCODING;
 jstring enc = (*env)->NewStringUTF(env, encname);
 if (enc == NULL)
 return;
+    fastEncoding = NO_FAST_ENCODING;
 jnuEncoding = (jstring)(*env)->NewGlobalRef(env, enc);
 (*env)->DeleteLocalRef(env, enc);
 }


Thanks, Roger




Re: RFR JDK-8187653: Lock in CoderResult.Cache becomes performance bottleneck

2018-03-02 Thread Claes Redestad

One less (benign) race - and possibly more efficient, too :-)

If we really worry about the startup costs here, we could make it so 
that the three Cache classes

themselves aren't loaded until someone actually has a need for them:

http://cr.openjdk.java.net/~redestad/scratch/coderresult_cache.00/

/Claes

On 2018-03-02 21:02, Xueming Shen wrote:


To follow Claes's suggestion to make the CoderResult.Cache.cache field 
final and allocate early.


issue: https://bugs.openjdk.java.net/browse/JDK-8198966
webrev: http://cr.openjdk.java.net/~sherman/8198966/webrev/

Thanks,
-Sherman




Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

2018-03-02 Thread David Lloyd
On Fri, Mar 2, 2018 at 2:34 PM, David Lloyd  wrote:
> On Fri, Mar 2, 2018 at 12:49 PM, Xueming Shen  wrote:
>> Hi David,
>>
>> (1) Deflater.deflate(Bytebuffer)
>>  the api doc regarding "no_flush" appears to be the copy/paste of the
>> byte[] version
>>  without being updated to the corresponding ByteBuffer?
>
> You're right, I missed that one.  I've incorporated this fix locally:

Oops, this should have been:

--- 8< --- cut here --- 8< ---

diff --git a/src/java.base/share/classes/java/util/zip/Deflater.java
b/src/java.base/share/classes/java/util/zip/Deflater.java
index 524125787a8..40f0d9736e2 100644
--- a/src/java.base/share/classes/java/util/zip/Deflater.java
+++ b/src/java.base/share/classes/java/util/zip/Deflater.java
@@ -481,9 +481,9 @@ public class Deflater {
  * in order to determine if more input data is required.
  *
  * This method uses {@link #NO_FLUSH} as its compression flush mode.
- * An invocation of this method of the form {@code deflater.deflate(b)}
+ * An invocation of this method of the form {@code
deflater.deflate(output)}
  * yields the same result as the invocation of
- * {@code deflater.deflate(b, 0, b.length, Deflater.NO_FLUSH)}.
+ * {@code deflater.deflate(output, Deflater.NO_FLUSH)}.
  *
  * @param output the buffer for the compressed data
  * @return the actual number of bytes of compressed data written to the

--- 8< --- cut here --- 8< ---

-- 
- DML


Re: RFR: JDK-8198955 - String#repeat loop optimization

2018-03-02 Thread Ivan Gerasimov

This looks good, thank you!

On 3/2/18 6:43 AM, Jim Laskey wrote:

Verified to work on edge cases.

JBS: https://bugs.openjdk.java.net/browse/JDK-8198955 

Webrev: http://cr.openjdk.java.net/~jlaskey/8198955/webrev/index.html 



--
With kind regards,
Ivan Gerasimov



Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

2018-03-02 Thread David Lloyd
On Fri, Mar 2, 2018 at 12:49 PM, Xueming Shen  wrote:
> Hi David,
>
> (1) Deflater.deflate(Bytebuffer)
>  the api doc regarding "no_flush" appears to be the copy/paste of the
> byte[] version
>  without being updated to the corresponding ByteBuffer?

You're right, I missed that one.  I've incorporated this fix locally:

--- 8< --- cut here --- 8< ---

diff --git a/src/java.base/share/classes/java/util/zip/Deflater.java
b/src/java.base/share/classes/java/util/zip/Deflater.java
index 524125787a8..9cf735a9477 100644
--- a/src/java.base/share/classes/java/util/zip/Deflater.java
+++ b/src/java.base/share/classes/java/util/zip/Deflater.java
@@ -483,7 +483,7 @@ public class Deflater {
  * This method uses {@link #NO_FLUSH} as its compression flush mode.
  * An invocation of this method of the form {@code deflater.deflate(b)}
  * yields the same result as the invocation of
- * {@code deflater.deflate(b, 0, b.length, Deflater.NO_FLUSH)}.
+ * {@code deflater.deflate(output, Deflater.NO_FLUSH)}.
  *
  * @param output the buffer for the compressed data
  * @return the actual number of bytes of compressed data written to the

--- 8< --- cut here --- 8< ---

I'll post an amended patch once we work out the other points below
(and any other feedback that comes in the meantime).

> (2) Inflater.inflate()
>  a "inputConsumed" field is added to handle the "bytes-read" if a DFE is
> being thrown.
>
>  While the API doc specifies that "if the setInput(bytebuffer) method
> was call "
>  it appears the inputPos/bytesRead are being updated for the
> "setInput(byte[]) case
>  as well. This might be a desirable change, but is an "incompatible"
> behavior change
>  as well. I doubt if there is any existing app depending on this
> existing behavior though,
>  it's really hard, if not impossible, to try to recover from this kind
> of error

I agree; this was implemented based on Alan's points but I could go
either way with it.

I had a thought that I could update the position only in the buffer
input case, not the array input case, but this would still have the
effect of changing the behavior of Inflater.getRemaining() when the
input is a buffer, which is still technically an incompatibility.  So
the three options are:

* Always update the input position on exception
* Only update the input position on exception when the input is a ByteBuffer
* Never update the input position on exception

AFAICT there are no inherent technical issues with any of these
approaches beyond the obscure compatibility change.  Since the
behavior was previously unspecified, I think that decreases the
compatibility risk even further, so I'm inclined to think that #1 is
the best overall choice.

>  There might also bring in a little inconsistency, as we are updating
> the "input" in this
>  case,  but why not the "output" buffer? It's true there is no way to do
> that with the
>  inflate(byte[] ...), it's kinda doable for the inflate(ByteBuffer) and
> in fact there might
>  be bytes that have been written into the output ByteBuffer in this
> case.

Since there is no way to get the output bytes consumed in the array
case, this is not an incompatible change, so I am happy to include it
in my next revision.  However I think it only makes sense to do this
if it is consistent with input; i.e. if option #3 above is chosen then
it would be inconsistent to update the output position on exception
and we should not do it.

> (3) there are usages like
>  Math.max(buf.limit() - outputPos, 0);
>  just wonder if there is any reason not using existing api method.
>  Math.max(buf.remaining(), 0);

This is because the buffer's position can change between the call to
buf.position() and buf.remaining(), which can have the ultimate effect
of allowing invalid memory accesses.  By acquiring the position and
limit each one time, I can guarantee some consistent view of these
properties.  This is consistent with (for example) sun.nio.ch.IOUtil.

Also just as a general principle, buf.remaining() reads both the limit
and position, and since I already have the position, it makes sense to
just read the limit which is the only other piece of data that must be
read in order to correctly calculate these values.

Thanks for the feedback.


-- 
- DML


Re: RFR JDK-8187653: Lock in CoderResult.Cache becomes performance bottleneck

2018-03-02 Thread Xueming Shen


To follow Claes's suggestion to make the CoderResult.Cache.cache field 
final and allocate early.


issue: https://bugs.openjdk.java.net/browse/JDK-8198966
webrev: http://cr.openjdk.java.net/~sherman/8198966/webrev/

Thanks,
-Sherman


Re: RFR 8198697: Simplify platform encoding initialization tweak

2018-03-02 Thread Roger Riggs

Hi Claes, Sherman,

Thanks for the reviews:

I reordered as the suggested the checks for encoding type to put utf-8, 
most common first

in JNU_NewStringPlatform() and JNU_GetStringPlatformChars().

Webrev updated in place:
  http://cr.openjdk.java.net/~rriggs/webrev-simplify-jnu-8198697/

Thanks, Roger

On 3/2/2018 12:03 PM, Xueming Shen wrote:

+1

On 03/02/2018 07:49 AM, Roger Riggs wrote:

Please review...

On 2/28/2018 9:45 AM, Roger Riggs wrote:

Hi,

In an effort to untangle some of the issues with property 
initialization I was looking

at the platform encoding initialization and found a simplification.

Currently, the initialization occurs as a side effect of the first 
call to JNU_NewStringPlatform and
involves a upcall to get sun.jnu.encoding from the system 
properties.  The value is cached for later use.


The native System.initProperties determines the platform specific 
encoding via java_props_md.c and does an upcall to set the 
sun.jnu.encoding system property, taking care to do it before the 
first string that needs platform encoding.


The change directly initializes the platform encoding fast path 
before it is needed to encode platform strings.
And moves the setting sun.jnu.encoding system property after the 
command line arguments are inserted
keeping it from being overridden by -D on the command line that can 
only confuse confusion

with code that later reads the property.

Please review and comment.

webrev:
http://cr.openjdk.java.net/~rriggs/webrev-simplify-jnu-8198697/

Issue:
   https://bugs.openjdk.java.net/browse/JDK-8198697

Thanks, Roger












Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

2018-03-02 Thread Xueming Shen

Hi David,

(1) Deflater.deflate(Bytebuffer)
 the api doc regarding "no_flush" appears to be the copy/paste of the 
byte[] version
 without being updated to the corresponding ByteBuffer?

(2) Inflater.inflate()
 a "inputConsumed" field is added to handle the "bytes-read" if a DFE is 
being thrown.

 While the API doc specifies that "if the setInput(bytebuffer) method was call 
"
 it appears the inputPos/bytesRead are being updated for the 
"setInput(byte[]) case
 as well. This might be a desirable change, but is an "incompatible" 
behavior change
 as well. I doubt if there is any existing app depending on this existing 
behavior though,
 it's really hard, if not impossible, to try to recover from this kind of 
error

 There might also bring in a little inconsistency, as we are updating the 
"input" in this
 case,  but why not the "output" buffer? It's true there is no way to do 
that with the
 inflate(byte[] ...), it's kinda doable for the inflate(ByteBuffer) and in 
fact there might
 be bytes that have been written into the output ByteBuffer in this case.

(3) there are usages like
 Math.max(buf.limit() - outputPos, 0);
 just wonder if there is any reason not using existing api method.
 Math.max(buf.remaining(), 0);

sherman

On 03/02/2018 08:07 AM, David Lloyd wrote:

The third... and hopefully final... version of this patch is attached.
It is the same as V2, however it also uses
Reference.reachabilityFence() defensively on buffers.  This may be
necessary because there are code paths which may allow such buffers to
be GCed after their address is acquired but before the native code
successfully is able to read it.

The online version of this patch is visible at [1].

[1] https://github.com/dmlloyd/openjdk/commit/zlib-bytebuffer-v8





Re: RFR: Here are some URLClassPath patches

2018-03-02 Thread Martin Buchholz
Thanks!  Delivered without further changes.

That should be the end of this batch of classloader changes.

On Fri, Mar 2, 2018 at 7:00 AM, Roger Riggs  wrote:

> Hi Martin,
>
> I created the subtask for the release note[1], please review and update
> the summary and description.
> When it is done, close it as 'delivered'.
>
> Thanks, Roger
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8198956
>
>
>
> On 3/1/2018 10:32 PM, Martin Buchholz wrote:
>
>> On Wed, Feb 28, 2018 at 3:58 PM, Martin Buchholz 
>> wrote:
>>
>>
>>> 8198810: URLClassLoader does not specify behavior when URL array contains
>>> null
>>> http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassLoader-NPE/
>>> https://bugs.openjdk.java.net/browse/JDK-8198810
>>>
>>>   Pushed. Can someone at Oracle help with the release note?
>>
>
>


Re: [PATCH] Reduce Chance Of Mistakenly Early Backing Memory Cleanup

2018-03-02 Thread Paul Sandoz
Thanks!
Paul.

> On Mar 2, 2018, at 9:11 AM, Vladimir Ivanov  
> wrote:
> 
> 
> 
> On 3/2/18 8:01 PM, Paul Sandoz wrote:
>> Here’s an update Ben and I tweaked:
>>   
>> http://cr.openjdk.java.net/~psandoz/jdk/buffer-reachability-fence/webrev/index.html
>> I think this looks good but would still like to double check with Vladimir 
>> that the @ForceInline is not problematic.
> 
> I confirm that my previous analysis [1] still applies when method is marked 
> w/ @ForceInline.
> 
> Best regards,
> Vladimir Ivanov
> 
> [1] 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051312.html
> 
>>> On Feb 26, 2018, at 6:50 PM, Paul Sandoz  wrote:
>>> 
>>> Hi Ben,
>>> 
>>> Here is the webrev online:
>>> 
>>>  
>>> http://cr.openjdk.java.net/~psandoz/jdk/buffer-reachability-fence/webrev/index.html
>>> 



Re: [PATCH] Reduce Chance Of Mistakenly Early Backing Memory Cleanup

2018-03-02 Thread Vladimir Ivanov



On 3/2/18 8:01 PM, Paul Sandoz wrote:

Here’s an update Ben and I tweaked:

   
http://cr.openjdk.java.net/~psandoz/jdk/buffer-reachability-fence/webrev/index.html

I think this looks good but would still like to double check with Vladimir that 
the @ForceInline is not problematic.


I confirm that my previous analysis [1] still applies when method is 
marked w/ @ForceInline.


Best regards,
Vladimir Ivanov

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051312.html



On Feb 26, 2018, at 6:50 PM, Paul Sandoz  wrote:

Hi Ben,

Here is the webrev online:

  
http://cr.openjdk.java.net/~psandoz/jdk/buffer-reachability-fence/webrev/index.html






Re: RFR 8198697: Simplify platform encoding initialization tweak

2018-03-02 Thread Xueming Shen

+1

On 03/02/2018 07:49 AM, Roger Riggs wrote:

Please review...

On 2/28/2018 9:45 AM, Roger Riggs wrote:

Hi,

In an effort to untangle some of the issues with property initialization I was 
looking
at the platform encoding initialization and found a simplification.

Currently, the initialization occurs as a side effect of the first call to 
JNU_NewStringPlatform and
involves a upcall to get sun.jnu.encoding from the system properties.  The 
value is cached for later use.

The native System.initProperties determines the platform specific encoding via 
java_props_md.c and does an upcall to set the sun.jnu.encoding system property, 
taking care to do it before the first string that needs platform encoding.

The change directly initializes the platform encoding fast path before it is 
needed to encode platform strings.
And moves the setting sun.jnu.encoding system property after the command line 
arguments are inserted
keeping it from being overridden by -D on the command line that can only 
confuse confusion
with code that later reads the property.

Please review and comment.

webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-simplify-jnu-8198697/

Issue:
   https://bugs.openjdk.java.net/browse/JDK-8198697

Thanks, Roger










Re: [PATCH] Reduce Chance Of Mistakenly Early Backing Memory Cleanup

2018-03-02 Thread Paul Sandoz
Here’s an update Ben and I tweaked:

  
http://cr.openjdk.java.net/~psandoz/jdk/buffer-reachability-fence/webrev/index.html

I think this looks good but would still like to double check with Vladimir that 
the @ForceInline is not problematic.

Paul.

> On Feb 26, 2018, at 6:50 PM, Paul Sandoz  wrote:
> 
> Hi Ben,
> 
> Here is the webrev online:
> 
>  
> http://cr.openjdk.java.net/~psandoz/jdk/buffer-reachability-fence/webrev/index.html
> 




Re: RFR: JDK-8198955 - String#repeat loop optimization

2018-03-02 Thread Claes Redestad

+1

On 2018-03-02 15:43, Jim Laskey wrote:

Verified to work on edge cases.

JBS: https://bugs.openjdk.java.net/browse/JDK-8198955 

Webrev: http://cr.openjdk.java.net/~jlaskey/8198955/webrev/index.html 





Re: RFR 8198697: Simplify platform encoding initialization tweak

2018-03-02 Thread Claes Redestad

Hi,

looks good to me, although part of me can't help wonder if we'd profit 
ever so little from rearranging if statements so that we test for UTF8 
first and move the should-be-asserts tests for NO_ENCODING_YET down.


/Claes

On 2018-02-28 15:45, Roger Riggs wrote:

Hi,

In an effort to untangle some of the issues with property 
initialization I was looking

at the platform encoding initialization and found a simplification.

Currently, the initialization occurs as a side effect of the first 
call to JNU_NewStringPlatform and
involves a upcall to get sun.jnu.encoding from the system properties.  
The value is cached for later use.


The native System.initProperties determines the platform specific 
encoding via java_props_md.c and does an upcall to set the 
sun.jnu.encoding system property, taking care to do it before the 
first string that needs platform encoding.


The change directly initializes the platform encoding fast path before 
it is needed to encode platform strings.
And moves the setting sun.jnu.encoding system property after the 
command line arguments are inserted
keeping it from being overridden by -D on the command line that can 
only confuse confusion

with code that later reads the property.

Please review and comment.

webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-simplify-jnu-8198697/

Issue:
   https://bugs.openjdk.java.net/browse/JDK-8198697

Thanks, Roger








Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

2018-03-02 Thread Paul Sandoz


> On Mar 2, 2018, at 8:07 AM, David Lloyd  wrote:
> 
> The third... and hopefully final... version of this patch is attached.
> It is the same as V2, however it also uses
> Reference.reachabilityFence() defensively on buffers.  This may be
> necessary because there are code paths which may allow such buffers to
> be GCed after their address is acquired but before the native code
> successfully is able to read it.
> 
> The online version of this patch is visible at [1].
> 

I have not been tracking this patch closely, but the use of 
Reference.reachabilityFence looks good to me.

Paul.

> [1] https://github.com/dmlloyd/openjdk/commit/zlib-bytebuffer-v8
> 
> -- 
> - DML
> 



[JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

2018-03-02 Thread David Lloyd
The third... and hopefully final... version of this patch is attached.
It is the same as V2, however it also uses
Reference.reachabilityFence() defensively on buffers.  This may be
necessary because there are code paths which may allow such buffers to
be GCed after their address is acquired but before the native code
successfully is able to read it.

The online version of this patch is visible at [1].

[1] https://github.com/dmlloyd/openjdk/commit/zlib-bytebuffer-v8

-- 
- DML
commit 09313d25a1362039806a0c1b2d443874eb101843
Author: David M. Lloyd 
Date:   Fri Feb 16 11:00:10 2018 -0600

[JDK-6341887] Update Inflater/Deflater to handle ByteBuffer

diff --git a/make/mapfiles/libzip/mapfile-vers 
b/make/mapfiles/libzip/mapfile-vers
index d711d8e17f4..11ccc2d6ecb 100644
--- a/make/mapfiles/libzip/mapfile-vers
+++ b/make/mapfiles/libzip/mapfile-vers
@@ -33,20 +33,28 @@ SUNWprivate_1.1 {
Java_java_util_zip_CRC32_update;
Java_java_util_zip_CRC32_updateBytes0;
Java_java_util_zip_CRC32_updateByteBuffer0;
-   Java_java_util_zip_Deflater_deflateBytes;
+   Java_java_util_zip_Deflater_deflateBytesBytes;
+   Java_java_util_zip_Deflater_deflateBytesBuffer;
+   Java_java_util_zip_Deflater_deflateBufferBytes;
+   Java_java_util_zip_Deflater_deflateBufferBuffer;
Java_java_util_zip_Deflater_end;
Java_java_util_zip_Deflater_getAdler;
Java_java_util_zip_Deflater_init;
Java_java_util_zip_Deflater_initIDs;
Java_java_util_zip_Deflater_reset;
Java_java_util_zip_Deflater_setDictionary;
+   Java_java_util_zip_Deflater_setDictionaryBuffer;
Java_java_util_zip_Inflater_end;
Java_java_util_zip_Inflater_getAdler;
-   Java_java_util_zip_Inflater_inflateBytes;
+   Java_java_util_zip_Inflater_inflateBytesBytes;
+   Java_java_util_zip_Inflater_inflateBytesBuffer;
+   Java_java_util_zip_Inflater_inflateBufferBytes;
+   Java_java_util_zip_Inflater_inflateBufferBuffer;
Java_java_util_zip_Inflater_init;
Java_java_util_zip_Inflater_initIDs;
Java_java_util_zip_Inflater_reset;
Java_java_util_zip_Inflater_setDictionary;
+   Java_java_util_zip_Inflater_setDictionaryBuffer;
ZIP_Close;
ZIP_CRC32;
ZIP_FreeEntry;
diff --git a/src/java.base/share/classes/java/util/zip/Deflater.java 
b/src/java.base/share/classes/java/util/zip/Deflater.java
index c75dd4a33f0..524125787a8 100644
--- a/src/java.base/share/classes/java/util/zip/Deflater.java
+++ b/src/java.base/share/classes/java/util/zip/Deflater.java
@@ -26,7 +26,13 @@
 package java.util.zip;
 
 import java.lang.ref.Cleaner.Cleanable;
+import java.lang.ref.Reference;
+import java.nio.ByteBuffer;
+import java.nio.ReadOnlyBufferException;
+import java.util.Objects;
+
 import jdk.internal.ref.CleanerFactory;
+import sun.nio.ch.DirectBuffer;
 
 /**
  * This class provides support for general purpose compression using the
@@ -92,8 +98,9 @@ import jdk.internal.ref.CleanerFactory;
 public class Deflater {
 
 private final DeflaterZStreamRef zsRef;
-private byte[] buf = new byte[0];
-private int off, len;
+private ByteBuffer input = ZipUtils.defaultBuf;
+private byte[] inputArray;
+private int inputPos, inputLim;
 private int level, strategy;
 private boolean setParams;
 private boolean finish, finished;
@@ -170,9 +177,14 @@ public class Deflater {
  */
 public static final int FULL_FLUSH = 3;
 
+/**
+ * Flush mode to use at the end of output.  Can only be provided by the
+ * user by way of {@link #finish()}.
+ */
+private static final int FINISH = 4;
+
 static {
 ZipUtils.loadLibrary();
-initIDs();
 }
 
 /**
@@ -216,16 +228,14 @@ public class Deflater {
  * @see Deflater#needsInput
  */
 public void setInput(byte[] b, int off, int len) {
-if (b== null) {
-throw new NullPointerException();
-}
 if (off < 0 || len < 0 || off > b.length - len) {
 throw new ArrayIndexOutOfBoundsException();
 }
 synchronized (zsRef) {
-this.buf = b;
-this.off = off;
-this.len = len;
+this.input = null;
+this.inputArray = b;
+this.inputPos = off;
+this.inputLim = off + len;
 }
 }
 
@@ -239,6 +249,31 @@ public class Deflater {
 setInput(b, 0, b.length);
 }
 
+/**
+ * Sets input data for compression. This should be called whenever
+ * needsInput() returns true indicating that more input data is required.
+ * 
+ * The given buffer's position will be updated as deflate operations are
+ * 

Re: RFR 8198697: Simplify platform encoding initialization tweak

2018-03-02 Thread Roger Riggs

Please review...

On 2/28/2018 9:45 AM, Roger Riggs wrote:

Hi,

In an effort to untangle some of the issues with property 
initialization I was looking

at the platform encoding initialization and found a simplification.

Currently, the initialization occurs as a side effect of the first 
call to JNU_NewStringPlatform and
involves a upcall to get sun.jnu.encoding from the system properties.  
The value is cached for later use.


The native System.initProperties determines the platform specific 
encoding via java_props_md.c and does an upcall to set the 
sun.jnu.encoding system property, taking care to do it before the 
first string that needs platform encoding.


The change directly initializes the platform encoding fast path before 
it is needed to encode platform strings.
And moves the setting sun.jnu.encoding system property after the 
command line arguments are inserted
keeping it from being overridden by -D on the command line that can 
only confuse confusion

with code that later reads the property.

Please review and comment.

webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-simplify-jnu-8198697/

Issue:
   https://bugs.openjdk.java.net/browse/JDK-8198697

Thanks, Roger








Re: RFR: JDK-8198955 - String#repeat loop optimization

2018-03-02 Thread Roger Riggs

+1

On 3/2/2018 9:43 AM, Jim Laskey wrote:

Verified to work on edge cases.

JBS: https://bugs.openjdk.java.net/browse/JDK-8198955 

Webrev: http://cr.openjdk.java.net/~jlaskey/8198955/webrev/index.html 





Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

2018-03-02 Thread Roger Riggs

Hi Naoto,

String.java:
3129: Please add a message to the throw IAE.  "Not a valid Unicode code 
point".


Roger



On 3/1/2018 9:47 PM, naoto.s...@oracle.com wrote:

Hi,

Please review the fix to the following issue:

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

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/4993841/webrev.03/

This stems from the recent discussion regarding String.repeat().[1] 
The corresponding CSR has already been approved.


Naoto

--
[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html




Re: RFR: Here are some URLClassPath patches

2018-03-02 Thread Roger Riggs

Hi Martin,

I created the subtask for the release note[1], please review and update 
the summary and description.

When it is done, close it as 'delivered'.

Thanks, Roger

[1] https://bugs.openjdk.java.net/browse/JDK-8198956


On 3/1/2018 10:32 PM, Martin Buchholz wrote:

On Wed, Feb 28, 2018 at 3:58 PM, Martin Buchholz 
wrote:



8198810: URLClassLoader does not specify behavior when URL array contains
null
http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassLoader-NPE/
https://bugs.openjdk.java.net/browse/JDK-8198810


  Pushed. Can someone at Oracle help with the release note?




RFR: JDK-8198955 - String#repeat loop optimization

2018-03-02 Thread Jim Laskey
Verified to work on edge cases.

JBS: https://bugs.openjdk.java.net/browse/JDK-8198955 

Webrev: http://cr.openjdk.java.net/~jlaskey/8198955/webrev/index.html 


Re: RFR: JDK-8190187: C++ code calling JNI_CreateJavaVM can be killed by Java

2018-03-02 Thread Adam Farley8
Hi Alan

Thanks for getting back to me on this. :)

I've changed the hg_diff as described below, see the attached.

> On 27/02/2018 15:04, Adam Farley8 wrote:
>
> Resending. Bump. :) 
>
> On 14/02/2018 14:13, Adam Farley8 wrote: 
>>> Hi All, 
>>> 
>>> -- Short version -- 
>>> 
>>> Could a committer please take the fix for JDK-8190187 (full code 
included 
>>> in the bug) and: 
>>> 
>>> 1) Complete the CSR process for the new JNI Return code. 
>>> 2) Commit the changes that contain (a) the new return code, and (b) 
the 
>>> non-Hotspot code that handles the new code. 
>> The patches attached to the JIRA issue are missing the changes to the 
>> JVM TI spec (jvmti.xml). 
>
>> I'm not seeing the JNI return codes in that file. Are you after one of 
those 
>> dated updates near the bottom? 
>> e.g. 
>> ``` 
>>   
>>  Added JNI_SILENT_EXIT return code for early exits without errors. 
>>  java.c's ParseArguments function now sets pret value to 2 for a 
NULL pwhat. 
>> This allows us to clearly identify when no class was set, but 
no other error has occurred. 
>> This is undone in java.c's ContinueInNewThread method, so the 
surface behaviour does not change. 
>>   
>> ```
> The "Agent Start-Up" section is the section to look at. The important 
part is:
>
> "The return value from Agent_OnLoad or Agent_OnLoad_ is 
used to indicate an error. Any value other than zero indicates an error 
and causes termination of the VM."
>
> If there is special return value to mean "VM terminates without error" 
then this part of the spec will need to be adjusted. 

Ah, that makes sense. I altered that bit and regenerated the hg_diff.

> An additional point is that you can start several agents from the 
command line, does the VM terminate after it has started all agents or 
does it terminate when the first agent returns asks the VM to terminate 
quietly?

If I'm reading the code correctly, the loop that initialises the different 
agents 
(which I believe to be the loop containing "// Invoke the Agent_OnLoad 
function")
is not interrupted by the return of a silent exit code, however it only 
takes one 
agent returning this code to cause the VM to be destroyed once startup is
complete.

>
>
>
>
>
>>> There is also text to be written for the JNI spec if this proposal 
goes 
>>> ahead. 
>
>> I assume you mean the "RETURNS" section of the JNI_CreateJavaVM 
>> bit on the invocation.html web page. Something like this? 
>
>> ``` 
>> RETURNS: 
>> Returns JNI_OK on success; returns a suitable JNI error code (a 
negative number) on failure. 
>>
>> The sole exception is a silent exit, which returns JNI error code 
JNI_SILENT_EXIT. 
>> This indicates that the VM cannot be used, but that this is the 
intended behaviour for the 
>> arguments used. E.g. -Xlog:help (which prints help output and then 
destroys the VM) 
>> ```
> Yes, this is the place that will need changes.
>

Ok. The most official-looking place for this documentation is on the 
Oracle website. I'm not sure how to go about making this change
happen though.

Is the change to this documentation something I can push through 
one of the mailing lists? Or is it perhaps part of the CSR process?

>
>
>
>
>>> 
>>> I don't agree that the launcher should be looking for 
>>> "-agentlib:jdwp=help" in the command line as it's just one of many 
ways 
>>> that the debugger agent might be started (e.g. -Xrunjdwp:, 
>>> _JAVA_TOOLS_OPTIONS, ...). 
>
>> We can avoid that by finding a way around this line in 
ContinueInNewThread (java.c): 
>
>> ``` 
>> return (ret != 0) ? ret : rslt; 
>> ``` 
>
>> I have devised a means to do this, as outlined in the jvmti.xml change 
above. I put the 
>> changes into a recent clone of jdk/jdk, and attached the hg diff, along 
with an improved 
>> test. 
>
> The launcher should only need to look at the return value from JNI 
CreateJavaVM. I don't think it should do any special handling for the JDWP 
or other agents (there are just too many ways to inject command lines and 
the launcher cannot be expected to handle all of them).
>
>-Alan
>

I agree. The change I made in response to your earlier comment is not 
jdwp-specific.
Rather, it sets "ret" to "2" if the user has not specified an executable 
class in the 
command line. I did this because a ret value of "1" indicates an error, 
but we don't 
want to override a return code of JNI_SILENT_EXIT with ret's value if the 
only error
was "no class was specified".

So I set ret to "2" if no class is specified, and altered the 
aforementioned bit in
ContinueInNewThread (java.c) to pick up on a ret value of 2. If the ret 
value is 2,
and rslt is JNI_SILENT_EXIT, then we know that no error has occurred 
(other
than "no class was specified"), and that JNI_SILENT_EXIT should be 
returned,
as opposed to the current functionality, where it is the non-0 ret value 
which
is returned (erroneously, in the event of a silent exit).

I think my changeset explains this more concisely than I have. :)
Let me know if 

Re: RFR JDK-8197533 move javax.transaction.xa into its own module

2018-03-02 Thread Alan Bateman

On 28/02/2018 19:54, Lance Andersen wrote:

:


Is there any XA text from the original JTA spec that should be added 
to the module description as part of this? Another way to ask this is 
whether the JTA 1.3 drops any text dealing with the XA part.
Still waiting to see what changes are made to the PDF spec, that is 
still needing to be completed. So I think for now, we go with what we 
have and can circle back if needed.


Thanks. I checked the latest webrev (moving the package description to 
package-info.java and the langtools dot test) and it all looks good to me.


-Alan


Re: RFR JDK-8187653: Lock in CoderResult.Cache becomes performance bottleneck

2018-03-02 Thread Peter Levart

Hi Sherman,

On 03/02/18 03:20, David Holmes wrote:

Also this:

195 private Map cache = null;

should now be volatile.


Either that, or you should load the 'cache' field only once per method 
call into a local variable unless you want reorderings of reads and 
writes observed from concurrent threads to result in NPE-s.


If you do replace it with a volatile, you should also load the field 
into local variable just once (although not strictly necessary for 
correctness).


Also:

 193 private abstract static class Cache {
 197 *protected* abstract CoderResult create(int len);

the abstract method is protected, but the implementations:

 217 = new Cache() {
 218 *public* CoderResult create(int len) {
 219 return new CoderResult(CR_MALFORMED, len);
 220 }};

...are public.


Since you are dealing with private nested class, all create() methods 
could be package-private. Less words to write and read...


Regards, Peter