Re: RFC: Make test failed because of the locale LANG

[David Holmes]

Hi Jing,

On 28/03/2019 3:22 pm, Jing Tian wrote:

Hi,

When I am doing the 'make test'.If the local LANG is set as 
'zh_CN.UTF-8', Test cases will have a lot of error messages.


==
Test summary
==
    TEST  TOTAL  PASS FAIL 
ERROR

 >> jtreg:test/hotspot/jtreg:tier1 1373 1371 2 0 <<
 >> jtreg:test/jdk:tier1   1867 1860 7 0 <<
 >> jtreg:test/langtools:tier1 3922 2470 
1450 2 <<

    jtreg:test/nashorn:tier1  0 0 0 0
    jtreg:test/jaxp:tier1 0 0 0 0
 >> jtreg:test/jdk:tier2   3334 3305 29 
0 <<

 >> jtreg:test/langtools:tier2 11 9 2 0 <<
    jtreg:test/nashorn:tier2 35 35 0 0
 >> jtreg:test/jaxp:tier2   438 437 1 0 <<
 >> jtreg:test/jdk:tier3   1104 1068 36 
0 <<

    jtreg:test/langtools:tier3    0 0 0 0
    jtreg:test/nashorn:tier3  0 0 0 0
    jtreg:test/jaxp:tier3 0 0 0 0
==


Given most of these are not hotspot issues and given the number of 
failures, I think this is something that would need to be discussed much 
more broadly. So I've cc'd core-libs-dev and compiler-dev. I recall 
there was a very recent discussion regarding some tests failing for the 
same reason, but don't recall the outcome.


David
-


On the same machine,when i set LANG=en_US.UTF-8.

==
Test summary
==
    TEST  TOTAL  PASS FAIL 
ERROR

 >> jtreg:test/hotspot/jtreg:tier1 1388 1386 2 0 <<
 >> jtreg:test/jdk:tier1   1867 1843 19 
5 <<

    jtreg:test/langtools:tier1 3920 3920 0 0
    jtreg:test/nashorn:tier1  0 0 0 0
    jtreg:test/jaxp:tier1 0 0 0 0
 >> jtreg:test/jdk:tier2   3328 3290 31 
7 <<

    jtreg:test/langtools:tier2   11 11 0 0
    jtreg:test/nashorn:tier2 35 35 0 0
    jtreg:test/jaxp:tier2   438 438 0 0
 >> jtreg:test/jdk:tier3   1104 1067 37 
0 <<

    jtreg:test/langtools:tier3    0 0 0 0
    jtreg:test/nashorn:tier3  0 0 0 0
    jtreg:test/jaxp:tier3 0 0 0 0


By comparison we can find, lots of(1000+) langtools tests will get fail, 
and other(about 30+, exclude some X11 and sanity that


result problem) test cases will also fail because of local LANG.


such as in the test/hotspot/jtreg/compiler/c2/Test8062950.java,

shouldContain("Error: Could not find or load main class " + CLASSNAME)


When in the zh_CN.UTF-8 environment, because some of the output 
information is changed to Chinese by some properties file,


the English cannot be matched, which will result in an fail.

When I set  LANG=en_US/LC_ALL=C, this test will pass.


I think there are some possible solutions.


1.we set LC_ALL=C/LANG=EN_us in the Runtests.gmk, but this modification 
is more violent because he will affect all test cases.



2.We modify each individual test，E.g 
test/hotspot/jtreg/compiler/c2/Test8062950.java


diff -r 0421d49b6217 test/hotspot/jtreg/compiler/c2/Test8062950.java
  package compiler.c2;
  import jdk.test.lib.process.ProcessTools;
