Re: RFR: 8174222: LambdaMetafactory: validate inputs and improve documentation

[Mandy Chung]

On 3/20/19 9:03 PM, Dan Smith wrote:

http://cr.openjdk.java.net/~dlsmith/8174222/webrev.00/


AbstractValidatingLambdaMetafactory.java

+ throw new LambdaConversionException("implementation is not direct or 
cannot be cracked"); It may help to print implementation method handle: 
throw new LambdaConversionException(implementation + " is not direct or 
cannot be cracked");


If you mind the formatting, the text descripting @param seems
to be aligned with the first word in the description above it.
I don't know if the webrev shows the whitespace properly
you may want to check out line 90-93.

Where does SecurityException get thrown?

I think this needs a CSR.  metafactory and altMetafactory @throws
IAE, NPE and SecurityException.

The class description of LambdaMetafactory also promotes @implNote to the spec.

Otherwise looks good.

Mandy

RFR: 8174222: LambdaMetafactory: validate inputs and improve documentation

[Dan Smith]

http://cr.openjdk.java.net/~dlsmith/8174222/webrev.00/

Please review this update to java.lang.invoke.LambdaMetafactory. Adds some 
argument validation with tests; improves documentation; renames some variables.

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

—Dan

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

[Mandy Chung]

On 3/20/19 1:54 AM, Lindenmaier, Goetz wrote:

Should I move the JEP to status 'submitted'?


Per the Process states section  [1]

Draft --- In circulation by the author for initial review and 
consensus-building


I certainly don't want to be the bottleneck.   Others can help do the 
initial review.


Mandy
[1] http://openjdk.java.net/jeps/1#Process-states

Re: RFR 8211941 : Enable checking/ignoring of non-conforming Class-Path entries

[Mandy Chung]

Hi Brent,

The change looks fine.

Can you file a JBS issue to follow up a place to document this
JDK-specific property?

thanks
Mandy

On 3/20/19 2:01 PM, Brent Christian wrote:

Hi,

JDK-8216401[1][2] added code to improve JAR spec conformance (WRT 
relative URLs in the Class-Path attribute), while maintaining some key 
compatibility aspects (entries using "file:" URLs are not relative, 
but are used by some libraries).


This code was disabled by default, but we would now like to enable it.

Bug:
https://bugs.openjdk.java.net/browse/JDK-8211941
Webrev:
http://cr.openjdk.java.net/~bchristi/8211941/webrev-06/

This change includes a new system property 
("jdk.net.URLClassPath.showIgnoredClassPathEntries") that can be set 
to aid in debugging JAR Class-Paths by printing entries that are ignored.


Thanks,
-Brent

1. https://bugs.openjdk.java.net/browse/JDK-8216401

2. 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-January/057868.html

RFR: JDK-8215019: Allow --install-dir on windows

[Alexander Matveev]

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).


- Added support for --install-dir on Windows. It should be relative path 
to "Program Files" or "AppData".

- If --install-dir is invalid we will use app name instead.
- Added two new tests to validate --install-dir and related functionality.

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

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

Thanks,
Alexander

Re: RFR: docs JDK-8220261: fix headings in java.base

[Mandy Chung]

I looked through the webrev and this looks fine to me.

Mandy

On 3/20/19 2:50 PM, Jonathan Gibbons wrote:
Please review a medium-sized but conceptually simple patch to fix most 
of the headings in the generated documentation for java.base, as 
described in [1]. This does not address the headings in 
java.util.concurrent which are reported separately, in JDK-8220248.


The intent is to ensure that the headings in the generated pages are 
well-structured, with no gaps, and correct implicit nesting. 
Generally, the changes fall into three kinds:


 * Headings in doc-comments for modules, packages and types start at 
 * Headings in doc comments for constructors, fields, methods and other
   members start at 
 * Headings in package.html and doc-files/*.html start at 

The headings have been verified with the doccheck utility.

We cannot re-enable doclint accessibility checking for java.base until 
JDK-8220248 has also been fixed.


-- Jon

JBS: https://bugs.openjdk.java.net/browse/JDK-8220261
Webrev: http://cr.openjdk.java.net/~jjg/8220261/webrev.00/index.html

[1]: 
https://mail.openjdk.java.net/pipermail/jdk-dev/2019-March/002671.html

Re: [PATCH] Some improvements for java.lang.Class

[Peter Levart]

Hi Sergei,

I don't know if you are aware that the new invokedynamic based 
translation of string concatenation expressions introduced in JDK 9 
(http://openjdk.java.net/jeps/280) only applies to code outside 
java.base module. java.base module is still compiled with old-style 
StringBuilder based translation of string concatenation expressions 
because of bootstraping issues.


If your benchmarks bellow are compiled with option 
-XDstringConcat=inline to disable JEP280 translation (as java.base 
module is), then it's OK. Else you are not benchmarking the right 
translation of the code as you intend to change the code in java.base 
module.


Regards, Peter

On 3/20/19 7:35 PM, Сергей Цыпанов wrote:

I had a brief conversation with Brian Goetz which has left off the mailing list 
for some reason. Here's the text:
---
Brian:

These enhancements seem reasonable; these are exactly the cases that 
String::repeat was intended for.  (This is a “is this a reasonable idea” 
review, not a code review.).
Have you looked for other places where similar transformations could be done?

---
Me:

Hello,
after I had realized that looped StringBuilder::append calls can sometimes be 
replaced with String::repeat I requested corresponding inspection in IDEA issue 
tracker.
Using the inspection I’ve found 129 similar warnings in jdk13 repo.
Some of them are very promising, e.g. BigDecimal:: toPlainString where 
currently we have


int trailingZeros = checkScaleNonZero((-(long)scale));
StringBuilder buf;
if(intCompact!=INFLATED) {
 buf = new StringBuilder(20+trailingZeros);
 buf.append(intCompact);
} else {
 String str = intVal.toString();
 buf = new StringBuilder(str.length()+trailingZeros);
 buf.append(str);
}
for (int i = 0; i < trailingZeros; i++) {
 buf.append('0');
}
return buf.toString();

which in fact can be simplified to


int trailingZeros = checkScaleNonZero((-(long)scale));
if(intCompact!=INFLATED) {
 return intCompact + "0".repeat(trailingZeros);
} else {
 return intVal.toString() + "0".repeat(trailingZeros);
}

The second solution is not only shorter and more simple but it likely to be 
significantly faster and memory-saving.
BTW, your reply to original message for some reason is missing from web-view.

---
Brian:

Cool.  Note that replacing append() calls with repeat is not necessarily a win 
for anything other than code compactness; String::repeat will create a new 
string, which will then get appended to another StrinBuilder.  So when used 
correctly (such as presized StringBuilders) appending in a loop is probably 
just as fast (look at the implementation of String::repeat.).


if(intCompact!=INFLATED) {
 return intCompact + "0".repeat(trailingZeros);
} else {
 return intVal.toString() + "0".repeat(trailingZeros);
}

Which can be further simplified to


((intCompact != INFLATED) ? intCompact : intVal.toString) + 
“0”.repeat(trailingZeros);
  
The second solution is not only shorter and more simple but it likely to be significantly faster and memory-saving.

I like short and simple.  I am questioning the “faster and memory saving.”  
That should be validated.

---
Me:

I think optimizations provided by JEP 280 allow compiler to optimize String 
concatenation executed with '+' by using StringBuilder of exact size (when the 
size of all components is known, like in our case).

I've checked this with benchmark


@State(Scope.Thread)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
public class ConcatBenchmark {

 @Param({"1", "2", "5", "8"})
 private int trailingZeros;

 private final long intCompact = 1L;

 @Benchmark
 public String stringBuilder() {
 StringBuilder buf;
 buf = new StringBuilder(20 + trailingZeros);
 buf.append(intCompact);
 for (int i = 0; i < trailingZeros; i++) {
 buf.append('0');
 }
 return buf.toString();
 }
 @Benchmark
 public String stringRepeat() {
 return intCompact + "0".repeat(trailingZeros);
 }
}

Results:
 trailingZeros  Mode  Cnt Score 
Error   Units
stringBuilder   1  avgt   1517,683 ±   
0,664   ns/op
stringRepeat1  avgt   15 9,052 ±   
0,042   ns/op

stringBuilder   2  avgt   1519,053 ±   
1,206   ns/op
stringRepeat2  avgt   1515,864 ±   
1,331   ns/op

stringBuilder   5  avgt   1525,839 ±   
2,256   ns/op
stringRepeat5  avgt   1519,290 ±   
1,685   ns/op

stringBuilder   8  avgt   1526,488 ±   
0,371   ns/op
stringRepeat 

Re: RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java fails

[Stuart Marks]

I agree, this is not a test of locale-sensitive number parsing, it's a test of 
Scanner. Thus I think it's reasonable to keep the focus on the Scanner methods.


To step back a bit, this bug is about fixing the "intermittent" failure of this 
test. This isn't case where the test fails randomly 0.5% of the time. The 
failures are caused by a dependency on the JVM's default locale, which is a 
*hidden global variable*. It's possible to demonstrate a 100% failure rate the 
default locale is set to one for which the test is unprepared. Given this, 
setting the default locale to a known good locale is the right way to fix this test.


(I'm reminded of tests and even production code that has a hidden dependency on 
the VM's default charset. Incorrect assumptions about the default charset can 
cause all kinds of hilarity.)


s'marks

On 3/20/19 2:23 AM, Langer, Christoph wrote:

Hi Goetz,

this test is specifically for java.util.Scanner.

Generally, it could probably be overhauled to make it run with any Locale. 
However, then the input data for scanning will need to be written in the Locale 
that the scanner uses. This is a bit out of scope for me at this point...

But anyway, thanks for your hint 

/christoph


-Original Message-
From: Lindenmaier, Goetz
Sent: Mittwoch, 20. März 2019 10:21
To: Langer, Christoph ; Stuart Marks

Cc: core-libs-dev 
Subject: RE: RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java fails

Hi,

if it is only about parsing floats, why not use
NumberFormat.getInstance().parse(value)?

I did
8205108: [testbug] Fix pattern matching in jstatd tests.
http://hg.openjdk.java.net/jdk-
updates/jdk11u/rev/1637a4e50fc9?revcount=80
some while ago, because we had wrong float parsing on mac in our tests.

... just a hint to think about, you don't need to redo the change ...

Best regards,
   Goetz.



-Original Message-
From: core-libs-dev  On Behalf

Of

Langer, Christoph
Sent: Mittwoch, 20. März 2019 10:10
To: Stuart Marks 
Cc: core-libs-dev 
Subject: [CAUTION] RE: RFR(S): 8172695: (scanner)
java/util/Scanner/ScanTest.java fails

Hi Stuart,

thanks for consulting with Naoto and providing the update.

So I'll run the fix through jdk-submit and our test system at SAP with

Locale.US.

Provided I don't see issues, I'll push it thereafter...

Best regards
Christoph


-Original Message-
From: Stuart Marks 
Sent: Dienstag, 19. März 2019 23:22
To: Langer, Christoph 
Cc: Brian Burkhalter ; core-libs-dev 
Subject: Re: RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java

fails


Hi Christoph,

I spoke with Naoto Sato (internationalization) and he recommends using
Locale.US
instead of Locale.ROOT. (Sorry for having misdirected you.)

The test case in question, parsing float strings, relies on the decimal
separator, which isn't defined in the ROOT locale. Of course it's "." in the

US

locale. Also, the US locale is the only one that's guaranteed to be

available;

see Locale::getAvailableLocales.

So I think your current changeset will be fine if Locale.ROOT is replaced

with

Locale.US. Assuming it passes test runs. :-)

s'marks


On 3/19/19 1:46 AM, Langer, Christoph wrote:

Hi Stuart,

thanks for your analysis of this.

So you are suggesting to use Locale.ROOT for this test unconditionally? I

agree 


The test coding as it is right now is not ready to support arbitrary locales,

as

the comment already suggests. Generally, the test probably needs some
overhaul. The scanned Strings should for instance use the Locale's

decimal

separators instead of hard coding ".". Maybe there are other

shortcomings.

But as long as this is not done, using Locale ROOT seems to be safe for all
possible scenarios.


I've updated the webrev:

http://cr.openjdk.java.net/~clanger/webrevs/8172695.1/


Are you ok with that? Brian also?

Thanks
Christoph



-Original Message-
From: Stuart Marks 
Sent: Dienstag, 19. März 2019 00:44
To: Langer, Christoph 
Cc: Brian Burkhalter ; core-libs-dev


libs-...@openjdk.java.net>
Subject: Re: RFR(S): 8172695: (scanner)

java/util/Scanner/ScanTest.java

fails


Hi Christoph,

After looking at this for a little bit I realized that it's not necessary to
have a system particular default locale in order to get the test case to

fail.

It's possible create a locale that causes test case to fail, like so:

   Locale.setDefault(new Locale("en", "DE"))
   Scanner sc = new Scanner("\u0f23.\u0f27");
   sc.nextFloat();
   ^ throws InputMismatchException

(This is because using "DE" as the locale's country causes texpects ,

instead of

. as the decimal separator.)

The test already saves the current default locale and attempts to find

a

known
good locale for running the tests. The difficulty seems to be finding a

"known

good" locale. The current check for the language of English doesn't

work if

the
default is en_DE.

Instead, I'd suggest we use the root locale Locale.ROOT as the known

good

locale
for running the tests. We 

Re: RFR: docs JDK-8220261: fix headings in java.base

[Jonathan Gibbons]

Thanks.

-- Jon


On 03/20/2019 03:16 PM, Lance Andersen wrote:

Hi Jon,

I went through the patch and it looks good (matches what i did for 
java.sql.rowset and java.naming :-) )


On Mar 20, 2019, at 5:50 PM, Jonathan Gibbons 
mailto:jonathan.gibb...@oracle.com>> wrote:


JDK-8220248




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: docs JDK-8220261: fix headings in java.base

[Lance Andersen]

Hi Jon,

I went through the patch and it looks good (matches what i did for 
java.sql.rowset and java.naming :-) )

> On Mar 20, 2019, at 5:50 PM, Jonathan Gibbons  
> wrote:
> 
> JDK-8220248

 
  

 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: docs JDK-8220261: fix headings in java.base

[Jonathan Gibbons]

Please review a medium-sized but conceptually simple patch to fix most 
of the headings in the generated documentation for java.base, as 
described in [1]. This does not address the headings in 
java.util.concurrent which are reported separately, in JDK-8220248.


The intent is to ensure that the headings in the generated pages are 
well-structured, with no gaps, and correct implicit nesting. Generally, 
the changes fall into three kinds:


 * Headings in doc-comments for modules, packages and types start at 
 * Headings in doc comments for constructors, fields, methods and other
   members start at 
 * Headings in package.html and doc-files/*.html start at 

The headings have been verified with the doccheck utility.

We cannot re-enable doclint accessibility checking for java.base until 
JDK-8220248 has also been fixed.


-- Jon

JBS: https://bugs.openjdk.java.net/browse/JDK-8220261
Webrev: http://cr.openjdk.java.net/~jjg/8220261/webrev.00/index.html

[1]: https://mail.openjdk.java.net/pipermail/jdk-dev/2019-March/002671.html

RFR 8211941 : Enable checking/ignoring of non-conforming Class-Path entries

[Brent Christian]

Hi,

JDK-8216401[1][2] added code to improve JAR spec conformance (WRT 
relative URLs in the Class-Path attribute), while maintaining some key 
compatibility aspects (entries using "file:" URLs are not relative, but 
are used by some libraries).


This code was disabled by default, but we would now like to enable it.

Bug:
https://bugs.openjdk.java.net/browse/JDK-8211941
Webrev:
http://cr.openjdk.java.net/~bchristi/8211941/webrev-06/

This change includes a new system property 
("jdk.net.URLClassPath.showIgnoredClassPathEntries") that can be set to 
aid in debugging JAR Class-Paths by printing entries that are ignored.


Thanks,
-Brent

1. https://bugs.openjdk.java.net/browse/JDK-8216401

2. 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-January/057868.html

Re: java.lang.CharSequence#compare - lexicographical ordering of custom Comparable CharSequence(s)

[Florian Weimer]

* Jason Mehrens:

> The initial implementation was only optimized to call into
> String.compare if the arguments were string [1].  I proposed the
> current code a general form to catch java.nio.CharBuffer and any new
> JDK implementations of CharSequence + Comparable.
>
> Naively, I lean towards "- CharSequence interface specification
> should be extended to require Comparable CharSeqeunces to implement
> lexicographical ordering".

There are natural lexicographical orderings, unfortunately.  One is
according to UCS-2 (currently implemented by String and specified by
CharSequence.compare), and the other one according UTF-16, for
compatibility with the lexicographical ordering according to Unicode
codepoints (which is also compatible with lexicographical ordering of
bytes after UTF-8 encoding).

There must be implementations of CharSequence out there which
implement UTF-16 ordering.

Re: [PATCH] Some improvements for java.lang.Class

[Сергей Цыпанов]

I had a brief conversation with Brian Goetz which has left off the mailing list 
for some reason. Here's the text:
---
Brian:

These enhancements seem reasonable; these are exactly the cases that 
String::repeat was intended for.  (This is a “is this a reasonable idea” 
review, not a code review.). 
Have you looked for other places where similar transformations could be done?

---
Me:

Hello,
after I had realized that looped StringBuilder::append calls can sometimes be 
replaced with String::repeat I requested corresponding inspection in IDEA issue 
tracker.
Using the inspection I’ve found 129 similar warnings in jdk13 repo.
Some of them are very promising, e.g. BigDecimal:: toPlainString where 
currently we have

> int trailingZeros = checkScaleNonZero((-(long)scale));
> StringBuilder buf;
> if(intCompact!=INFLATED) {
> buf = new StringBuilder(20+trailingZeros);
> buf.append(intCompact);
> } else {
> String str = intVal.toString();
> buf = new StringBuilder(str.length()+trailingZeros);
> buf.append(str);
> }
> for (int i = 0; i < trailingZeros; i++) {
> buf.append('0');
> }
> return buf.toString();

which in fact can be simplified to

> int trailingZeros = checkScaleNonZero((-(long)scale));
> if(intCompact!=INFLATED) {
> return intCompact + "0".repeat(trailingZeros);
> } else {
> return intVal.toString() + "0".repeat(trailingZeros);
> }

The second solution is not only shorter and more simple but it likely to be 
significantly faster and memory-saving.
BTW, your reply to original message for some reason is missing from web-view.

---
Brian:

Cool.  Note that replacing append() calls with repeat is not necessarily a win 
for anything other than code compactness; String::repeat will create a new 
string, which will then get appended to another StrinBuilder.  So when used 
correctly (such as presized StringBuilders) appending in a loop is probably 
just as fast (look at the implementation of String::repeat.).

> if(intCompact!=INFLATED) {
> return intCompact + "0".repeat(trailingZeros);
> } else {
> return intVal.toString() + "0".repeat(trailingZeros);
> }

Which can be further simplified to

>((intCompact != INFLATED) ? intCompact : intVal.toString) + 
> “0”.repeat(trailingZeros);
 
The second solution is not only shorter and more simple but it likely to be 
significantly faster and memory-saving.
I like short and simple.  I am questioning the “faster and memory saving.”  
That should be validated.   

---
Me:

I think optimizations provided by JEP 280 allow compiler to optimize String 
concatenation executed with '+' by using StringBuilder of exact size (when the 
size of all components is known, like in our case).

I've checked this with benchmark

> @State(Scope.Thread)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
> public class ConcatBenchmark {
> 
> @Param({"1", "2", "5", "8"})
> private int trailingZeros;
> 
> private final long intCompact = 1L;
> 
> @Benchmark
> public String stringBuilder() {
> StringBuilder buf;
> buf = new StringBuilder(20 + trailingZeros);
> buf.append(intCompact);
> for (int i = 0; i < trailingZeros; i++) {
> buf.append('0');
> }
> return buf.toString();
> }
> @Benchmark
> public String stringRepeat() {
> return intCompact + "0".repeat(trailingZeros);
> }
> }

Results:
trailingZeros  Mode  Cnt Score 
Error   Units
stringBuilder   1  avgt   1517,683 ±   
0,664   ns/op
stringRepeat1  avgt   15 9,052 ±   
0,042   ns/op

stringBuilder   2  avgt   1519,053 ±   
1,206   ns/op
stringRepeat2  avgt   1515,864 ±   
1,331   ns/op

stringBuilder   5  avgt   1525,839 ±   
2,256   ns/op
stringRepeat5  avgt   1519,290 ±   
1,685   ns/op

stringBuilder   8  avgt   1526,488 ±   
0,371   ns/op
stringRepeat8  avgt   1519,420 ±   
1,593   ns/op

stringBuilder:·gc.alloc.rate.norm   1  avgt   1588,000 ±   
0,001B/op
stringRepeat:·gc.alloc.rate.norm1  avgt   1548,000 ±   
0,001B/op

stringBuilder:·gc.alloc.rate.norm   2  avgt   1588,000 ±   
0,001B/op
stringRepeat:·gc.alloc.rate.norm2  avgt   1572,000 ±   
0,001B/op

stringBuilder:·gc.alloc.rate.norm   5  avgt   1596,000 ±   
0,001B/op
stringRepeat:·gc.alloc.rate.norm5  avgt   1572,000 ±   
0,001B/op

RE: RFR 8213031: (zipfs) Add support for POSIX file permissions

[Langer, Christoph]

Hi Alan,

I now found time to get back to the POSIX file permissions in zipfs. The goal 
would be to get it done for JDK13 

I went through your mail in details, please see my comments:

> 1. By default, a zip file system will have no support for the "posix"
> and "owner" views of file attributes, this means the following will fail
> with UOE:
> 
>    PosixFileAttributes attrs = Files.readAttributes(file,
> PosixFileAttributes.class);
> 
> and the zip file system's supportedFileAttributView method will not
> include "posix".

Correct.

> 2. Creating a zip file system with configuration parameter XXX set to
> "true" will create the zip file system that supports
> PosixFileAttributeView (and FileOwnerAttributeView). The current patch
> names the configuration parameter "posix". That name might be misleading
> as a configuration parameter as it conveys more than it actually does.
> Something like "enablePosixFileAttributes" clear conveys that it enables
> support for POSIX file attribute but might be a better wordy.

OK. I've changed to "enablePosixFileAttributes" in my new webrev.

> The value in the configuration map is documented as Boolean but I think
> we are allowing it to be the string representation of a Boolean, meaning
> it can be "true" or "false" like we have with the "create" parameter.

I fixed the doc.

> 3. The map of configurations parameters can optionally include the
> parameters "defaultOwner", "defaultGroup", and "defaultPermissions" with
> the default owner/group/permissions to return. These names seem okay to
> me.
> 
> For the owner/group, the parameter type can be
> UserPrincipal/GroupPrincipal or strings. If the value is a String then
> we need to decide if there is any validation. If I read the current
> patch correctly then it doesn't do any validation - that might be okay
> but minimally maybe we should disallow the empty string.

I added checking for the empty string in my new webrev.

> If the defaultXXX parameters aren't present then the zip file
> owner/group would be a sensible default. If the zip file is on a file
> store that supports FileOwnerAttributeView then we've got the owner. If
> the file store supports PosixFileAttributeView then we have a group
> owner. If PosixFileAttributeView is not supported then using the owner
> as the group is okay. I see the current patch as a fallback to fallback
> to "" but I'd prefer not have that because it will be
> very confusing to diagnose if there are issues.

Ok, but what shall we do then in the case of cases? That is, if the file store 
does not support the FileOwnerAttributeView or security permissions don't allow 
us to access them? Shall we fail, e.g. throw an IOException upon attempting to 
create a zipfs?

> For defaultPermissions, the value is a Set or the
> string representation. In the implementation, initPermissions can be
> changed to use PosixFilePermissions.fromString as it does the
> translation that we need.

I don't see where it still needs to be changed. It's using 
PosixFilePermissions.fromString already.

> 4. As an alternative to the type safe access that PosixFileAttributeView
> provides, the proposal is to document the "zip" view of file attributes.
> Initially, it will admit to one file attribute for the permissions. The
> current patch uses the name "storedPermissions" but it might be simpler
> to use "permissions" so that "zip:permissions" and "posix:permissions"
> are the same when the zip entry has permissions.

There's both, "storedPermissions" and "permissions. "storedPermissions" was 
chosen deliberately and has a different semantical meaning than "permissions". 
The former one will reflect the actual permission value on a zip entry, e.g. it 
can be null. The latter one will use the default, in case no permission 
information is associated with the zip entry.

> With this support you can write code like this:
>      Set perms = (Set)
> Files.getAttribute(file, "zip:permissions");
> 
> The cast is ugly of course but that's the consequence of not exposing
> ZipFileAttributes in the API so there is no type token to specify on the
> RHS.

That should work with the proposed patch.

> Documenting the "zip" view brings up a few issues, e.g. will
> Files.readAttributes(file, "zip:*") reveal more than "permissions". For
> this API it is okay for the map to attributes beyond the specified list.

I guess we should reveal all attributes of a zip view in the documentation.

> 5. There will be no support for specifying as POSIX FileAttribute to
> createFile or createDirectory to atomically set the permissions when
> creating a new file/directory, UOE will thrown as is is now.

Nope. It will work with the current state of the patch.

Here is an updated webrev: 
http://cr.openjdk.java.net/~clanger/webrevs/8213031.8/

What's next?

Thanks
Christoph

Re: [11u]: RFR CSR backport: JDK-8220362: Add "POSIX_SPAWN" as valid value to jdk.lang.Process.launchMechanism on Linux

[Andrew Haley]

On 3/20/19 8:02 AM, Langer, Christoph wrote:
> Hi Andrew,
> 
>> Thanks. I get that, I'm just questioning the need to backport it to 11.
>> No matter, I've approved it now.
> 
> What do you mean by that? The bug JDK-8220362 [0] still has jdk11u-fix-no.

No it doesn't. JDK-8212828 still had a jdk11u-fix-no, so I fixed that.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. 
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

RE: RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java fails

[Langer, Christoph]

Hi Goetz,

this test is specifically for java.util.Scanner.

Generally, it could probably be overhauled to make it run with any Locale. 
However, then the input data for scanning will need to be written in the Locale 
that the scanner uses. This is a bit out of scope for me at this point...

But anyway, thanks for your hint 

/christoph

> -Original Message-
> From: Lindenmaier, Goetz
> Sent: Mittwoch, 20. März 2019 10:21
> To: Langer, Christoph ; Stuart Marks
> 
> Cc: core-libs-dev 
> Subject: RE: RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java fails
> 
> Hi,
> 
> if it is only about parsing floats, why not use
> NumberFormat.getInstance().parse(value)?
> 
> I did
> 8205108: [testbug] Fix pattern matching in jstatd tests.
> http://hg.openjdk.java.net/jdk-
> updates/jdk11u/rev/1637a4e50fc9?revcount=80
> some while ago, because we had wrong float parsing on mac in our tests.
> 
> ... just a hint to think about, you don't need to redo the change ...
> 
> Best regards,
>   Goetz.
> 
> 
> > -Original Message-
> > From: core-libs-dev  On Behalf
> Of
> > Langer, Christoph
> > Sent: Mittwoch, 20. März 2019 10:10
> > To: Stuart Marks 
> > Cc: core-libs-dev 
> > Subject: [CAUTION] RE: RFR(S): 8172695: (scanner)
> > java/util/Scanner/ScanTest.java fails
> >
> > Hi Stuart,
> >
> > thanks for consulting with Naoto and providing the update.
> >
> > So I'll run the fix through jdk-submit and our test system at SAP with
> Locale.US.
> > Provided I don't see issues, I'll push it thereafter...
> >
> > Best regards
> > Christoph
> >
> > > -Original Message-
> > > From: Stuart Marks 
> > > Sent: Dienstag, 19. März 2019 23:22
> > > To: Langer, Christoph 
> > > Cc: Brian Burkhalter ; core-libs-dev  > > libs-...@openjdk.java.net>
> > > Subject: Re: RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java
> fails
> > >
> > > Hi Christoph,
> > >
> > > I spoke with Naoto Sato (internationalization) and he recommends using
> > > Locale.US
> > > instead of Locale.ROOT. (Sorry for having misdirected you.)
> > >
> > > The test case in question, parsing float strings, relies on the decimal
> > > separator, which isn't defined in the ROOT locale. Of course it's "." in 
> > > the
> US
> > > locale. Also, the US locale is the only one that's guaranteed to be
> available;
> > > see Locale::getAvailableLocales.
> > >
> > > So I think your current changeset will be fine if Locale.ROOT is replaced
> with
> > > Locale.US. Assuming it passes test runs. :-)
> > >
> > > s'marks
> > >
> > >
> > > On 3/19/19 1:46 AM, Langer, Christoph wrote:
> > > > Hi Stuart,
> > > >
> > > > thanks for your analysis of this.
> > > >
> > > > So you are suggesting to use Locale.ROOT for this test unconditionally? 
> > > > I
> > > agree 
> > > >
> > > > The test coding as it is right now is not ready to support arbitrary 
> > > > locales,
> as
> > > the comment already suggests. Generally, the test probably needs some
> > > overhaul. The scanned Strings should for instance use the Locale's
> decimal
> > > separators instead of hard coding ".". Maybe there are other
> shortcomings.
> > > But as long as this is not done, using Locale ROOT seems to be safe for 
> > > all
> > > possible scenarios.
> > > >
> > > > I've updated the webrev:
> > > http://cr.openjdk.java.net/~clanger/webrevs/8172695.1/
> > > >
> > > > Are you ok with that? Brian also?
> > > >
> > > > Thanks
> > > > Christoph
> > > >
> > > >
> > > >> -Original Message-
> > > >> From: Stuart Marks 
> > > >> Sent: Dienstag, 19. März 2019 00:44
> > > >> To: Langer, Christoph 
> > > >> Cc: Brian Burkhalter ; core-libs-dev
>  > > >> libs-...@openjdk.java.net>
> > > >> Subject: Re: RFR(S): 8172695: (scanner)
> java/util/Scanner/ScanTest.java
> > > fails
> > > >>
> > > >> Hi Christoph,
> > > >>
> > > >> After looking at this for a little bit I realized that it's not 
> > > >> necessary to
> > > >> have a system particular default locale in order to get the test case 
> > > >> to
> fail.
> > > >> It's possible create a locale that causes test case to fail, like so:
> > > >>
> > > >>   Locale.setDefault(new Locale("en", "DE"))
> > > >>   Scanner sc = new Scanner("\u0f23.\u0f27");
> > > >>   sc.nextFloat();
> > > >>   ^ throws InputMismatchException
> > > >>
> > > >> (This is because using "DE" as the locale's country causes texpects ,
> > > instead of
> > > >> . as the decimal separator.)
> > > >>
> > > >> The test already saves the current default locale and attempts to find
> a
> > > >> known
> > > >> good locale for running the tests. The difficulty seems to be finding a
> > > "known
> > > >> good" locale. The current check for the language of English doesn't
> work if
> > > >> the
> > > >> default is en_DE.
> > > >>
> > > >> Instead, I'd suggest we use the root locale Locale.ROOT as the known
> > > good
> > > >> locale
> > > >> for running the tests. We could then do something like this:
> > > >>
> > > >>   Locale reservedLocale = 

RE: RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java fails

[Lindenmaier, Goetz]

Hi,

if it is only about parsing floats, why not use
NumberFormat.getInstance().parse(value)?

I did 
8205108: [testbug] Fix pattern matching in jstatd tests.
http://hg.openjdk.java.net/jdk-updates/jdk11u/rev/1637a4e50fc9?revcount=80
some while ago, because we had wrong float parsing on mac in our tests.

... just a hint to think about, you don't need to redo the change ...

Best regards,
  Goetz.


> -Original Message-
> From: core-libs-dev  On Behalf Of
> Langer, Christoph
> Sent: Mittwoch, 20. März 2019 10:10
> To: Stuart Marks 
> Cc: core-libs-dev 
> Subject: [CAUTION] RE: RFR(S): 8172695: (scanner)
> java/util/Scanner/ScanTest.java fails
> 
> Hi Stuart,
> 
> thanks for consulting with Naoto and providing the update.
> 
> So I'll run the fix through jdk-submit and our test system at SAP with 
> Locale.US.
> Provided I don't see issues, I'll push it thereafter...
> 
> Best regards
> Christoph
> 
> > -Original Message-
> > From: Stuart Marks 
> > Sent: Dienstag, 19. März 2019 23:22
> > To: Langer, Christoph 
> > Cc: Brian Burkhalter ; core-libs-dev  > libs-...@openjdk.java.net>
> > Subject: Re: RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java 
> > fails
> >
> > Hi Christoph,
> >
> > I spoke with Naoto Sato (internationalization) and he recommends using
> > Locale.US
> > instead of Locale.ROOT. (Sorry for having misdirected you.)
> >
> > The test case in question, parsing float strings, relies on the decimal
> > separator, which isn't defined in the ROOT locale. Of course it's "." in 
> > the US
> > locale. Also, the US locale is the only one that's guaranteed to be 
> > available;
> > see Locale::getAvailableLocales.
> >
> > So I think your current changeset will be fine if Locale.ROOT is replaced 
> > with
> > Locale.US. Assuming it passes test runs. :-)
> >
> > s'marks
> >
> >
> > On 3/19/19 1:46 AM, Langer, Christoph wrote:
> > > Hi Stuart,
> > >
> > > thanks for your analysis of this.
> > >
> > > So you are suggesting to use Locale.ROOT for this test unconditionally? I
> > agree 
> > >
> > > The test coding as it is right now is not ready to support arbitrary 
> > > locales, as
> > the comment already suggests. Generally, the test probably needs some
> > overhaul. The scanned Strings should for instance use the Locale's decimal
> > separators instead of hard coding ".". Maybe there are other shortcomings.
> > But as long as this is not done, using Locale ROOT seems to be safe for all
> > possible scenarios.
> > >
> > > I've updated the webrev:
> > http://cr.openjdk.java.net/~clanger/webrevs/8172695.1/
> > >
> > > Are you ok with that? Brian also?
> > >
> > > Thanks
> > > Christoph
> > >
> > >
> > >> -Original Message-
> > >> From: Stuart Marks 
> > >> Sent: Dienstag, 19. März 2019 00:44
> > >> To: Langer, Christoph 
> > >> Cc: Brian Burkhalter ; core-libs-dev  > >> libs-...@openjdk.java.net>
> > >> Subject: Re: RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java
> > fails
> > >>
> > >> Hi Christoph,
> > >>
> > >> After looking at this for a little bit I realized that it's not 
> > >> necessary to
> > >> have a system particular default locale in order to get the test case to 
> > >> fail.
> > >> It's possible create a locale that causes test case to fail, like so:
> > >>
> > >>   Locale.setDefault(new Locale("en", "DE"))
> > >>   Scanner sc = new Scanner("\u0f23.\u0f27");
> > >>   sc.nextFloat();
> > >>   ^ throws InputMismatchException
> > >>
> > >> (This is because using "DE" as the locale's country causes texpects ,
> > instead of
> > >> . as the decimal separator.)
> > >>
> > >> The test already saves the current default locale and attempts to find a
> > >> known
> > >> good locale for running the tests. The difficulty seems to be finding a
> > "known
> > >> good" locale. The current check for the language of English doesn't work 
> > >> if
> > >> the
> > >> default is en_DE.
> > >>
> > >> Instead, I'd suggest we use the root locale Locale.ROOT as the known
> > good
> > >> locale
> > >> for running the tests. We could then do something like this:
> > >>
> > >>   Locale reservedLocale = Locale.getDefault();
> > >>   try {
> > >>   Locale.setDefault(Locale.ROOT);
> > >>   // run all the tests
> > >>   } finally {
> > >>   // restore the default locale
> > >>   Locale.setDefault(reservedLocale);
> > >>   }
> > >>
> > >> and dispense with the logic of searching the existing locales for one 
> > >> that
> > we
> > >> think is suitable.
> > >>
> > >> (Actually, since the test is being run in /othervm mode, it's not clear 
> > >> to me
> > >> that restoring the previous default locale is necessary. But that's kind 
> > >> of a
> > >> separate discussion. I'm ok with leaving the save/restore in place.)
> > >>
> > >> s'marks
> > >>
> > >> On 3/18/19 10:57 AM, Brian Burkhalter wrote:
> > >>> Hi Christoph,
> > >>>
> > >>> Not a locale expert here so you probably need to hear from someone
> > else,

RE: RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java fails

[Langer, Christoph]

Hi Stuart,

thanks for consulting with Naoto and providing the update.

So I'll run the fix through jdk-submit and our test system at SAP with 
Locale.US. Provided I don't see issues, I'll push it thereafter...

Best regards
Christoph

> -Original Message-
> From: Stuart Marks 
> Sent: Dienstag, 19. März 2019 23:22
> To: Langer, Christoph 
> Cc: Brian Burkhalter ; core-libs-dev  libs-...@openjdk.java.net>
> Subject: Re: RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java fails
> 
> Hi Christoph,
> 
> I spoke with Naoto Sato (internationalization) and he recommends using
> Locale.US
> instead of Locale.ROOT. (Sorry for having misdirected you.)
> 
> The test case in question, parsing float strings, relies on the decimal
> separator, which isn't defined in the ROOT locale. Of course it's "." in the 
> US
> locale. Also, the US locale is the only one that's guaranteed to be available;
> see Locale::getAvailableLocales.
> 
> So I think your current changeset will be fine if Locale.ROOT is replaced with
> Locale.US. Assuming it passes test runs. :-)
> 
> s'marks
> 
> 
> On 3/19/19 1:46 AM, Langer, Christoph wrote:
> > Hi Stuart,
> >
> > thanks for your analysis of this.
> >
> > So you are suggesting to use Locale.ROOT for this test unconditionally? I
> agree 
> >
> > The test coding as it is right now is not ready to support arbitrary 
> > locales, as
> the comment already suggests. Generally, the test probably needs some
> overhaul. The scanned Strings should for instance use the Locale's decimal
> separators instead of hard coding ".". Maybe there are other shortcomings.
> But as long as this is not done, using Locale ROOT seems to be safe for all
> possible scenarios.
> >
> > I've updated the webrev:
> http://cr.openjdk.java.net/~clanger/webrevs/8172695.1/
> >
> > Are you ok with that? Brian also?
> >
> > Thanks
> > Christoph
> >
> >
> >> -Original Message-
> >> From: Stuart Marks 
> >> Sent: Dienstag, 19. März 2019 00:44
> >> To: Langer, Christoph 
> >> Cc: Brian Burkhalter ; core-libs-dev  >> libs-...@openjdk.java.net>
> >> Subject: Re: RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java
> fails
> >>
> >> Hi Christoph,
> >>
> >> After looking at this for a little bit I realized that it's not necessary 
> >> to
> >> have a system particular default locale in order to get the test case to 
> >> fail.
> >> It's possible create a locale that causes test case to fail, like so:
> >>
> >>   Locale.setDefault(new Locale("en", "DE"))
> >>   Scanner sc = new Scanner("\u0f23.\u0f27");
> >>   sc.nextFloat();
> >>   ^ throws InputMismatchException
> >>
> >> (This is because using "DE" as the locale's country causes texpects ,
> instead of
> >> . as the decimal separator.)
> >>
> >> The test already saves the current default locale and attempts to find a
> >> known
> >> good locale for running the tests. The difficulty seems to be finding a
> "known
> >> good" locale. The current check for the language of English doesn't work if
> >> the
> >> default is en_DE.
> >>
> >> Instead, I'd suggest we use the root locale Locale.ROOT as the known
> good
> >> locale
> >> for running the tests. We could then do something like this:
> >>
> >>   Locale reservedLocale = Locale.getDefault();
> >>   try {
> >>   Locale.setDefault(Locale.ROOT);
> >>   // run all the tests
> >>   } finally {
> >>   // restore the default locale
> >>   Locale.setDefault(reservedLocale);
> >>   }
> >>
> >> and dispense with the logic of searching the existing locales for one that
> we
> >> think is suitable.
> >>
> >> (Actually, since the test is being run in /othervm mode, it's not clear to 
> >> me
> >> that restoring the previous default locale is necessary. But that's kind 
> >> of a
> >> separate discussion. I'm ok with leaving the save/restore in place.)
> >>
> >> s'marks
> >>
> >> On 3/18/19 10:57 AM, Brian Burkhalter wrote:
> >>> Hi Christoph,
> >>>
> >>> Not a locale expert here so you probably need to hear from someone
> else,
> >> but this looks like a reasonable approach to me. One minor comment is to
> >> perhaps use format() at lines 67-68 instead of println().
> >>>
> >>> Thanks,
> >>>
> >>> Brian
> >>>
>  On Mar 18, 2019, at 7:51 AM, Langer, Christoph
> >>  wrote:
> 
>  please review a fix for 8172695, where java/util/Scanner/ScanTest.java
> >> fails. It reproducibly fails on one of our Mac servers, where the locale is
> set to
> >> en_DE.
> 
>  The test in its current form checks if the test VM is run with a
> supported
> >> default Locale by just checking the language to be one of “en”, “zh”, “ko”,
> >> “ja”. But it turns out, that also the country part seems to be important. I
> >> suggest to change the test case to test whether the VM’s default locale is
> >> one of a restricted set of locales and if it isn’t, it shall try to run 
> >> with
> >> “ENGLISH”.
> 
>  I tested the set of US, UK, CHINA, KOREA, JAPAN 

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

[Lindenmaier, Goetz]

Hi Mandy,

> Sorry I have been busy on other time-critical things.  I will
> get to it hopefully next week.
No problem, I have been improving the text slightly
anyways.
I also pushed my change and a follow up needed to the sandbox 
repos so it can be played with.

Should I move the JEP to status 'submitted'?

Best regards,
  Goetz




> 
> Mandy
> 
> On 3/15/19 3:55 AM, Lindenmaier, Goetz wrote:
> > Hi everybody, Mark,
> >
> > I followed your advice and created a JEP:
> > https://bugs.openjdk.java.net/browse/JDK-8220715
> >
> > Please point me to things I need to improve formally, this is my first
> > JEP. Also feel free to fix the administrative information in the
> > Jira issue if it is wrong.
> >
> > And, naturally, you're welcome to discuss the topic!
> >
> > Best regards,
> >Goetz.
> >
> >> -Original Message-
> >> From: mark.reinh...@oracle.com 
> >> Sent: Donnerstag, 14. März 2019 22:38
> >> To: maurizio.cimadam...@oracle.com; Lindenmaier, Goetz
> >> 
> >> Cc: mandy.ch...@oracle.com; roger.ri...@oracle.com; core-libs-
> >> d...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net
> >> Subject: Re: RFR(L): 8218628: Add detailed message to NullPointerException
> >> describing what is null.
> >>
> >> 2019/3/14 8:00:20 -0700, maurizio.cimadam...@oracle.com:
> >>> I second what Mandy says.
> >>>
> >>> First let me start by saying that this enhancement will be a great
> >>> addition to our platform; back in the days when I was teaching some Java
> >>> classes at the university, I was very aware of how hard it is to
> >>> diagnose a NPE for someone novel to Java programming.
> >>
> >> Agreed!
> >>
> >>> ...
> >>>
> >>> I also think that the design space for such an enhancement is non
> >>> trivial, and would best be explored (and captured!) in a medium that is
> >>> something other than a patch. ...
> >>
> >> Agreed, also.
> >>
> >> Goetz -- if, per Mandy’s suggestion, you’re going to write something
> >> up using the JEP template, might I suggest that you then submit it as
> >> an actual JEP?  Giving visibility to, and recording, such design-space
> >> explorations is one of the primary goals of the JEP process.
> >>
> >> - Mark

Re: [11u]: RFR CSR backport: JDK-8220362: Add "POSIX_SPAWN" as valid value to jdk.lang.Process.launchMechanism on Linux

[Thomas Stüfe]

On Tue, Mar 19, 2019 at 5:10 PM Martin Buchholz  wrote:

> I'm the one who introduced vfork for process spawning, and I support
> Thomas' plan to switch to posix_spawn.
> (although I have not seen a crash attributed to using vfork on Linux)
>

Thanks Martin. I think for a long time vfork was the best alternative.

I remember seeing one or two cases in the last five years which could have
been caused by vfork. In both cases, in a fork-heavy environment with many
short-lived processes, a broken script killed the wrong pid - a just
spawned child - and pid's father disappeared at the same time without a
trace.

Cheers, Thomas

RE: [11u]: RFR CSR backport: JDK-8220362: Add "POSIX_SPAWN" as valid value to jdk.lang.Process.launchMechanism on Linux

[Langer, Christoph]

Hi Andrew,

> Thanks. I get that, I'm just questioning the need to backport it to 11.
> No matter, I've approved it now.

What do you mean by that? The bug JDK-8220362 [0] still has jdk11u-fix-no.

In case you approve JDK-8220362, I'll set the CSR (JDK-8220362 [1]) to 
finalized, referring to your approval.

Thanks
Christoph

[0] https://bugs.openjdk.java.net/browse/JDK-8212828
[1] https://bugs.openjdk.java.net/browse/JDK-8220362