+import java.util.Map;
+import jdk.test.lib.process.OutputAnalyzer;

  public class Test8062950 {
  private static final String CLASSNAME = "DoesNotExist";
  public static void main(String[] args) throws Exception {
- ProcessTools.executeTestJvm("-Xcomp",
- "-XX:-TieredCompilation",
- "-XX:-UseOptoBiasInlining",
- CLASSNAME)
- .shouldHaveExitValue(1)
-    .shouldContain("Error: Could not find or load main 
class " + CLASSNAME)

-    .shouldNotContain("A fatal error has been detected")
-    .shouldNotContain("Internal Error")
-    .shouldNotContain("HotSpot Virtual Machine Error");
+    final ProcessBuilder pb = 
ProcessTools.createJavaProcessBuilder(true,

+ "-Xcomp",
+ "-XX:-TieredCompilation",
+ "-XX:-UseOptoBiasInlining",
+ CLASSNAME);
+    final Map env = pb.environment();
+    env.put("LC_ALL", "en_US.UTF-8");
+    OutputAnalyzer output = new OutputAnalyzer(pb.start());
+ output.shouldHaveExitValue(1);
+    output.shouldContain("Error: Could not find or load main class 
" + CLASSNAME);

+    output.shouldNotContain("A fatal error has 

Re: Review Request: 8221530: Caller sensitive methods not handling caller = null when invoked by JNI code with no java frames on stack

[David Holmes]

Hi Mandy,

This sounds quite reasonable to me. If there is no calling context then 
only public entities of publicly accessible packages should be 
accessible. JNI itself does not apply access checks so JNI code should 
be using direct field access, and not core reflection, if it needs to 
bypass access checks.


David

On 28/03/2019 9:17 am, Mandy Chung wrote:

This is to fix a regression introduced in JDK 12 by JDK-8206240.
This impacts only native application that creates JVM via JNI
and also invokes Field::getField (or other reflection API) via
JNI that triggers reflection access check against the caller class.
The regression happens only when there is no caller frame, i.e.
when a native thread attaches to JVM e.g. custom launcher.

It's unclear why the native code invokes Field::getField via JNI
to get the value of `java.lang.Integer::TYPE`.  Alternatively it could
call GetFieldID to get jfieldID of that field and then GetObjectField
if this has to be done in JNI (perhaps bypass encapsulation?).

There is no clear semantics what reflection should behave when there
is no caller frame.   Previously, JNI call to access 
`java.lang.Integer.TYPE`

field happens to work since it is called very early at runtime and
AccessibleObject::cache is null and happens that cache == caller == null.

The proposed fix is to perform proper access check.  When there is no
caller frame, it only allows to access to public members of a public type
in an unconditional exported API package.

If a native thread attaches to the VM and attempts to access a non-public
member or a public member in a non-exported type e.g. 
jdk.internal.misc.Unsafe,

it will throw IAE whereas the access check succeeds in JDK 11. It is
illegal to access a non-exported type.   I think the compatibility risk
should be low as this only happens to a native code creating its VM and
calling reflection to access a member in a non-exported type. There is
no change to the behavior of JNI GetFieldID and GetObjectField.

Webrev:
http://cr.openjdk.java.net/~mchung/jdk13/webrevs/8221530/webrev.00/

I will create a CSR.

Thanks
Mandy

Re: RFR: 8216539: tools/jar/modularJar/Basic.java timed out

[Mandy Chung]

On 3/27/19 4:56 PM, Lance Andersen wrote:

Hi Mandy,


On Mar 27, 2019, at 7:23 PM, Mandy Chung > wrote:


Hi Lance,

Do you understand what takes so long for this test to run?


Well it is executing a lot of jar commands.  I did not see anything 
that sticks out in the failed logs that point to anything obvious.


One thing you could do is to instrument the test to gather the timing 
statistics.



Is it running fastdebug and -Xcomp or other flag?


It is just a standard windows run.


This is even strange if it's running normal product build.

Mandy



Mandy

On 3/27/19 1:55 PM, Lance Andersen wrote:

Hi all ,

Please review this fix forhttps://bugs.openjdk.java.net/browse/JDK-8216539  
which increases the timeout value for this test which fails once in a blue moon 
on windows.


———
$ hg diff
diff -r dc66ada06693 test/jdk/tools/jar/modularJar/Basic.java
--- a/test/jdk/tools/jar/modularJar/Basic.java  Tue Mar 26 15:36:19 2019 -0700
+++ b/test/jdk/tools/jar/modularJar/Basic.java  Wed Mar 27 16:50:53 2019 -0400
@@ -54,7 +54,7 @@
   *jdk.test.lib.util.FileUtils
   *jdk.test.lib.JDKToolFinder
   * @compile Basic.java
- * @run testng Basic
+ * @run testng/timeout=300 Basic
   * @summary Tests for plain Modular jars & Multi-Release Modular jars
   */
  


——

Best
Lance
  
    

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





  
    

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









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: Review Request: 8221530: Caller sensitive methods not handling caller = null when invoked by JNI code with no java frames on stack

[Sundararajan Athijegannathan]

Looks good.

-Sundar

On 28/03/19, 4:47 AM, Mandy Chung wrote:

This is to fix a regression introduced in JDK 12 by JDK-8206240.
This impacts only native application that creates JVM via JNI
and also invokes Field::getField (or other reflection API) via
JNI that triggers reflection access check against the caller class.
The regression happens only when there is no caller frame, i.e.
when a native thread attaches to JVM e.g. custom launcher.

It's unclear why the native code invokes Field::getField via JNI
to get the value of `java.lang.Integer::TYPE`.  Alternatively it could
call GetFieldID to get jfieldID of that field and then GetObjectField
if this has to be done in JNI (perhaps bypass encapsulation?).

There is no clear semantics what reflection should behave when there
is no caller frame.   Previously, JNI call to access 
`java.lang.Integer.TYPE`

field happens to work since it is called very early at runtime and
AccessibleObject::cache is null and happens that cache == caller == null.

The proposed fix is to perform proper access check.  When there is no
caller frame, it only allows to access to public members of a public type
in an unconditional exported API package.

If a native thread attaches to the VM and attempts to access a non-public
member or a public member in a non-exported type e.g. 
jdk.internal.misc.Unsafe,

it will throw IAE whereas the access check succeeds in JDK 11. It is
illegal to access a non-exported type.   I think the compatibility risk
should be low as this only happens to a native code creating its VM and
calling reflection to access a member in a non-exported type. There is
no change to the behavior of JNI GetFieldID and GetObjectField.

Webrev:
http://cr.openjdk.java.net/~mchung/jdk13/webrevs/8221530/webrev.00/

I will create a CSR.

Thanks
Mandy

Re: RFR: 8216539: tools/jar/modularJar/Basic.java timed out

[Lance Andersen]

Hi Mandy,


> On Mar 27, 2019, at 7:23 PM, Mandy Chung  wrote:
> 
> Hi Lance,
> 
> Do you understand what takes so long for this test to run?

Well it is executing a lot of jar commands.  I did not see anything that sticks 
out in the failed logs that point to anything obvious.
> Is it running fastdebug and -Xcomp or other flag?

It is just a standard windows run.
> 
> Mandy
> 
> On 3/27/19 1:55 PM, Lance Andersen wrote:
>> Hi all ,
>> 
>> Please review this fix for https://bugs.openjdk.java.net/browse/JDK-8216539 
>>  which increases the 
>> timeout value for this test which fails once in a blue moon on windows.
>> 
>> 
>> ———
>> $ hg diff
>> diff -r dc66ada06693 test/jdk/tools/jar/modularJar/Basic.java
>> --- a/test/jdk/tools/jar/modularJar/Basic.java   Tue Mar 26 15:36:19 
>> 2019 -0700
>> +++ b/test/jdk/tools/jar/modularJar/Basic.java   Wed Mar 27 16:50:53 
>> 2019 -0400
>> @@ -54,7 +54,7 @@
>>   *jdk.test.lib.util.FileUtils
>>   *jdk.test.lib.JDKToolFinder
>>   * @compile Basic.java
>> - * @run testng Basic
>> + * @run testng/timeout=300 Basic
>>   * @summary Tests for plain Modular jars & Multi-Release Modular jars
>>   */
>>  
>> 
>> ——
>> 
>> Best
>> Lance
>>   
>> 
>>   
>>  
>>  
>> 
>>   
>> Lance Andersen| 
>> Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering 
>> 1 Network Drive 
>> Burlington, MA 01803
>> lance.ander...@oracle.com  
>>  
>> 
>> 
>> 
>> 
>> 
>>   
>> 
>>   
>>  
>>  
>> 
>>   
>> Lance Andersen| 
>> Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering 
>> 1 Network Drive 
>> Burlington, MA 01803
>> lance.ander...@oracle.com  
>>  
>> 
>> 
>> 
> 

 
  

 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: 8216539: tools/jar/modularJar/Basic.java timed out

[Joe Darcy]

Hello,

I agree it would be preferable to understand why the test times out 
rather than just upping its timeout value.


Thanks,

-Joe

On 3/27/2019 4:23 PM, Mandy Chung wrote:

Hi Lance,

Do you understand what takes so long for this test to run?
Is it running fastdebug and -Xcomp or other flag?

Mandy

On 3/27/19 1:55 PM, Lance Andersen wrote:

Hi all ,

Please review this fix for 
https://bugs.openjdk.java.net/browse/JDK-8216539 which increases the 
timeout value for this test which fails once in a blue moon on windows.



———
$ hg diff
diff -r dc66ada06693 test/jdk/tools/jar/modularJar/Basic.java
--- a/test/jdk/tools/jar/modularJar/Basic.java    Tue Mar 26 15:36:19 
2019 -0700
+++ b/test/jdk/tools/jar/modularJar/Basic.java    Wed Mar 27 16:50:53 
2019 -0400

@@ -54,7 +54,7 @@
   *    jdk.test.lib.util.FileUtils
   *    jdk.test.lib.JDKToolFinder
   * @compile Basic.java
- * @run testng Basic
+ * @run testng/timeout=300 Basic
   * @summary Tests for plain Modular jars & Multi-Release Modular jars
   */

——

Best
Lance

 

Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

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






 

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: 8216539: tools/jar/modularJar/Basic.java timed out

[Mandy Chung]

Hi Lance,

Do you understand what takes so long for this test to run?
Is it running fastdebug and -Xcomp or other flag?

Mandy

On 3/27/19 1:55 PM, Lance Andersen wrote:

Hi all ,

Please review this fix for https://bugs.openjdk.java.net/browse/JDK-8216539 
which increases the timeout value for this test which fails once in a blue moon 
on windows.


———
$ hg diff
diff -r dc66ada06693 test/jdk/tools/jar/modularJar/Basic.java
--- a/test/jdk/tools/jar/modularJar/Basic.java  Tue Mar 26 15:36:19 2019 -0700
+++ b/test/jdk/tools/jar/modularJar/Basic.java  Wed Mar 27 16:50:53 2019 -0400
@@ -54,7 +54,7 @@
   *jdk.test.lib.util.FileUtils
   *jdk.test.lib.JDKToolFinder
   * @compile Basic.java
- * @run testng Basic
+ * @run testng/timeout=300 Basic
   * @summary Tests for plain Modular jars & Multi-Release Modular jars
   */
  


——

Best
Lance
  
   

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





  
   

  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: JDK-8215241: Permissions are not set correctly on sub-folders in /Applications

[Andy Herrick]

fix looks good

/Andy


On 3/27/2019 4:43 PM, semyon.sadet...@oracle.com wrote:

bug: https://bugs.openjdk.java.net/browse/JDK-8215241
webrev: http://cr.openjdk.java.net/~ssadetsky/8215241/webrev.00/ 



The fix adds preinstall/postinstall scripts to the created package 
which ensure that installation dir is created and got proper permissions

to guarantee the application is available to run after installing it.

--Semyon

Review Request: 8221530: Caller sensitive methods not handling caller = null when invoked by JNI code with no java frames on stack

[Mandy Chung]

This is to fix a regression introduced in JDK 12 by JDK-8206240.
This impacts only native application that creates JVM via JNI
and also invokes Field::getField (or other reflection API) via
JNI that triggers reflection access check against the caller class.
The regression happens only when there is no caller frame, i.e.
when a native thread attaches to JVM e.g. custom launcher.

It's unclear why the native code invokes Field::getField via JNI
to get the value of `java.lang.Integer::TYPE`.  Alternatively it could
call GetFieldID to get jfieldID of that field and then GetObjectField
if this has to be done in JNI (perhaps bypass encapsulation?).

There is no clear semantics what reflection should behave when there
is no caller frame.   Previously, JNI call to access 
`java.lang.Integer.TYPE`

field happens to work since it is called very early at runtime and
AccessibleObject::cache is null and happens that cache == caller == null.

The proposed fix is to perform proper access check.  When there is no
caller frame, it only allows to access to public members of a public type
in an unconditional exported API package.

If a native thread attaches to the VM and attempts to access a non-public
member or a public member in a non-exported type e.g. 
jdk.internal.misc.Unsafe,

it will throw IAE whereas the access check succeeds in JDK 11. It is
illegal to access a non-exported type.   I think the compatibility risk
should be low as this only happens to a native code creating its VM and
calling reflection to access a member in a non-exported type. There is
no change to the behavior of JNI GetFieldID and GetObjectField.

Webrev:
http://cr.openjdk.java.net/~mchung/jdk13/webrevs/8221530/webrev.00/

I will create a CSR.

Thanks
Mandy

Re: RFR: JDK-8215241: Permissions are not set correctly on sub-folders in /Applications

[Alexander Matveev]

Hi Semyon,

Looks good.

Thanks,
Alexander

On 3/27/2019 1:43 PM, semyon.sadet...@oracle.com wrote:

bug: https://bugs.openjdk.java.net/browse/JDK-8215241
webrev: http://cr.openjdk.java.net/~ssadetsky/8215241/webrev.00/ 



The fix adds preinstall/postinstall scripts to the created package 
which ensure that installation dir is created and got proper permissions

to guarantee the application is available to run after installing it.

--Semyon

Re: RFR: JDK-8221565: Linux build broken after merge with default branch

[Alexander Matveev]

Hi Andy,

Changes looks fine, but do we really need such try/catch block? I think 
this code needs to be modify, so we do not need such try/catch or handle 
this error as real error. Probably another bug can be filed to do this.


Thanks,
Alexander

On 3/27/2019 12:33 PM, Andy Herrick wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


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

[2] - http://cr.openjdk.java.net/~herrick/8221565

/Andy

Re: RFR (JDK 13) 8221533: Incorrect copyright header in DurationDayTimeImpl.java, DurationYearMonthImpl.java and XMLStreamException.java

[Joe Wang]

Thanks Lance, Brian!

-Joe

On 3/27/19, 1:37 PM, Lance Andersen wrote:

+1
On Mar 27, 2019, at 3:51 PM, Joe Wang > wrote:


Please review a trivial fix for the missing commas in three class files.

hg diff
diff --git 
a/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationDayTimeImpl.java 
b/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationDayTimeImpl.java
--- 
a/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationDayTimeImpl.java
+++ 
b/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationDayTimeImpl.java

@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2017 Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2009, 2017, 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
diff --git 
a/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationYearMonthImpl.java 
b/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationYearMonthImpl.java
--- 
a/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationYearMonthImpl.java
+++ 
b/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationYearMonthImpl.java

@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2010, 2017 Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2010, 2017, 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
diff --git 
a/src/java.xml/share/classes/javax/xml/stream/XMLStreamException.java 
b/src/java.xml/share/classes/javax/xml/stream/XMLStreamException.java

--- a/src/java.xml/share/classes/javax/xml/stream/XMLStreamException.java
+++ b/src/java.xml/share/classes/javax/xml/stream/XMLStreamException.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2017 Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2009, 2017, 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

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: 8221568: DataOutputStream/WriteUTF.java fails due to "OutOfMemoryError: Java heap space"

[Aleksey Shipilev]

On 3/27/19 10:17 PM, Brian Burkhalter wrote:
> https://bugs.openjdk.java.net/browse/JDK-8221568
> 
> Require 64-bit arch and increase heap size to 3 GB [0]. Without this patch, 
> the test failed on macosx-x64 and solaris-sparcv9. With the patch, it passed 
> on those systems as well as linux-x64 and windows-x64.
> 
> Thanks,
> 
> Brian
> 
> [0] Patch
> 
> --- a/test/jdk/java/io/DataOutputStream/WriteUTF.java
> +++ b/test/jdk/java/io/DataOutputStream/WriteUTF.java
> @@ -24,7 +24,8 @@
>  /* @test
>   * @bug 4260284 8219196
>   * @summary Test if DataOutputStream will overcount written field.
> - * @run testng/othervm -Xmx2g WriteUTF
> + * @requires (sun.arch.data.model == "64" & os.maxMemory >= 3g)
> + * @run testng/othervm -Xmx3g WriteUTF
>   */
>  
>  import java.io.ByteArrayOutputStream;
> @@ -59,7 +60,7 @@
>  }
>  
>  // Without 8219196 fix, throws ArrayIndexOutOfBoundsException instead of
> -// expected UTFDataFormatException. Requires 2GB of heap (-Xmx2g) to run
> +// expected UTFDataFormatException. Requires 3GB of heap (-Xmx3g) to run
>  // without throwing an OutOfMemoryError.
>  @Test(expectedExceptions = UTFDataFormatException.class)
>  public void arrayIndexOutOfBoundsException() throws IOException {

Looks good. Thanks for caring about 32-bit.

-Aleksey

8221568: DataOutputStream/WriteUTF.java fails due to "OutOfMemoryError: Java heap space"

[Brian Burkhalter]

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

Require 64-bit arch and increase heap size to 3 GB [0]. Without this patch, the 
test failed on macosx-x64 and solaris-sparcv9. With the patch, it passed on 
those systems as well as linux-x64 and windows-x64.

Thanks,

Brian

[0] Patch

--- a/test/jdk/java/io/DataOutputStream/WriteUTF.java
+++ b/test/jdk/java/io/DataOutputStream/WriteUTF.java
@@ -24,7 +24,8 @@
 /* @test
  * @bug 4260284 8219196
  * @summary Test if DataOutputStream will overcount written field.
- * @run testng/othervm -Xmx2g WriteUTF
+ * @requires (sun.arch.data.model == "64" & os.maxMemory >= 3g)
+ * @run testng/othervm -Xmx3g WriteUTF
  */
 
 import java.io.ByteArrayOutputStream;
@@ -59,7 +60,7 @@
 }
 
 // Without 8219196 fix, throws ArrayIndexOutOfBoundsException instead of
-// expected UTFDataFormatException. Requires 2GB of heap (-Xmx2g) to run
+// expected UTFDataFormatException. Requires 3GB of heap (-Xmx3g) to run
 // without throwing an OutOfMemoryError.
 @Test(expectedExceptions = UTFDataFormatException.class)
 public void arrayIndexOutOfBoundsException() throws IOException {

Re: RFR: 8216539: tools/jar/modularJar/Basic.java timed out

[Brian Burkhalter]

Seems reasonable. [1]

Brian

> On Mar 27, 2019, at 1:55 PM, Lance Andersen  wrote:
> 
> Please review this fix for https://bugs.openjdk.java.net/browse/JDK-8216539 
>  which increases the 
> timeout value for this test which fails once in a blue moon on windows.

[1] https://www.timeanddate.com/astronomy/moon/blue-moon.html

RFR: 8216539: tools/jar/modularJar/Basic.java timed out

[Lance Andersen]

Hi all ,

Please review this fix for https://bugs.openjdk.java.net/browse/JDK-8216539 
which increases the timeout value for this test which fails once in a blue moon 
on windows.


———
$ hg diff
diff -r dc66ada06693 test/jdk/tools/jar/modularJar/Basic.java
--- a/test/jdk/tools/jar/modularJar/Basic.java  Tue Mar 26 15:36:19 2019 -0700
+++ b/test/jdk/tools/jar/modularJar/Basic.java  Wed Mar 27 16:50:53 2019 -0400
@@ -54,7 +54,7 @@
  *jdk.test.lib.util.FileUtils
  *jdk.test.lib.JDKToolFinder
  * @compile Basic.java
- * @run testng Basic
+ * @run testng/timeout=300 Basic
  * @summary Tests for plain Modular jars & Multi-Release Modular jars
  */
 

——

Best
Lance
 
  

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





 
  

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

RFR: JDK-8215241: Permissions are not set correctly on sub-folders in /Applications

[semyon . sadetsky]

bug: https://bugs.openjdk.java.net/browse/JDK-8215241
webrev: http://cr.openjdk.java.net/~ssadetsky/8215241/webrev.00/ 



The fix adds preinstall/postinstall scripts to the created package which 
ensure that installation dir is created and got proper permissions

to guarantee the application is available to run after installing it.

--Semyon

Re: RFR (JDK 13) 8221533: Incorrect copyright header in DurationDayTimeImpl.java, DurationYearMonthImpl.java and XMLStreamException.java

[Lance Andersen]

+1
> On Mar 27, 2019, at 3:51 PM, Joe Wang  wrote:
> 
> Please review a trivial fix for the missing commas in three class files.
> 
> hg diff
> diff --git 
> a/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationDayTimeImpl.java
>  
> b/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationDayTimeImpl.java
> --- 
> a/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationDayTimeImpl.java
> +++ 
> b/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationDayTimeImpl.java
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2009, 2017 Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 2009, 2017, 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
> diff --git 
> a/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationYearMonthImpl.java
>  
> b/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationYearMonthImpl.java
> --- 
> a/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationYearMonthImpl.java
> +++ 
> b/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationYearMonthImpl.java
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2010, 2017 Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 2010, 2017, 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
> diff --git 
> a/src/java.xml/share/classes/javax/xml/stream/XMLStreamException.java 
> b/src/java.xml/share/classes/javax/xml/stream/XMLStreamException.java
> --- a/src/java.xml/share/classes/javax/xml/stream/XMLStreamException.java
> +++ b/src/java.xml/share/classes/javax/xml/stream/XMLStreamException.java
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2009, 2017 Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 2009, 2017, 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
> 
> 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 (JDK 13) 8221533: Incorrect copyright header in DurationDayTimeImpl.java, DurationYearMonthImpl.java and XMLStreamException.java

[Brian Burkhalter]

+1 ,

> On Mar 27, 2019, at 12:51 PM, Joe Wang  wrote:
> 
> Please review a trivial fix for the missing commas in three class files.

RFR (JDK 13) 8221533: Incorrect copyright header in DurationDayTimeImpl.java, DurationYearMonthImpl.java and XMLStreamException.java

[Joe Wang]

Please review a trivial fix for the missing commas in three class files.

hg diff
diff --git 
a/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationDayTimeImpl.java 
b/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationDayTimeImpl.java
--- 
a/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationDayTimeImpl.java
+++ 
b/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationDayTimeImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2017 Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2009, 2017, 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
diff --git 
a/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationYearMonthImpl.java 
b/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationYearMonthImpl.java
--- 
a/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationYearMonthImpl.java
+++ 
b/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/jaxp/datatype/DurationYearMonthImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2017 Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2010, 2017, 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
diff --git 
a/src/java.xml/share/classes/javax/xml/stream/XMLStreamException.java 
b/src/java.xml/share/classes/javax/xml/stream/XMLStreamException.java

--- a/src/java.xml/share/classes/javax/xml/stream/XMLStreamException.java
+++ b/src/java.xml/share/classes/javax/xml/stream/XMLStreamException.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2017 Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2009, 2017, 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

Thanks,
Joe

RFR: JDK-8221565: Linux build broken after merge with default branch

[Andy Herrick]

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


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

[2] - http://cr.openjdk.java.net/~herrick/8221565

/Andy

Re: RFR(xs): 8221375: Windows 32bit build (VS2017) broken

[Martijn Verburg]

Hi Thomas,

Will do and appreciate the support!

Cheers,
Martijn


On Wed, 27 Mar 2019 at 11:13, Thomas Stüfe  wrote:

>
>
> On Wed, Mar 27, 2019 at 11:08 AM Martijn Verburg 
> wrote:
>
>> Hi Thomas,
>>
>> Release only (we've only created debug builds for x64),
>>
>
> Okay. A number of those errors were debug only.
>
>
>> we do use the treat errors as warnings flag.
>>
>
> Other way around, I hope ;)
>
>
>>
>> You can look at all of our build configurations and build histories at
>> ci.adoptopenjdk.net - and I've copied in Ali Ince who's working for
>> jClarity on Windows 32/64 builds amongst other things.
>>
>> Okay, that's useful.
>
> JDK changes have been pushed (JDK-8221405
>  , JDK-8221406
>  , JDK-8221407
>  , first one in
> jdk/client since awt maintainers asked me to). Hotspot changes still under
> review, we ponder the best way to deal with a supposed vc++ bug here (
> JDK-8221408  ). Feel
> free to test or review.
>
> Note that I do not plan to keep windows 32 bit alive, this was just a
> weekend thing because I needed a 32bit VM on windows for some comparisons.
> So I'd be happy that you guys are regularly building it (if possible
> fastdebug too). Please report any build breaks. When reported promptly,
> with the offending change linked in JBS, the authors will often try to fix
> it themselves.
>
> Cheers, Thomas
>
>
>> Cheers,
>> Martijn
>>
>>
>> On Tue, 26 Mar 2019 at 06:04, Thomas Stüfe 
>> wrote:
>>
>>>
>>>
>>> On Mon, Mar 25, 2019 at 10:49 PM Martijn Verburg <
>>> martijnverb...@gmail.com> wrote:
>>>
 Hi all,

 Snipping much, but commenting on one statement inline below.

 On Mon, 25 Mar 2019 at 01:58, David Holmes 
 wrote:

> Hi Thomas,
>
> A few queries, comments and concerns ...
>
> On 25/03/2019 6:58 am, Thomas Stüfe wrote:
> > Hi all,
> >
> > After a long time I tried to build a Windows 32bit VM (VS2017) and
> failed;
>
> I'm somewhat surprised as I thought someone was actively doing Windows
> 32-bit builds out there, plus there are shared code changes that
> should
> also have been caught by non-Windows 32-bit builds. :(
>

 AdoptOpenJDK is building Win-32 but we only recently swapped to VS2017
 to do so and have hit the same issues (Thomas beat us to reporting!).


>>> Still curious what you actually build, since I believe not all of the
>>> issues are related to vs2017.
>>>
>>> Do you build release only or also debug? Do you build with
>>> warnings-as-errors disabled?
>>>
>>>
 Hopefully, we'll have that nightly warning system in place for you
 going forward soon.


>>> That would be helpful.
>>>
>>> Cheers, Thomas
>>>
>>>
 Cheers,
 Martijn

>>>

RE: RFR: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows

[Baesken, Matthias]

Hello could you please look into it ?

Best regards, Matthias



> -Original Message-
> From: Baesken, Matthias
> Sent: Montag, 25. März 2019 18:05
> To: Langer, Christoph 
> Cc: 'core-libs-dev@openjdk.java.net' ;
> 'Alan Bateman' 
> Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code
> (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> parse_manifest.c on windows
> 
> Hello here is an updated webrev :
> 
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.1/webrev/
> 
> 
> 
> > > In  JLI_Open(const char* name, int flags), you should remove ret and only
> > > use fd, I think.
> 
> 
> I removed ret  and adjusted some comments.
> 
> Additionally I added a  test  that uses (on Windows) JLI_Open  on  a  jar 
> file
> with a "long"   path  (> 260 chars).
>  [ JLI_Open  is currently called  from  parse_manifest.c  with jarfile as
> parameter ]
> 

Re: Formatting rules for exception messages

[mark . reinhold]

2019/3/25 5:24:37 -0700, Florian Weimer :
> Are there any guidelines for formatting exception messages?
> 
> In particular, I'm interested in the case when the exception message
> is a (clipped) sentence.  Is it supposed to start with a capital
> letter?
> 
> If the message refers to a parameter name, the spelling should reflect
> the name exactly, of course.  There seems to be a slight bias towards
> capitalization, based on a few greps.  ...

The first word of any exception message in code that I’ve written, or
reviewed, is always capitalized unless that word conveys case-sensitive
technical information (e.g., a parameter name, as you mentioned).  This
improves readability, especially in longer exception messages that
contain additional punctuation characters.

- Mark

Re: Request for sponsor: JDK-8221430: StringBuffer(CharSequence) constructor truncates when -XX:-CompactStrings specified

[Martin Buchholz]

Anyone who does String coder hacking should meditate on the wisdom at:
https://shipilev.net/#string-catechism

RE: RFR(L): 8218628: Add detailed message to NullPointerException describing what is null.

[Lindenmaier, Goetz]

Hi Mandy,

>and also the cases when it requires to look at its caller 
> frame.
> 
>   Never.  Only the method that was executed when the exception is
>   thrown is needed. I only mention the backtrace because it happens to
>   store the top method and the corresponding bytecode position in the
>   top method.
> 
> 
> That explains why I don't see an example of:
>  getNull().m();
> 
>  public void m() {};
> 
>  static NPE getNull() {
>  return null;
>  }
> 
> For a compiler expert or one who study the data flow analysis paper
Yes, I think compiler people should understand this.

> , is this example very clear to them that it won't be supported?
The example for your code is supported, see[1], examples 11-14. (I should 
enumerate them)
For your code, it will print
"The return value of 'MyClass.getNULL()LNPE;' is null. Can not invoke method 
'MyOtherClass.m()V'".

getNull().m() are not nested calls, but subsequent ones. Therefore you will not 
find
them on the call stack.
The byte code index of the invoke bytecode invoking m() is known from the 
backtrace or
StackWalker/StackFrame.  Looking at the bytecode the part "Can not invoke 
method 'MyOtherClass.m()V"
is printed.
With the result of the data flow analysis it is possible to find the bytecode 
that produced
the reference on which m() is invoked.  This is another invoke bytecode in the 
method.
Looking at this other bytecode, " The return value of 'MyClass.getNULL()LNPE;' 
is null." 
is printed.

You probably meant that for 
  n().getNull().m()
it is not printed that getNull() was called on the result of n()?
This is a design decision not to make the printout too complex.
 
> This is related to my comment w.r.t. the data flow analysis comes from.  I was
> looking for some level of information be spelled out in the JEP that can
> sufficiently give a non-compiler expert an idea but of course no point to
> replicate the paper here.
"The simulated stack" ... contains ..."information about which bytecode pushed 
the value to the stack."
That's all.

> It's good to know that this proposal needs the top frame only.   Please make 
> it
> clear in the JEP.   
I tried to point out more precisely why and when I need the top frame. I 
pushed this down a bit in the text in "basic algorithm".

> The JEP can take out `StackWalker` but instead talks about
> `StackFrame` since doing it in Java means that it only needs to get back a 
> `StackFrame`
> instance representing the top frame stored in `Throwable::backtrace`
Yes. I tried to make the text more precise.

> This will avoid the confusion on calling StackWalker in the NPE constructor 
> and
> constructing many StackFrame and many ResolvedMethodName.  That was
> part of the code review discussion.
Yes, only the top StackFrame is needed.

>   "Computing either part of these messages can fail due to
> insufficient
>   information."
>   What are the cases that it fails to obtain the information? It'd
> be useful
>   to list some examples if not all.
> 
>   (a ? b : c).d
>   You can not know whether the ?-expression yielded b or c, thus
>   you don't know why the access .d failed.  There is a corresponding
> example
>   in [1].
> 
> 
> Please describe in the JEP the known cases that this proposal doesn't cover.
> Conditional expression is one and 

The algorithm looks at a lot of bytecodes to print the method. I can not 
enumerate all combinations of bytecodes, it's millions. I will add a 
per-bytecode table to 
the "Message content" section describing what is printed for which bytecode.

>null receiver if it's the return value from a caller frame (I think this would 
>help the readers).
... this is supported, see above.

>   The "Different bytecode variants of a method" section
> 
>   This section raises the case when a method representing an
> execution stack
>   frame throwing NPE becomes obsolete, it does not have the
> information
>   to compute the message lazily.  So this falls into the
> "insufficient
>   information" category and no improved NPE message can be
> generated.
> 
>   Yes, note this also only addresses the method in which the exception
> was raised.
> 
> This should also go to the list of known cases that this proposal does not 
> cover.

This paragraph lists a possible problem -- multiple variants of methods
with the same name and class name -- and how this is solved. 
I added a section "Cases not covered by this JEP" and mention it there, too.

> Since the JEP quotes that NullPointerExceptions are thrown frequently
> and swallowed, it would help if you can generate some numbers.
> This JEP will help the discussion on the proposed feature and design and avoid
> any
> confusion/assumption of the implementation.
I'll generate some numbers.  Is running jvm2008 fine?

> I think this is related to this JEP.The question 

Re: Request for sponsor: JDK-8221430: StringBuffer(CharSequence) constructor truncates when -XX:-CompactStrings specified

[Roger Riggs]

Hi Ivan,

On 03/26/2019 07:30 PM, Ivan Gerasimov wrote:
The main source of confusion here seems to be due to that the coder is 
passed in as an argument, so it either needs to be trusted or has to 
be verified in the constructor.


The design for coder is that it *is* trusted in all 
internal/implementation APIs.

Subsequent changes should not weaken this invariant, it will cause doubt
in the code and cast doubt on all of the performance work that has
been done to optimize its use.


So, let's instead introduce AbstractStringBuilder(CharSequence) and 
make it do all manipulations.


http://cr.openjdk.java.net/~igerasim/8221430/01/webrev/

Note that the common case of StringBuilder(String) already gets 
intrinsified by hotspot.


What do you think?


This looks good, the logic is in one place.

Since StringBuilder(String) is an intrinsic, my doubt about a slight 
performance

impact is unwarranted in that specific case.

Are there any StringBuffer/Builder jmh tests than be easily rerun to 
compare?


Thanks, Roger



With kind regards,
Ivan

On 3/26/19 1:04 PM, Ivan Gerasimov wrote:
From the design point of view, I believe it is better to have the 
constructor AbstractStringBuilder(int, int, int) to check if the 
coder argument makes sense with respect to the value of 
COMPACT_STRING, so it won't be possible to create a StringBuilder 
with the coder==LATIN1, when it is not supported.


For calculating the coderHint then, it is not necessary to check 
COMPACT_STRING:  If the CharSequence argument is in fact String or 
AbstractStringBuilder, the coder is known, otherwise LATIN1 can be 
passed in as a hint (for an arbitrary CharSequence it is not 100% 
accurate anyway).
The constructor AbstractStringBuilder(int, int, int) will then either 
use the provided coder, or adjust it if necessary.


Will we agree on something like following?

http://cr.openjdk.java.net/~igerasim/8221430/00/webrev/

With kind regards,

Ivan


On 3/26/19 12:14 PM, Roger Riggs wrote:

Hi,

We've got the subject open and its fresh, there's no need for a 
separate review cycle.


The first fix (-01) does not seem to be consistent with the original 
design
and handling of the coder.  The StringBuilder(String) and 
StringBuffer(String)

constructors are the pattern that should be followed for determining
the coder for the new instance.

Checking for COMPACT_STRING in two places (the AbstractStringBuilder 
and
the sub classes) is unnecessary and distributes the information 
about the

correct coder across two classes where determining what it should be
in the subclass has more complete  information and can correctly 
determine

the coder once.

We can likely find a reviewer to be a tie-breaker if Ivan sees it as 
desirable.


Thanks, Roger


On 03/26/2019 02:38 PM, Andrew Leonard wrote:
Sorry chaps, I think my brain is getting confused!, I think we have 
conflicting reviews here?


Roger, I added the getCharSequenceCoder() to AbstractStringBuilder 
so it was only defined in one place..
I agree with this being called in StringBuffer/Builder then we 
don't need the change to AbstractStringBuild() constuctor, however 
Ivan wants getCharSequenceCoder() that done as a separate "bug".


So I think it comes down to do we do this as 2 "bugs" or 1 ?

Thanks
Andrew

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com




From: Roger Riggs 
To: Andrew Leonard , Ivan Gerasimov 


Cc: core-libs-dev@openjdk.java.net
Date: 26/03/2019 18:19
Subject: Re: Request for sponsor: JDK-8221430: 
StringBuffer(CharSequence) constructor truncates when 
-XX:-CompactStrings specified
 





Hi Andrew,

You are going to have to change the same code twice
because the changes should be the StringBuffer and StringBuilder
constructors and would remove the code that is added to
the AbstractStringBuilder constructor.  That's a waste of review 
cycles.



On 03/26/2019 11:45 AM, Andrew Leonard wrote:
Hi Roger,
No worries, the more the merrier!
So that was one of my reasoning for adding getCharSequenceCode() 
was, I think what you're suggesting is my webrev.01, 
_http://cr.openjdk.java.net/~aleonard/8221430/webrev.01/_ 

Ivan's view is that behaviour was an extended issue, which it is, 
but I thought it was nice to add..


Which patch do we favour? webrev-01 or -02 ?

Neither, there should be no change to the AbstractStringBuilder 
constructor

and the change should be done in the subclass constructors.

Roger


Thanks
Andrew

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: _andrew_m_leon...@uk.ibm.com_ 






From: Roger Riggs __ 

To: _core-libs-...@openjdk.java.net_ 

Re: RFR: JDK-8221525: jpackage fails with non-ASCII characters in --copyright

[Andy Herrick]

change looks good.

/Andy


On 3/26/2019 10:57 PM, Alexander Matveev wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


- .properties file was written in default encoding 
(file.encoding=Cp1252). Native code was expecting UTF-8 and thus was
not able to read non-ASCII characters correctly. Fixed by writing file 
using UTF-8.


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

[2] http://cr.openjdk.java.net/~almatvee/8221525/webrev.00/

Thanks,
Alexander

Re: Request for sponsor: JDK-8221430: StringBuffer(CharSequence) constructor truncates when -XX:-CompactStrings specified

[Andrew Leonard]

I like your change Ivan, i've just reviewed it all and it works nicely for 
me, it puts all the key logic in one place. Roger what do you think?
Thanks
Andrew

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com 




From:   Ivan Gerasimov 
To: Roger Riggs , Andrew Leonard 
, Roger Riggs 
Cc: core-libs-dev@openjdk.java.net
Date:   26/03/2019 23:32
Subject:Re: Request for sponsor: JDK-8221430: 
StringBuffer(CharSequence) constructor truncates when -XX:-CompactStrings 
specified



The main source of confusion here seems to be due to that the coder is 
passed in as an argument, so it either needs to be trusted or has to be 
verified in the constructor.

So, let's instead introduce AbstractStringBuilder(CharSequence) and make 
it do all manipulations.

https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Eigerasim_8221430_01_webrev_=DwIC-g=jf_iaSHvJObTbx-siA1ZOg=NaV8Iy8Ld-vjpXZFDdTbgGlRTghGHnwM75wUPd5_NUQ=pE0lO6dslRWLLCEXQmHn9OPe4VOGzmjvtUfoAHAi1fQ=j-sj2TN_BPxc56godShkZiyhX_3m_a1rw1TERHfpIwQ=


Note that the common case of StringBuilder(String) already gets 
intrinsified by hotspot.

What do you think?

With kind regards,
Ivan

On 3/26/19 1:04 PM, Ivan Gerasimov wrote:
> From the design point of view, I believe it is better to have the 
> constructor AbstractStringBuilder(int, int, int) to check if the coder 
> argument makes sense with respect to the value of COMPACT_STRING, so 
> it won't be possible to create a StringBuilder with the coder==LATIN1, 
> when it is not supported.
>
> For calculating the coderHint then, it is not necessary to check 
> COMPACT_STRING:  If the CharSequence argument is in fact String or 
> AbstractStringBuilder, the coder is known, otherwise LATIN1 can be 
> passed in as a hint (for an arbitrary CharSequence it is not 100% 
> accurate anyway).
> The constructor AbstractStringBuilder(int, int, int) will then either 
> use the provided coder, or adjust it if necessary.
>
> Will we agree on something like following?
>
> 
https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Eigerasim_8221430_00_webrev_=DwIC-g=jf_iaSHvJObTbx-siA1ZOg=NaV8Iy8Ld-vjpXZFDdTbgGlRTghGHnwM75wUPd5_NUQ=pE0lO6dslRWLLCEXQmHn9OPe4VOGzmjvtUfoAHAi1fQ=ZbLXiATpnDRMbb66KK6lsr6rdglh7N7ld_x10JKKa9Q=

>
> With kind regards,
>
> Ivan
>
>
> On 3/26/19 12:14 PM, Roger Riggs wrote:
>> Hi,
>>
>> We've got the subject open and its fresh, there's no need for a 
>> separate review cycle.
>>
>> The first fix (-01) does not seem to be consistent with the original 
>> design
>> and handling of the coder.  The StringBuilder(String) and 
>> StringBuffer(String)
>> constructors are the pattern that should be followed for determining
>> the coder for the new instance.
>>
>> Checking for COMPACT_STRING in two places (the AbstractStringBuilder 
and
>> the sub classes) is unnecessary and distributes the information about 
>> the
>> correct coder across two classes where determining what it should be
>> in the subclass has more complete  information and can correctly 
>> determine
>> the coder once.
>>
>> We can likely find a reviewer to be a tie-breaker if Ivan sees it as 
>> desirable.
>>
>> Thanks, Roger
>>
>>
>> On 03/26/2019 02:38 PM, Andrew Leonard wrote:
>>> Sorry chaps, I think my brain is getting confused!, I think we have 
>>> conflicting reviews here?
>>>
>>> Roger, I added the getCharSequenceCoder() to AbstractStringBuilder 
>>> so it was only defined in one place..
>>> I agree with this being called in StringBuffer/Builder then we don't 
>>> need the change to AbstractStringBuild() constuctor, however Ivan 
>>> wants getCharSequenceCoder() that done as a separate "bug".
>>>
>>> So I think it comes down to do we do this as 2 "bugs" or 1 ?
>>>
>>> Thanks
>>> Andrew
>>>
>>> Andrew Leonard
>>> Java Runtimes Development
>>> IBM Hursley
>>> IBM United Kingdom Ltd
>>> Phone internal: 245913, external: 01962 815913
>>> internet email: andrew_m_leon...@uk.ibm.com
>>>
>>>
>>>
>>>
>>> From: Roger Riggs 
>>> To: Andrew Leonard , Ivan Gerasimov 
>>> 
>>> Cc: core-libs-dev@openjdk.java.net
>>> Date: 26/03/2019 18:19
>>> Subject: Re: Request for sponsor: JDK-8221430: 
>>> StringBuffer(CharSequence) constructor truncates when 
>>> -XX:-CompactStrings specified
>>> 
 
>>>
>>>
>>>
>>>
>>> Hi Andrew,
>>>
>>> You are going to have to change the same code twice
>>> because the changes should be the StringBuffer and StringBuilder
>>> constructors and would remove the code that is added to
>>> the AbstractStringBuilder constructor.  That's a waste of review 
>>> cycles.
>>>
>>>
>>> On 03/26/2019 11:45 AM, Andrew Leonard wrote:
>>> Hi Roger,
>>> No worries, the more the merrier!
>>> So that was one of my reasoning for adding getCharSequenceCode() 
>>> was, I think what you're suggesting is my webrev.01, 
>>> 

Re: RFR: 8221473: Configuration::reads can use Set.copyOf

[Peter Levart]

Hi,

On 3/26/19 7:44 PM, Daniel Fuchs wrote:

Hi Peter,

On 26/03/2019 18:01, Peter Levart wrote:

Would such method addition be worth it?


Here is the impl of Set.copyOf:

    static  Set copyOf(Collection coll) {
    if (coll instanceof ImmutableCollections.AbstractImmutableSet) {
    return (Set)coll;
    } else {
    return (Set)Set.of(new HashSet<>(coll).toArray());
    }
    }



Yes, and that method is suitable for situations where there is a chance 
of one or the other case and the user is aware of performance 
implications of either case. There are other situations where the code 
guarantees that the Set at hand is already immutable and programmer 
might just want to verify that as a post-condition before returning it. 
In that case the non-immutability would be considered an error. I'm 
thinking of the following implementation:


    static  Set requireImmutable(Set set) {
    if (set instanceof ImmutableCollections.AbstractImmutableSet) {
    return set;
    } else {
    throw new IllegalStateException("Not an immutable Set");
    }
    }

Currently the immutable implementation classes are package-private so 
there's no way for 3rd party code (short of introspecting the class 
hierarchy of the implementation class against the FQN of the 
AbstractImmutableSet class, which might be costly and fragile) to 
implement a helper method like this.


Regards, Peter


Best regards,

-- daniel

Re: RFR(xs): 8221375: Windows 32bit build (VS2017) broken

[Thomas Stüfe]

On Wed, Mar 27, 2019 at 11:08 AM Martijn Verburg 
wrote:

> Hi Thomas,
>
> Release only (we've only created debug builds for x64),
>

Okay. A number of those errors were debug only.


> we do use the treat errors as warnings flag.
>

Other way around, I hope ;)


>
> You can look at all of our build configurations and build histories at
> ci.adoptopenjdk.net - and I've copied in Ali Ince who's working for
> jClarity on Windows 32/64 builds amongst other things.
>
> Okay, that's useful.

JDK changes have been pushed (JDK-8221405
 , JDK-8221406
 , JDK-8221407
 , first one in
jdk/client since awt maintainers asked me to). Hotspot changes still under
review, we ponder the best way to deal with a supposed vc++ bug here (
JDK-8221408  ). Feel free
to test or review.

Note that I do not plan to keep windows 32 bit alive, this was just a
weekend thing because I needed a 32bit VM on windows for some comparisons.
So I'd be happy that you guys are regularly building it (if possible
fastdebug too). Please report any build breaks. When reported promptly,
with the offending change linked in JBS, the authors will often try to fix
it themselves.

Cheers, Thomas


> Cheers,
> Martijn
>
>
> On Tue, 26 Mar 2019 at 06:04, Thomas Stüfe 
> wrote:
>
>>
>>
>> On Mon, Mar 25, 2019 at 10:49 PM Martijn Verburg <
>> martijnverb...@gmail.com> wrote:
>>
>>> Hi all,
>>>
>>> Snipping much, but commenting on one statement inline below.
>>>
>>> On Mon, 25 Mar 2019 at 01:58, David Holmes 
>>> wrote:
>>>
 Hi Thomas,

 A few queries, comments and concerns ...

 On 25/03/2019 6:58 am, Thomas Stüfe wrote:
 > Hi all,
 >
 > After a long time I tried to build a Windows 32bit VM (VS2017) and
 failed;

 I'm somewhat surprised as I thought someone was actively doing Windows
 32-bit builds out there, plus there are shared code changes that should
 also have been caught by non-Windows 32-bit builds. :(

>>>
>>> AdoptOpenJDK is building Win-32 but we only recently swapped to VS2017
>>> to do so and have hit the same issues (Thomas beat us to reporting!).
>>>
>>>
>> Still curious what you actually build, since I believe not all of the
>> issues are related to vs2017.
>>
>> Do you build release only or also debug? Do you build with
>> warnings-as-errors disabled?
>>
>>
>>> Hopefully, we'll have that nightly warning system in place for you going
>>> forward soon.
>>>
>>>
>> That would be helpful.
>>
>> Cheers, Thomas
>>
>>
>>> Cheers,
>>> Martijn
>>>
>>

Re: RFR (XS) 8221524: java/util/Base64/TestEncodingDecodingLength.java should be disabled on 32-bit platforms

[Alan Bateman]

On 26/03/2019 23:00, Aleksey Shipilev wrote:

- * @requires os.maxMemory >= 10g
+ * @requires (sun.arch.data.model == "64" & os.maxMemory >= 10g)


Looks okay to me.

-Alan

Re: RFR (XS) 8221524: java/util/Base64/TestEncodingDecodingLength.java should be disabled on 32-bit platforms

[Thomas Stüfe]

Good & trivial.

..Thomas

On Wed, Mar 27, 2019 at 12:00 AM Aleksey Shipilev  wrote:

> Bug:
>   https://bugs.openjdk.java.net/browse/JDK-8221524
>
> It currently requests -Xmx8g, which is above the max representable memory
> for 32-bit VM. The test
> allocates byte[Integer.MAX_VALUE - C], which means we cannot trim the heap
> size. Therefore, we have
> to disable the test on 32-bit.
>
> Fix:
>
> diff -r 35cb5d1c5881
> test/jdk/java/util/Base64/TestEncodingDecodingLength.java
> --- a/test/jdk/java/util/Base64/TestEncodingDecodingLength.java Tue Mar 26
> 22:29:54 2019 +0100
> +++ b/test/jdk/java/util/Base64/TestEncodingDecodingLength.java Tue Mar 26
> 23:57:45 2019 +0100
> @@ -28,11 +28,11 @@
>  /**
>   * @test
>   * @bug 8210583 8217969 8218265
>   * @summary Tests Base64.Encoder.encode and Base64.Decoder.decode
>   * with the large size of input array/buffer
> - * @requires os.maxMemory >= 10g
> + * @requires (sun.arch.data.model == "64" & os.maxMemory >= 10g)
>   * @run main/othervm -Xms6g -Xmx8g TestEncodingDecodingLength
>   *
>   */
>
> Testing: Linux {x86_64, x86_32} fastdebug, affected test
>
> --
> Thanks,
> -Aleksey
>
>
>

Re: RFR(xs): 8221375: Windows 32bit build (VS2017) broken

[Martijn Verburg]

Hi Thomas,

Release only (we've only created debug builds for x64), we do use the treat
errors as warnings flag.

You can look at all of our build configurations and build histories at
ci.adoptopenjdk.net - and I've copied in Ali Ince who's working for
jClarity on Windows 32/64 builds amongst other things.

Cheers,
Martijn


On Tue, 26 Mar 2019 at 06:04, Thomas Stüfe  wrote:

>
>
> On Mon, Mar 25, 2019 at 10:49 PM Martijn Verburg 
> wrote:
>
>> Hi all,
>>
>> Snipping much, but commenting on one statement inline below.
>>
>> On Mon, 25 Mar 2019 at 01:58, David Holmes 
>> wrote:
>>
>>> Hi Thomas,
>>>
>>> A few queries, comments and concerns ...
>>>
>>> On 25/03/2019 6:58 am, Thomas Stüfe wrote:
>>> > Hi all,
>>> >
>>> > After a long time I tried to build a Windows 32bit VM (VS2017) and
>>> failed;
>>>
>>> I'm somewhat surprised as I thought someone was actively doing Windows
>>> 32-bit builds out there, plus there are shared code changes that should
>>> also have been caught by non-Windows 32-bit builds. :(
>>>
>>
>> AdoptOpenJDK is building Win-32 but we only recently swapped to VS2017 to
>> do so and have hit the same issues (Thomas beat us to reporting!).
>>
>>
> Still curious what you actually build, since I believe not all of the
> issues are related to vs2017.
>
> Do you build release only or also debug? Do you build with
> warnings-as-errors disabled?
>
>
>> Hopefully, we'll have that nightly warning system in place for you going
>> forward soon.
>>
>>
> That would be helpful.
>
> Cheers, Thomas
>
>
>> Cheers,
>> Martijn
>>
>