Re: RFR 8220684 : Process.waitFor(long, TimeUnit) can return false for a process that exited within the timeout

2019-03-14 Thread Ivan Gerasimov

Hi David!


On 3/14/19 5:28 PM, David Holmes wrote:

Hi Ivan,

On 15/03/2019 5:49 am, Ivan Gerasimov wrote:

Hello!

The default implementation of Process.waitFor(long, TimeUnit) does 
not check if the process has exited after the last portion of the 
timeout has expired.


Please clarify. There is always a race between detecting a timeout and 
the process actually terminating. If the process actually terminates 
while you're deciding to report a timeout that seems just  an 
acceptable possibility. No matter what you do the process could 
terminate just after you decided to report the timeout.




Current code for waitFor(...) is

 212 do {
 213 try {
 214 exitValue();
 215 return true;
 216 } catch(IllegalThreadStateException ex) {
 217 if (rem > 0)
 218 Thread.sleep(
 219 Math.min(TimeUnit.NANOSECONDS.toMillis(rem) + 1, 100));
 220 }
 221 rem = unit.toNanos(timeout) - (System.nanoTime() - 
startTime);

 222 } while (rem > 0);
 223 return false;

So, if the loop has just processed the last 100 ms portion of the 
timeout, it ends right away, without checking if the process has exited 
during this period.


Not a big deal of course, but it is more accurate to check if the 
process has exited at the *end* of the specified timeout, and not 100 
milliseconds before the end.


With kind regards,
Ivan



David
-

JDK has two implementations of Process (for Unix and Windows) and 
they both override waitFor(), so it's not an issue for them.


Still, it is better to provide a more accurate default implementation.

I'm not quite certain the regression test needs to be included in the 
fix.  The test does demonstrate the issue with the unfixed JDK and 
passed Okay on all tested platforms in Mach5. Yet, I suspect the test 
can still show false negative results, as there are no guaranties 
that even such simple application as `true` will finish in 100 ms.
I can tag the test as @ignored with a comment, or simply remove it 
from the fix.


BUGURL: https://bugs.openjdk.java.net/browse/JDK-8220684
WEBREV: http://cr.openjdk.java.net/~igerasim/8220684/00/webrev/

Thanks in advance for reviewing!





--
With kind regards,
Ivan Gerasimov



Re: RFR 6307456 : UnixFileSystem_md.c use of chmod() and access() should handle EINTR signal appropriately (unix)

2019-03-14 Thread Ivan Gerasimov

Thank you David!


On 3/14/19 4:48 PM, David Holmes wrote:

Hi Ivan,

This is an "ancient" bug that you are fixing. I don't think it is valid.

On 15/03/2019 3:29 am, Ivan Gerasimov wrote:

Hello!

Not all the man pages agree that chmod, access and statvfs64 can be 
interrupted, but at least on some platforms they are allowed to fail 
with EINTR:  chmod(2) on MacOS, access(2) on Solaris and statvfs(3) 
on Linux.


So, it would be more accurate to wrap these up into a RESTARTABLE loop.


When Java threads are created, or native threads attach to the VM to 
become Java threads, they all get a very specific signal-mask to block 
most (non synchronous) signals. The signals that we install handlers 
for in the VM are also configured with SA_RESTART. So unless 
specifically specified as not honouring SA_RESTART we should not need 
the RESTARTABLE loop.



But isn't it possible to install a custom signal handler through JNI, 
omitting SA_RESTART flag?


So I'm not clear exactly what signals we need to be guarding against 
here, or whether this indicates some kind of (historic) mismatch 
between the library and VM code?


grep shows that RESTARTABLE macro and its variants are used throughout 
hotspot and jdk code.
If it were possible to guarantee that no syscalls are ever interrupted, 
it would surely be much cleaner to remove all these wrappers and loops.


With kind regards,
Ivan


Thanks,
David
-

Also, java_io_UnixFileSystem_list was tiny-optimized to avoid 
unnecessary reallocation.


Would you please help review the fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-6307456
WEBREV: http://cr.openjdk.java.net/~igerasim/6307456/00/webrev/

Thanks in advance!





--
With kind regards,
Ivan Gerasimov



Re: RFR(S) : 8219139 : move hotspot tests from test/jdk/vm

2019-03-14 Thread David Holmes

Hi Igor,

This all seems fine to me.

Thanks,
David

On 15/03/2019 7:38 am, Igor Ignatyev wrote:

Hi Misha,

thanks for your suggestions, I have moved all runtime tests into 
subdirectories. here is the updated webrev: 
http://cr.openjdk.java.net/~iignatyev//8219139/webrev.01/index.html


Thanks,
-- Igor

On Mar 4, 2019, at 1:57 PM, mikhailo.seledt...@oracle.com 
 wrote:


Hi Igor,

  Sorry it took a while to get back to you on this one. See my comment 
below



On 2/22/19 10:35 AM,mikhailo.seledt...@oracle.com 
wrote:
Overall the change looks good; thank you Igor for doing this. I have 
couple of comments:


- I am in favor of the approach where we move tests first under 
corresponding sub-component directories in
test/hotspot sub-tree, and then figure out whether to keep them. Even 
though in general I am in favor
of removing tests that are obsolete or of questionable value, this 
requires time, consideration and discussions.
Hence, I recommend filing an RFE for that, which can be addressed 
after the migration.


- Runtime normally avoids tests directly in test/hotspot/jtreg/runtime
The tests should go into underlying sub-directories which best match 
functional area or topic of that test.
In most cases you did it in your change, but there are several tests 
that your change places directly under

test/hotspot/jtreg/runtime/:

ExplicitArithmeticCheck.java
MonitorCacheMaybeExpand_DeadLock.java
ReflectStackOverflow.java
ShiftTest.java - David commented this can go under compiler (a jit test)
WideStrictInline.java
I have looked at the tests in more detail, and here are my 
recommendation of placements:

    ExplicitArithmeticCheck
    This test checks that ArithmeticException is thrown when 
appropriate

    I would recommend placing it under runtime/ErrorHandling
    MonitorCacheMaybeExpand_DeadLock
    Existing folder: runtime/Thread (it does have a locking test)
    Or, alternatively, create a new folder: 'locking' or 'monitors'
    ReflectStackOverflow
    Uses recursive reflection attempting to provoke stack overflow
    Can go under: runtime/reflect
    WideStrictInline:
    checks for correct FP inlining by the interpreter
    I could not find existing sections; perhaps create 'interpreter'
    folder under 'runtime'

Thank you,
Misha


Since we plan (as discussed) to follow up this work with an RFE to 
review and consider removal of old and
not-that-useful tests, you could place them under 'misc' for now. 
Alternatively, find the best match

or create new sub-directories under runtime/ if necessary.


Thank you,
Misha


On 2/21/19 11:53 AM, Igor Ignatyev wrote:


On Feb 21, 2019, at 12:11 AM, David Holmes > wrote:


Hi Igor,

On 21/02/2019 3:19 pm, Igor Ignatyev wrote:

http://cr.openjdk.java.net/~iignatyev//8219139/webrev.00/index.html

40 lines changed: 17 ins; 2 del; 21 mod;

Hi all,
could you please review this small patch which moves tests from 
test/jdk/vm?
I find some of these tests - the runtime ones at least - of 
extremely dubious value. They either cover basic functionality that 
is already well covered, or are regression tests for bugs in code 
that hasn't existed for many many years!
as I wrote in another related email: "one of the reason I'm 
proposing this move is exactly questionable value of these tests, I 
want to believe that having these tests in hotspot/ test directories 
will bring more attention to them from corresponding component teams 
and hence they will be removed/reworked/re-whatever faster and 
better. and I also believe that one of the reason we got 
duplications exactly because these tests were located in jdk test 
suite."



BTW:

test/hotspot/jtreg/runtime/ShiftTest.java

is actually a jit test according to the test comment.

sure, I will move it to hotspot/compiler.
there are 16 tests in test/jdk/vm directory. all but 
JniInvocationTest are hotspot tests, so they are moved to 
test/hotspot test suite; JniInvocationTest is a launcher test
No its a JNI invocation API test - nothing to do with our launcher. 
It belongs in runtime/jni. But we already have tests in runtime 
that use the JNI invocation API so this test adds no new coverage.
this is actually was my first reaction, and I even have the webrev 
which moves it to runtime/jni, but then I looked at the associated 
bug and it is filed against tools/launcher. and I even got a false 
(as I know by now) memory that I saw JLI_ method being called from 
the test. there is actually another test 
(dk/tools/launcher/exeJliLaunchTest.c) associated w/ this bug which 
calls JLI_Launch. anyhow, I'll move this test to hotspot/runtime/jni.


I really think the value of these tests needs to be examined before 
they are brought over.
I'd prefer to have follow-up RFEs/tasks, b/c the longer we keep 
jdk/vm directory the more tests can end up there and the more rotten 
these tests 

Re: RFR: Container Fixes (8219652, 8217766, 8212528)

2019-03-14 Thread David Holmes

Hi Bob,

Sorry I took a look but can't really review it in detail as I don't know 
any of the cgroup details. Not sure who may ... perhaps Misha (cc'd).


On 15/03/2019 12:15 am, Bob Vandette wrote:

Ping ...


Please review these three fixes for Linux Docker/cgroup container support.

WEBREV:

http://cr.openjdk.java.net/~bobv/8217766/webrev.0


src/java.base/linux/classes/jdk/internal/platform/cgroupv1/Metrics.java

The copyright update should have been from:

* Copyright (c) 2018, Oracle

to

* Copyright (c) 2018, 2019, Oracle

David
-


ISSUE1:

https://bugs.openjdk.java.net/browse/JDK-8219562 - Line of code in 
osContainer_linux.cpp#L102 appears unreachable

This change corrects a rarely used hotspot code path making it compatible with 
the Java based Metrics.

ISSUE2:

https://bugs.openjdk.java.net/browse/JDK-8212528 - Wrong cgroup subsystem being 
used for some CPU Container Metrics

Most Linux distros provide symbolic links for cpuacct and cpu controller 
directories.  Docker on the Mac does not.
This causes some of the cpu statistics to be unreported since the directory 
name was incorrect.

ISSUE3:

https://bugs.openjdk.java.net/browse/JDK-8217766 - Container Support doesn't 
work for some Join Controllers combinations

The cgroup identification -implemented by parsing /proc/self/mountinfo
and /proc/self/cgroup- assumed each cgroup controller was mounted
disjoint from the others (except for "cpu" and "cpuacct" controllers).
Which means, we expected one single controller per mountinfo line.

This matches the way most Linux distributions currently configure
cgroupsv1 by default. Yet controllers can be grouped arbitrarily.
For instance, using the JoinControllers systemd directive.
One use case for that is to let Kubernetes' kubelet discover his own
dedicated and reserved cgroup hierarchy. In that situation, the JVM
fails to discover the expected cgroup controllers set, and, when running
containerized, default to a suboptimal understanding of available resources.

Supporting arbitrarily controllers groups per mountpoint actually
allows for simpler and easier to follow code, as we don't need nested
if/else for every controller.

This fix also updates the Containers Metrics, to support joint controllers.


Bob.



Re: RFR 8220684 : Process.waitFor(long, TimeUnit) can return false for a process that exited within the timeout

2019-03-14 Thread David Holmes

Hi Ivan,

On 15/03/2019 5:49 am, Ivan Gerasimov wrote:

Hello!

The default implementation of Process.waitFor(long, TimeUnit) does not 
check if the process has exited after the last portion of the timeout 
has expired.


Please clarify. There is always a race between detecting a timeout and 
the process actually terminating. If the process actually terminates 
while you're deciding to report a timeout that seems just  an acceptable 
possibility. No matter what you do the process could terminate just 
after you decided to report the timeout.


David
-

JDK has two implementations of Process (for Unix and Windows) and they 
both override waitFor(), so it's not an issue for them.


Still, it is better to provide a more accurate default implementation.

I'm not quite certain the regression test needs to be included in the 
fix.  The test does demonstrate the issue with the unfixed JDK and 
passed Okay on all tested platforms in Mach5.  Yet, I suspect the test 
can still show false negative results, as there are no guaranties that 
even such simple application as `true` will finish in 100 ms.
I can tag the test as @ignored with a comment, or simply remove it from 
the fix.


BUGURL: https://bugs.openjdk.java.net/browse/JDK-8220684
WEBREV: http://cr.openjdk.java.net/~igerasim/8220684/00/webrev/

Thanks in advance for reviewing!



Re: RFR 6307456 : UnixFileSystem_md.c use of chmod() and access() should handle EINTR signal appropriately (unix)

2019-03-14 Thread David Holmes

Hi Ivan,

This is an "ancient" bug that you are fixing. I don't think it is valid.

On 15/03/2019 3:29 am, Ivan Gerasimov wrote:

Hello!

Not all the man pages agree that chmod, access and statvfs64 can be 
interrupted, but at least on some platforms they are allowed to fail 
with EINTR:  chmod(2) on MacOS, access(2) on Solaris and statvfs(3) on 
Linux.


So, it would be more accurate to wrap these up into a RESTARTABLE loop.


When Java threads are created, or native threads attach to the VM to 
become Java threads, they all get a very specific signal-mask to block 
most (non synchronous) signals. The signals that we install handlers for 
in the VM are also configured with SA_RESTART. So unless specifically 
specified as not honouring SA_RESTART we should not need the RESTARTABLE 
loop.


So I'm not clear exactly what signals we need to be guarding against 
here, or whether this indicates some kind of (historic) mismatch between 
the library and VM code?


Thanks,
David
-

Also, java_io_UnixFileSystem_list was tiny-optimized to avoid 
unnecessary reallocation.


Would you please help review the fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-6307456
WEBREV: http://cr.openjdk.java.net/~igerasim/6307456/00/webrev/

Thanks in advance!



Re: RFR(S) : 8219139 : move hotspot tests from test/jdk/vm

2019-03-14 Thread Igor Ignatyev
Thanks for the review Misha.

can I get a LGTM from a Reviewer?

-- Igor

> On Mar 14, 2019, at 3:53 PM, mikhailo.seledt...@oracle.com wrote:
> 
> Looks good to me,
> 
> Thank you,
> 
> Misha
> 
> 
> On 3/14/19 2:38 PM, Igor Ignatyev wrote:
>> Hi Misha,
>> 
>> thanks for your suggestions, I have moved all runtime tests into 
>> subdirectories. here is the updated webrev: 
>> http://cr.openjdk.java.net/~iignatyev//8219139/webrev.01/index.html 
>> 
>> 
>> Thanks,
>> -- Igor
>> 
>>> On Mar 4, 2019, at 1:57 PM, mikhailo.seledt...@oracle.com 
>>>  wrote:
>>> 
>>> Hi Igor,
>>> 
>>>   Sorry it took a while to get back to you on this one. See my comment below
>>> 
>>> 
>>> On 2/22/19 10:35 AM, mikhailo.seledt...@oracle.com 
>>>  wrote:
 Overall the change looks good; thank you Igor for doing this. I have 
 couple of comments:
 
   - I am in favor of the approach where we move tests first under 
 corresponding sub-component directories in
 test/hotspot sub-tree, and then figure out whether to keep them. Even 
 though in general I am in favor
 of removing tests that are obsolete or of questionable value, this 
 requires time, consideration and discussions.
 Hence, I recommend filing an RFE for that, which can be addressed 
 after the migration.
 
   - Runtime normally avoids tests directly in test/hotspot/jtreg/runtime
 The tests should go into underlying sub-directories which best match 
 functional area or topic of that test.
 In most cases you did it in your change, but there are several tests 
 that your change places directly under
  test/hotspot/jtreg/runtime/:
 
  ExplicitArithmeticCheck.java
  MonitorCacheMaybeExpand_DeadLock.java
  ReflectStackOverflow.java
  ShiftTest.java - David commented this can go under compiler (a jit 
 test)
  WideStrictInline.java
>>> I have looked at the tests in more detail, and here are my recommendation 
>>> of placements:
>>> ExplicitArithmeticCheck
>>> This test checks that ArithmeticException is thrown when appropriate
>>> I would recommend placing it under runtime/ErrorHandling
>>> MonitorCacheMaybeExpand_DeadLock
>>> Existing folder: runtime/Thread (it does have a locking test)
>>> Or, alternatively, create a new folder: 'locking' or 'monitors'
>>> ReflectStackOverflow
>>> Uses recursive reflection attempting to provoke stack overflow
>>> Can go under: runtime/reflect
>>> WideStrictInline:
>>> checks for correct FP inlining by the interpreter
>>> I could not find existing sections; perhaps create 'interpreter'
>>> folder under 'runtime'
>>> 
>>> Thank you,
>>> Misha
 
  Since we plan (as discussed) to follow up this work with an RFE to 
 review and consider removal of old and
  not-that-useful tests, you could place them under 'misc' for now. 
 Alternatively, find the best match
  or create new sub-directories under runtime/ if necessary.
 
 
 Thank you,
 Misha
 
 
 On 2/21/19 11:53 AM, Igor Ignatyev wrote:
> 
>> On Feb 21, 2019, at 12:11 AM, David Holmes > > wrote:
>> 
>> Hi Igor,
>> 
>> On 21/02/2019 3:19 pm, Igor Ignatyev wrote:
>>> http://cr.openjdk.java.net/~iignatyev//8219139/webrev.00/index.html 
>>> 
 40 lines changed: 17 ins; 2 del; 21 mod;
>>> Hi all,
>>> could you please review this small patch which moves tests from 
>>> test/jdk/vm?
>> I find some of these tests - the runtime ones at least - of extremely 
>> dubious value. They either cover basic functionality that is already 
>> well covered, or are regression tests for bugs in code that hasn't 
>> existed for many many years!
> as I wrote in another related email: "one of the reason I'm proposing 
> this move is exactly questionable value of these tests, I want to believe 
> that having these tests in hotspot/ test directories will bring more 
> attention to them from corresponding component teams and hence they will 
> be removed/reworked/re-whatever faster and better. and I also believe 
> that one of the reason we got duplications exactly because these tests 
> were located in jdk test suite."
> 
>> BTW:
>> 
>> test/hotspot/jtreg/runtime/ShiftTest.java
>> 
>> is actually a jit test according to the test comment.
> sure, I will move it to hotspot/compiler.
>>> there are 16 tests in test/jdk/vm directory. all but JniInvocationTest 
>>> are hotspot tests, so they are moved to test/hotspot test suite; 
>>> JniInvocationTest is a 

Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread Stephen Colebourne
On Thu, 14 Mar 2019 at 19:45, Brian Goetz  wrote:
> Why not make `Iterator` implement `IterableOnce`? The default method
> would obviously just return `this`.
>
> Such a default would not conform to the contract, as IO requires that 
> subsequent calls throw.

IterableOnce.wrap(iterator) ?

Not providing some kind of connection between these types will look
pretty silly I think.
Stephen


Re: RFR(S) : 8219139 : move hotspot tests from test/jdk/vm

2019-03-14 Thread mikhailo . seledtsov

Looks good to me,

Thank you,

Misha


On 3/14/19 2:38 PM, Igor Ignatyev wrote:

Hi Misha,

thanks for your suggestions, I have moved all runtime tests into 
subdirectories. here is the updated webrev: 
http://cr.openjdk.java.net/~iignatyev//8219139/webrev.01/index.html 



Thanks,
-- Igor

On Mar 4, 2019, at 1:57 PM, mikhailo.seledt...@oracle.com 
 wrote:


Hi Igor,

  Sorry it took a while to get back to you on this one. See my 
comment below



On 2/22/19 10:35 AM,mikhailo.seledt...@oracle.com 
wrote:
Overall the change looks good; thank you Igor for doing this. I have 
couple of comments:


- I am in favor of the approach where we move tests first under 
corresponding sub-component directories in
test/hotspot sub-tree, and then figure out whether to keep them. 
Even though in general I am in favor
of removing tests that are obsolete or of questionable value, this 
requires time, consideration and discussions.
Hence, I recommend filing an RFE for that, which can be addressed 
after the migration.


- Runtime normally avoids tests directly in test/hotspot/jtreg/runtime
The tests should go into underlying sub-directories which best match 
functional area or topic of that test.
In most cases you did it in your change, but there are several tests 
that your change places directly under

test/hotspot/jtreg/runtime/:

ExplicitArithmeticCheck.java
MonitorCacheMaybeExpand_DeadLock.java
ReflectStackOverflow.java
ShiftTest.java - David commented this can go under compiler (a jit test)
WideStrictInline.java
I have looked at the tests in more detail, and here are my 
recommendation of placements:

    ExplicitArithmeticCheck
    This test checks that ArithmeticException is thrown when 
appropriate

    I would recommend placing it under runtime/ErrorHandling
MonitorCacheMaybeExpand_DeadLock
    Existing folder: runtime/Thread (it does have a locking test)
    Or, alternatively, create a new folder: 'locking' or 'monitors'
    ReflectStackOverflow
    Uses recursive reflection attempting to provoke stack overflow
    Can go under: runtime/reflect
    WideStrictInline:
    checks for correct FP inlining by the interpreter
    I could not find existing sections; perhaps create 'interpreter'
    folder under 'runtime'

Thank you,
Misha


Since we plan (as discussed) to follow up this work with an RFE to 
review and consider removal of old and
not-that-useful tests, you could place them under 'misc' for now. 
Alternatively, find the best match

or create new sub-directories under runtime/ if necessary.


Thank you,
Misha


On 2/21/19 11:53 AM, Igor Ignatyev wrote:


On Feb 21, 2019, at 12:11 AM, David Holmes 
mailto:david.hol...@oracle.com>> wrote:


Hi Igor,

On 21/02/2019 3:19 pm, Igor Ignatyev wrote:
http://cr.openjdk.java.net/~iignatyev//8219139/webrev.00/index.html 


40 lines changed: 17 ins; 2 del; 21 mod;

Hi all,
could you please review this small patch which moves tests from 
test/jdk/vm?
I find some of these tests - the runtime ones at least - of 
extremely dubious value. They either cover basic functionality 
that is already well covered, or are regression tests for bugs in 
code that hasn't existed for many many years!
as I wrote in another related email: "one of the reason I'm 
proposing this move is exactly questionable value of these tests, I 
want to believe that having these tests in hotspot/ test 
directories will bring more attention to them from corresponding 
component teams and hence they will be removed/reworked/re-whatever 
faster and better. and I also believe that one of the reason we got 
duplications exactly because these tests were located in jdk test 
suite."



BTW:

test/hotspot/jtreg/runtime/ShiftTest.java

is actually a jit test according to the test comment.

sure, I will move it to hotspot/compiler.
there are 16 tests in test/jdk/vm directory. all but 
JniInvocationTest are hotspot tests, so they are moved to 
test/hotspot test suite; JniInvocationTest is a launcher test
No its a JNI invocation API test - nothing to do with our 
launcher. It belongs in runtime/jni. But we already have tests in 
runtime that use the JNI invocation API so this test adds no new 
coverage.
this is actually was my first reaction, and I even have the webrev 
which moves it to runtime/jni, but then I looked at the associated 
bug and it is filed against tools/launcher. and I even got a false 
(as I know by now) memory that I saw JLI_ method being called from 
the test. there is actually another test 
(dk/tools/launcher/exeJliLaunchTest.c) associated w/ this bug which 
calls JLI_Launch. anyhow, I'll move this test to hotspot/runtime/jni.


I really think the value of these tests needs to be examined 
before they are brought over.
I'd prefer to have 

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

2019-03-14 Thread mark . reinhold
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: RFR(S) : 8219139 : move hotspot tests from test/jdk/vm

2019-03-14 Thread Igor Ignatyev
Hi Misha,

thanks for your suggestions, I have moved all runtime tests into 
subdirectories. here is the updated webrev: 
http://cr.openjdk.java.net/~iignatyev//8219139/webrev.01/index.html 


Thanks,
-- Igor

> On Mar 4, 2019, at 1:57 PM, mikhailo.seledt...@oracle.com wrote:
> 
> Hi Igor,
> 
>   Sorry it took a while to get back to you on this one. See my comment below
> 
> 
> On 2/22/19 10:35 AM, mikhailo.seledt...@oracle.com 
>  wrote:
>> Overall the change looks good; thank you Igor for doing this. I have couple 
>> of comments:
>> 
>>   - I am in favor of the approach where we move tests first under 
>> corresponding sub-component directories in
>> test/hotspot sub-tree, and then figure out whether to keep them. Even 
>> though in general I am in favor
>> of removing tests that are obsolete or of questionable value, this 
>> requires time, consideration and discussions.
>> Hence, I recommend filing an RFE for that, which can be addressed after 
>> the migration.
>> 
>>   - Runtime normally avoids tests directly in test/hotspot/jtreg/runtime
>> The tests should go into underlying sub-directories which best match 
>> functional area or topic of that test.
>> In most cases you did it in your change, but there are several tests 
>> that your change places directly under
>>  test/hotspot/jtreg/runtime/:
>> 
>>  ExplicitArithmeticCheck.java
>>  MonitorCacheMaybeExpand_DeadLock.java
>>  ReflectStackOverflow.java
>>  ShiftTest.java - David commented this can go under compiler (a jit test)
>>  WideStrictInline.java
> I have looked at the tests in more detail, and here are my recommendation of 
> placements:
> ExplicitArithmeticCheck
> This test checks that ArithmeticException is thrown when appropriate
> I would recommend placing it under runtime/ErrorHandling
> MonitorCacheMaybeExpand_DeadLock
> Existing folder: runtime/Thread (it does have a locking test)
> Or, alternatively, create a new folder: 'locking' or 'monitors'
> ReflectStackOverflow
> Uses recursive reflection attempting to provoke stack overflow
> Can go under: runtime/reflect
> WideStrictInline:
> checks for correct FP inlining by the interpreter
> I could not find existing sections; perhaps create 'interpreter'
> folder under 'runtime'
> 
> Thank you,
> Misha
>> 
>>  Since we plan (as discussed) to follow up this work with an RFE to 
>> review and consider removal of old and
>>  not-that-useful tests, you could place them under 'misc' for now. 
>> Alternatively, find the best match
>>  or create new sub-directories under runtime/ if necessary.
>> 
>> 
>> Thank you,
>> Misha
>> 
>> 
>> On 2/21/19 11:53 AM, Igor Ignatyev wrote:
>>> 
 On Feb 21, 2019, at 12:11 AM, David Holmes  wrote:
 
 Hi Igor,
 
 On 21/02/2019 3:19 pm, Igor Ignatyev wrote:
> http://cr.openjdk.java.net/~iignatyev//8219139/webrev.00/index.html
>> 40 lines changed: 17 ins; 2 del; 21 mod;
> Hi all,
> could you please review this small patch which moves tests from 
> test/jdk/vm?
 I find some of these tests - the runtime ones at least - of extremely 
 dubious value. They either cover basic functionality that is already well 
 covered, or are regression tests for bugs in code that hasn't existed for 
 many many years!
>>> as I wrote in another related email: "one of the reason I'm proposing this 
>>> move is exactly questionable value of these tests, I want to believe that 
>>> having these tests in hotspot/ test directories will bring more attention 
>>> to them from corresponding component teams and hence they will be 
>>> removed/reworked/re-whatever faster and better. and I also believe that one 
>>> of the reason we got duplications exactly because these tests were located 
>>> in jdk test suite."
>>> 
 BTW:
 
 test/hotspot/jtreg/runtime/ShiftTest.java
 
 is actually a jit test according to the test comment.
>>> sure, I will move it to hotspot/compiler.
> there are 16 tests in test/jdk/vm directory. all but JniInvocationTest 
> are hotspot tests, so they are moved to test/hotspot test suite; 
> JniInvocationTest is a launcher test
 No its a JNI invocation API test - nothing to do with our launcher. It 
 belongs in runtime/jni. But we already have tests in runtime that use the 
 JNI invocation API so this test adds no new coverage.
>>> this is actually was my first reaction, and I even have the webrev which 
>>> moves it to runtime/jni, but then I looked at the associated bug and it is 
>>> filed against tools/launcher. and I even got a false (as I know by now) 
>>> memory that I saw JLI_ method being called from the test. there is actually 
>>> another test (dk/tools/launcher/exeJliLaunchTest.c) associated w/ this bug 
>>> which calls 

Re: RFR(S) : 8219139 : move hotspot tests from test/jdk/vm

2019-03-14 Thread Igor Ignatyev
Hi Alan,

I double checked, the test is linked w/ -ljli, and calls JNI_CreateJavaVM from 
libjli.so. hence I'll leave JniInvocationTest in jdk/tools/launcher.

-- Igor 

> On Feb 21, 2019, at 12:17 PM, Alan Bateman  wrote:
> 
> On 21/02/2019 19:55, Igor Ignatyev wrote:
>> :
>> Alan, you are right, this test is a JNI test and should be moved to 
>> hotspot/runtime/jni. more details in my answer to the same comment in 
>> David's email. in two words, I accidentally looked at another test.
>> 
> Can you double check that it is actually using the JNI invocation interface 
> directly? I don't think we were able to find anyone in Eclipse to explain 
> what their launcher on macOS is doing. I suspect it may be directly (or 
> indirectly) reading the CFBundleExecutable property from Info.plist and 
> calling the JNI_CreateJavaVM function in libjli (not libjvm). We probably 
> need more tests in this area and also a bit more archaeology to see whether 
> this was a supported interface in Apple's JDK.
> 
> -Alan



Re: RFR 8220684 : Process.waitFor(long, TimeUnit) can return false for a process that exited within the timeout

2019-03-14 Thread Ivan Gerasimov

Thank you Pavel!


On 3/14/19 2:02 PM, Pavel Rappo wrote:

I have to look at this patch in more detail, however here's what jumped out at
me straight away:

 long deadline = System.nanoTime() + remainingNanos;

It seems like a possibility for an overflow.

Not quite.  The deadline can surely become negative, which is alright.  
Later we only check the difference (deadline - System.nanoTime()).


Actually, the new code mimics what we have in PocessImpl for Unix and 
Windows.


With kind regards,
Ivan


  Documentation for System.nanoTime
has a special section on this:

 For example, to measure how long some code takes to execute:

long startTime = System.nanoTime();
// ... the code being measured ...
long elapsedNanos = System.nanoTime() - startTime;

 To compare elapsed time against a timeout, use

if (System.nanoTime() - startTime >= timeoutNanos) ...

 instead of

  if (System.nanoTime() >= startTime + timeoutNanos) ...

 because of the possibility of numerical overflow.

Is that of concern in this case?


On 14 Mar 2019, at 19:49, Ivan Gerasimov  wrote:

Hello!

The default implementation of Process.waitFor(long, TimeUnit) does not check if 
the process has exited after the last portion of the timeout has expired.

JDK has two implementations of Process (for Unix and Windows) and they both 
override waitFor(), so it's not an issue for them.

Still, it is better to provide a more accurate default implementation.

I'm not quite certain the regression test needs to be included in the fix.  The 
test does demonstrate the issue with the unfixed JDK and passed Okay on all 
tested platforms in Mach5.  Yet, I suspect the test can still show false 
negative results, as there are no guaranties that even such simple application 
as `true` will finish in 100 ms.
I can tag the test as @ignored with a comment, or simply remove it from the fix.

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8220684
WEBREV: http://cr.openjdk.java.net/~igerasim/8220684/00/webrev/

Thanks in advance for reviewing!

--
With kind regards,
Ivan Gerasimov





--
With kind regards,
Ivan Gerasimov



I don't understand why we need IterableOnce ? Was: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread Remi Forax
- Mail original -
> De: "Peter Levart" 
> À: "Brian Goetz" , "Stuart Marks" 
> , "core-libs-dev"
> 
> Envoyé: Mardi 12 Mars 2019 18:34:58
> Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

> On 3/12/19 5:04 PM, Brian Goetz wrote:
>> No. You have the LSP backwards (though this is easy to do.)
>>
>> IterableOnce means "*must throw* on subsequent use"; under this spec,
>> an arbitrary Iterable is most certainly *not* an IterableOnce, and
>> therefore an LSP violation.
>>
>> It sounds like you are suggesting that we instead spec
>> IterableAtLeastOnce, of which Iterable *would* be a credible subtype
>> -- but due to how Iterable is specified now, the problem is that
>> Iterable *already is* really "iterable at least once."  So there would
>> be no point in introducing this type; we already have it.
> 
> Due to how Iterable is specified now it does not promise multiple
> iterations, but due to how most (all that I know of except
> DirectoryStream or some 3rd party) Iterables are implemented now, the
> common expectation is that it does. By implementing more Iterable(s)
> that don't support multiple iteration (regardless of whether they
> implement this new sub-type or not), this expectation will be less and
> less honored. Perhaps this is not so bad. At various occasions the
> specification has been changed to accommodate the long-standing behavior
> or expectation of behavior, but this seems not to be one of them.
> 
> Let's pretend for a moment that Iterable specification was changed to
> guarantee multi-pass iteration. In that case the IterableAtLeastOnce as
> a supertype of multi-pass Iterable would make much more sense, wouldn't it?
> 
> But as there could be Iterables out there that by current spec are
> perfectly valid and don't support multi-pass iteration, such spec change
> would make them non-conformant and is therefore not allowed. So I'm
> convinced now that this is the least-bad solution.
> 
> Regards, Peter

yes, i think i prefer this solution, one Iterable to rule them all.

First, it's not in the spirit of the Collection API to multiply the interfaces, 
by example, we have only one kind of Iterator and not an Iterator and a 
ReadOnlyIterator even if a lot of iterators doesn't implement remove. It's a 
central design of the Collection API, reduce the number of interfaces to ease 
the use even if it means that each interface may have a broader definition. The 
Collection API design has chosen his side between users and library writers 
(people that provides implementations) because from the library writer point of 
view you can not specify exactly the semantics you want.

Then from the user POV, what is point of IterableOnce ? I will not using it as 
parameter because using Iterable is a super-type (like i will use a List 
instead of an ArrayList as parameter) and if i using it as return type, codes 
that call that method can put it in an Iterable, this is exactly what the 
for-each-loop will do BTW, so it's seems useless.

Also as Peter said, there are already codes in the wild that create an Iterable 
that can only be iterated once, ASM has such class, if IterableOnce is added to 
the JDK, i will have people ask me to retrofit the ASM class to use 
IterableOnce just for the sake of having the right semantics ? So basically by 
introducing IterableOnce, all codes that were perfectly fine in term of 
semantics before introducing IterableOnce are now not quite right because they 
are not implementing the right interface ? Hum, i think i still not get why we 
need such interface.

Rémi


Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

2019-03-14 Thread Brian Burkhalter
The CSR for this issue is available for review at 
https://bugs.openjdk.java.net/browse/JDK-8202555 
. If you have a JBS user name 
you can add yourself as reviewer by editing the issue directly, assuming you 
concur with the content.

Brian

> On Mar 8, 2019, at 8:56 AM, raffaello.giulie...@gmail.com wrote:
> 
> On 2019-03-08 14:35, Andrew Haley wrote:
>> Hi,
>> 
>> On 3/7/19 7:16 PM, Raffaello Giulietti wrote:
>> 
>>> a couple of weeks ago I tried to refactor the code assuming the 
>>> existence of unsignedMultiplyHigh() (either as some future intrinsic or 
>>> as a Java method) and a wider representations of g with either 127 or 
>>> 128 bits:
>>> g = g1 2^64 + g0
>>> 
>>> with either
>>> 2^63 <= g1 < 2^64 (128 bits)
>>> 
>>> or
>>> 2^62 <= g1 < 2^63 (127 bits)
>>> 
>>> Unfortunately, the resulting code of rop() isn't any simpler. That's 
>>> because then an intermediate sum can overflow the 64 bits of a long. As 
>>> a consequence, there's need for more elaborate logic to determine
>>> the carry and other slightly more complicated computations to assemble
>>> the final result. All in all, the resulting code has more operations and 
>>> looks longer.
>> 
>> Ah, I see. I agree, we still don't quite have the full set of operations
>> that we need in Java, in particular a nice way of doing an add with carry.
>> 
> 
> Yes.
> 
> 
>> Thank you for the explanation.
>> 
> 
> You're welcome.
> 
> 
>>> In the meantime I got rid of the last division. There's no division at 
>>> all in the whole algorithm.
>> 
>> Excellent. This is looking very good indeed.
>> 
> 



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread forax
yes,i should have been more specific, you can not *without* having some boxing 
in the middle.

Rémi

- Mail original -
> De: "Tagir Valeev" 
> À: "Remi Forax" 
> Cc: "Stuart Marks" , "core-libs-dev" 
> 
> Envoyé: Jeudi 7 Mars 2019 11:33:20
> Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

> Hello, Remi!
> 
> It actually works thanks to auto-boxing/unboxing. E.g. this
> implementation works:
> 
> static Iterable range(int from, int to) {
>  return () -> new PrimitiveIterator.OfInt() {
>int cur = from;
> 
>@Override
>public int nextInt() {
>  if (cur >= to) {
>throw new NoSuchElementException();
>  }
>  return cur++;
>}
> 
>@Override
>public boolean hasNext() {
>  return cur < to;
>}
>  };
> }
> 
> public static void main(String[] args) {
>  for(int i : range(0, 100)) {
>System.out.println(i);
>  }
> }
> 
> It correctly compiles and prints numbers from 0 to 99. As IntStream
> extends BaseStream and BaseStream BaseStream> defines Iterator iterator(), it would be no
> problem with using IntStream.range in such code pattern were
> BaseStream extends IterableOnce.
> 
> Of course this produces unnecessary garbage, as I said.
> 
> With best regards,
> Tagir Valeev.
> 
> On Wed, Mar 6, 2019 at 7:37 PM Remi Forax  wrote:
>>
>> Hi Tagir,
>> try to do it now and you will see that you can't, because you can not write
>> Iterable yet.
>> Once we will get generics over value types, it will be a no-brainer.
>>
>> Rémi
>>
>> - Mail original -
>> > De: "Tagir Valeev" 
>> > À: "Stuart Marks" 
>> > Cc: "core-libs-dev" 
>> > Envoyé: Mercredi 6 Mars 2019 11:10:41
>> > Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow 
>> > Streams
>>
>> > Hello!
>> >
>> > By the way one of the painful code patterns in modern Java is `for(int
>> > i = 0; i> > newbies and prone to errors as the variable need to be repeated three
>> > times. Also the variable is not effectively final, despite it never
>> > changes within the loop body, so could have been considered as
>> > redeclared on every loop iteration (like in for-each). With the new
>> > proposal it's possible to write `for(int i : range(0, bound).boxed())`
>> > (assuming import static j.u.s.IntStream.range), which looks much
>> > better, though it has obvious performance drawback. Moving
>> > IterableOnce to BaseStream would enable to use `for(int i : range(0,
>> > bound))` which looks even better, though again we have plenty of
>> > garbage (but considerably less than in previous case!). I wonder
>> > whether Java could evolve to the point where such kind of code would
>> > be a recommended way to iterate over subsequent integer values without
>> > any performance handicap.
>> >
>> > With best regards,
>> > Tagir Valeev.
>> >
>> > On Fri, Mar 1, 2019 at 9:47 AM Stuart Marks  
>> > wrote:
>> >>
>> >> Hi all,
>> >>
>> >> Please review and comment on this proposal to allow Stream instances to 
>> >> be used
>> >> in enhanced-for ("for-each") loops.
>> >>
>> >> Abstract
>> >>
>> >> Occasionally it's useful to iterate a Stream using a conventional loop. 
>> >> However,
>> >> the Stream interface doesn't implement Iterable, and therefore streams 
>> >> cannot be
>> >> used with the enhanced-for statement. This is a proposal to remedy that
>> >> situation by introducing a new interface IterableOnce that is a subtype of
>> >> Iterable, and then retrofitting the Stream interface to implement it. 
>> >> Other JDK
>> >> classes will also be retrofitted to implement IterableOnce.
>> >>
>> >> Full Proposal:
>> >>
>> >>  http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html
>> >>
>> >> Bug report:
>> >>
>> >>  https://bugs.openjdk.java.net/browse/JDK-8148917
>> >>
>> >> Webrev:
>> >>
>> >>  http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/
>> >>
>> >> Note, this changeset isn't ready to push yet. In particular, it has no 
>> >> tests
>> >> yet. However, the implementation is so simple that I figured I should 
>> >> include
>> >> it. Comments on the specification wording are also welcome.
>> >>
>> >> Thanks,
>> >>
> > > > s'marks


RFR 8220684 : Process.waitFor(long, TimeUnit) can return false for a process that exited within the timeout

2019-03-14 Thread Ivan Gerasimov

Hello!

The default implementation of Process.waitFor(long, TimeUnit) does not 
check if the process has exited after the last portion of the timeout 
has expired.


JDK has two implementations of Process (for Unix and Windows) and they 
both override waitFor(), so it's not an issue for them.


Still, it is better to provide a more accurate default implementation.

I'm not quite certain the regression test needs to be included in the 
fix.  The test does demonstrate the issue with the unfixed JDK and 
passed Okay on all tested platforms in Mach5.  Yet, I suspect the test 
can still show false negative results, as there are no guaranties that 
even such simple application as `true` will finish in 100 ms.
I can tag the test as @ignored with a comment, or simply remove it from 
the fix.


BUGURL: https://bugs.openjdk.java.net/browse/JDK-8220684
WEBREV: http://cr.openjdk.java.net/~igerasim/8220684/00/webrev/

Thanks in advance for reviewing!

--
With kind regards,
Ivan Gerasimov



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread Brian Goetz
> A new concern from me is that this change would allow Iterable and
> Stream to be used in foreach, but not Iterator. This seems like an
> odd/sharp conceptual edge.

Not actually a new concern — this was discussed back in JSR 335 as well.  

> Why not make `Iterator` implement `IterableOnce`? The default method
> would obviously just return `this`.

Such a default would not conform to the contract, as IO requires that 
subsequent calls throw.  

Re: RFR: 8220579: [Containers] SubSystem.java out of sync with osContainer_linux.cpp

2019-03-14 Thread Severin Gehwolf
On Thu, 2019-03-14 at 13:58 -0400, Bob Vandette wrote:
> The change looks good.  Thanks for fixing this.

Thanks for the review, Bob!

Cheers,
Severin

> I’d send this to core-libs (cc’d).
> 
> Bob.
> 
> 
> > On Mar 14, 2019, at 12:51 PM, Severin Gehwolf 
> > wrote:
> > 
> > Hi,
> > 
> > I'm not sure what the right list for Metrics.java[1] is. Assuming
> > it's
> > serviceability-dev:
> > 
> > Please review this one-liner for for SubSystem.java which currently
> > behaves differently from the native implementation in
> > osContainer_linux.cpp. Please see the details in the bug.
> > 
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8220579
> > webrev: 
> > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8220579/01/webrev/
> > 
> > Testing:
> > Manual testing of JDK-8217338 with Metrics.java support
> > with/without
> > this fix on Linux x86_64. Metrics tests and Docker tests continue
> > to
> > pass for fastdebug jvms (NOT for release jvms. see JDK-8220674,
> > which
> > was fun).
> > 
> > Thoughts?
> > 
> > Thanks,
> > Severin
> > 
> > [1] 
> > http://hg.openjdk.java.net/jdk/jdk/file/641768acb12e/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/Metrics.java
> > 



Re: RFR 6307456 : UnixFileSystem_md.c use of chmod() and access() should handle EINTR signal appropriately (unix)

2019-03-14 Thread Brian Burkhalter
Hello Ivan,

This looks all right to me.

Thanks,

Brian

> On Mar 14, 2019, at 10:29 AM, Ivan Gerasimov  
> wrote:
> 
> Not all the man pages agree that chmod, access and statvfs64 can be 
> interrupted, but at least on some platforms they are allowed to fail with 
> EINTR:  chmod(2) on MacOS, access(2) on Solaris and statvfs(3) on Linux.
> 
> So, it would be more accurate to wrap these up into a RESTARTABLE loop.
> 
> Also, java_io_UnixFileSystem_list was tiny-optimized to avoid unnecessary 
> reallocation.
> 
> Would you please help review the fix?
> 
> BUGURL: https://bugs.openjdk.java.net/browse/JDK-6307456 
> 
> WEBREV: http://cr.openjdk.java.net/~igerasim/6307456/00/webrev/ 
> 


Re: RFR: 8220579: [Containers] SubSystem.java out of sync with osContainer_linux.cpp

2019-03-14 Thread Bob Vandette
The change looks good.  Thanks for fixing this.

I’d send this to core-libs (cc’d).

Bob.


> On Mar 14, 2019, at 12:51 PM, Severin Gehwolf  wrote:
> 
> Hi,
> 
> I'm not sure what the right list for Metrics.java[1] is. Assuming it's
> serviceability-dev:
> 
> Please review this one-liner for for SubSystem.java which currently
> behaves differently from the native implementation in
> osContainer_linux.cpp. Please see the details in the bug.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8220579
> webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8220579/01/webrev/
> 
> Testing:
> Manual testing of JDK-8217338 with Metrics.java support with/without
> this fix on Linux x86_64. Metrics tests and Docker tests continue to
> pass for fastdebug jvms (NOT for release jvms. see JDK-8220674, which
> was fun).
> 
> Thoughts?
> 
> Thanks,
> Severin
> 
> [1] 
> http://hg.openjdk.java.net/jdk/jdk/file/641768acb12e/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/Metrics.java
> 



Re: RFR 8220088 : Test "java/lang/annotation/loaderLeak/Main.java" fails with -Xcomp

2019-03-14 Thread Brent Christian

On 3/13/19 6:08 PM, Martin Buchholz wrote:

Why not Reference.reachabilityFence ?


You mean the mechanism for this precise situation.  Yeah, OK.

http://cr.openjdk.java.net/~bchristi/8220088/webrev.01/

Thanks,
-Brent


RFR 6307456 : UnixFileSystem_md.c use of chmod() and access() should handle EINTR signal appropriately (unix)

2019-03-14 Thread Ivan Gerasimov

Hello!

Not all the man pages agree that chmod, access and statvfs64 can be 
interrupted, but at least on some platforms they are allowed to fail 
with EINTR:  chmod(2) on MacOS, access(2) on Solaris and statvfs(3) on 
Linux.


So, it would be more accurate to wrap these up into a RESTARTABLE loop.

Also, java_io_UnixFileSystem_list was tiny-optimized to avoid 
unnecessary reallocation.


Would you please help review the fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-6307456
WEBREV: http://cr.openjdk.java.net/~igerasim/6307456/00/webrev/

Thanks in advance!

--
With kind regards,
Ivan Gerasimov



Re: RFR: 8214712: Archive Attributes$Name.KNOWN_NAMES

2019-03-14 Thread Claes Redestad




On 2019-03-14 18:13, Alan Bateman wrote:

On 14/03/2019 16:26, Claes Redestad wrote:

Hi,

this RFE was stalled due an interaction with SA that has since been
resolved. As it still applies cleanly I'll consider it reviewed. I'm
just going to do some sanity testing (tier1) before push.
I think we need to rollback some of the changes that we done in 
JDK-6805750 so that we don't have non-standard attributes in the map. We 
shouldn't be hard coding names such as Ant-Version or Bnd-LastModified 
for example.


For the current webrev then I'm concerned it is brings back legacy 
attributes. The concept of "installed optional packages" was removed in 
Java SE 9, as was the ability for JAR packaged applets to trigger 
downloading of optional packages. I don't think the later was ever 
implemented in the JDK so surprising that we are finding JAR files with 
those attributes now. If we can prune that down then I think the changes 
will be okay.


Ok. I stumbled on some new test issues in SA with this patch, so I'll
need to pause this one for a while anyhow.

/Claes


Re: RFR: 8214712: Archive Attributes$Name.KNOWN_NAMES

2019-03-14 Thread Alan Bateman

On 14/03/2019 16:26, Claes Redestad wrote:

Hi,

this RFE was stalled due an interaction with SA that has since been
resolved. As it still applies cleanly I'll consider it reviewed. I'm
just going to do some sanity testing (tier1) before push.
I think we need to rollback some of the changes that we done in 
JDK-6805750 so that we don't have non-standard attributes in the map. We 
shouldn't be hard coding names such as Ant-Version or Bnd-LastModified 
for example.


For the current webrev then I'm concerned it is brings back legacy 
attributes. The concept of "installed optional packages" was removed in 
Java SE 9, as was the ability for JAR packaged applets to trigger 
downloading of optional packages. I don't think the later was ever 
implemented in the JDK so surprising that we are finding JAR files with 
those attributes now. If we can prune that down then I think the changes 
will be okay.


-Alan.


Re: RFR: 8214712: Archive Attributes$Name.KNOWN_NAMES

2019-03-14 Thread Jiangli Zhou
Looks good!

Thanks,
Jiangli

On Thu, Mar 14, 2019 at 9:26 AM Claes Redestad 
wrote:

> Hi,
>
> this RFE was stalled due an interaction with SA that has since been
> resolved. As it still applies cleanly I'll consider it reviewed. I'm
> just going to do some sanity testing (tier1) before push.
>
> Thanks!
>
> /Claes
>
> On 2018-12-03 17:02, Claes Redestad wrote:
> > Hi,
> >
> > initializing java.util.jar.Attributes.Name. executes ~20k
> > bytecodes setting up and eagerly calculating case-insensitive hash codes
> > for a slew of Name objects.
> >
> > By archiving the resulting set of Names and initializing public
> > constants from the archived map, we reduce time spent starting up
> > (Name. drops to 368 executed bytecodes) and improve the
> > footprint sharing effect of using CDS:
> >
> > http://cr.openjdk.java.net/~redestad/8214712/jdk.00/
> >
> > Testing: tier1-2 running
> >
> > Verified a 1-2.5ms startup improvement on java -jar Hello.jar
> > - significant and stable reduction in instruction count, branches and
> > branch misses
> > - only adds ~1.1Kb to the dumped CDS archive
> >
> > Thanks!
> >
> > /Claes
>


Re: RFR: 8214712: Archive Attributes$Name.KNOWN_NAMES

2019-03-14 Thread Claes Redestad

Hi,

this RFE was stalled due an interaction with SA that has since been
resolved. As it still applies cleanly I'll consider it reviewed. I'm
just going to do some sanity testing (tier1) before push.

Thanks!

/Claes

On 2018-12-03 17:02, Claes Redestad wrote:

Hi,

initializing java.util.jar.Attributes.Name. executes ~20k 
bytecodes setting up and eagerly calculating case-insensitive hash codes 
for a slew of Name objects.


By archiving the resulting set of Names and initializing public 
constants from the archived map, we reduce time spent starting up 
(Name. drops to 368 executed bytecodes) and improve the 
footprint sharing effect of using CDS:


http://cr.openjdk.java.net/~redestad/8214712/jdk.00/

Testing: tier1-2 running

Verified a 1-2.5ms startup improvement on java -jar Hello.jar
- significant and stable reduction in instruction count, branches and 
branch misses

- only adds ~1.1Kb to the dumped CDS archive

Thanks!

/Claes


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

2019-03-14 Thread Lindenmaier, Goetz
Hi Coleen, 

thanks for looking at my change..

> For the record, I think the C++ implementation is more straightforward
> than trying to use the Stackwalker and ASM because there's other code
> just like it right here. You have all the information you need directly
> in the Throwable.backtrace field. Walking it makes sense to me.  Also
> the StackWalker has a cost because it creates ResolvedMethodNames that
> must be interned in the native code in case of redefinition.
Yes, I also think that a Java implementation has more costs at 
runtime.  But as this is only encountered if the message is actually
accessed, it's not that much of a concern.

> I didn't make it through the code for bytecodeUtils but I think it
> should be in the interpreter directory where all the other bytecode
> iterating methods are.  
I moved it to the interpreter directory.
http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/05-otherMessages/

> It does seem expensive when printing an NPE
> message.  I wonder if there's some easily helpful variable names that
> you could pick out and not have to do all this work for every sort of
> bytecode. 
I'll describe the algorithm in some detail in the paper requested. 
This might answer this point. 

> Also the file uses non hotspot names also like
> 'createInvalidSource' that should be fixed.
I still need to fix these. I’ll leave this to a final review.  The 
code still might be discarded in favour of another implementation.

> http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-
> NPE/02/src/hotspot/share/classfile/javaClasses.cpp.udiff.html
> 
> There's a 'version' in the backtrace.  If the method->version() doesn't
> match the one in the backtrace, this should return false, because the
> method was redefined and the bci probably won't match. There's a bunch
> of code in javaClasses that does the similar thing.
Starting with the back trace, I will always get the right bytecodes.
The backtrace keeps the method alive as I understand.

The problem I mentioned in other mails is to get that accessible in Java code 
... 
for the C-implementation I have all the proper information 
just per design of the metaspace/backtrace etc. at hand.

Best regards,
  Goetz.



> 
> This isn't a full review.
> 
> Thanks,
> Coleen
> 
> >
> > Thanks!
> >Goetz
> >
> >> Also, it was not strictly correct when I said all that is retained is
> >> the method bytecodes. Exception tables, line number tables and local var
> >> name & type tables are also retained.
> >>
> >> regards,
> >>
> >>
> >> Andrew Dinn
> >> ---
> >> Senior Principal Software Engineer
> >> Red Hat UK Ltd
> >> Registered in England and Wales under Company Registration No.
> 03798903
> >> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander



Re: RFR 8220613: [TEST] java/util/Arrays/TimSortStackSize2.java times out with fastdebug build

2019-03-14 Thread Roger Riggs

Hi Alan,

I didn't have a good idea where to look, the times do seem excessive.
Suggestions?

Thanks, Roger


On 03/14/2019 11:09 AM, Alan Bateman wrote:

On 14/03/2019 14:37, Roger Riggs wrote:
Looks okay but the fastdebug times looks very long compared to other 
tests - do you know if there a specific assertion that is the issue here?






Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread Peter Levart
Now that Brian put it so nicely, I'm convinced and I'd like to draw back 
and support this proposal as is...


On 3/14/19 3:50 PM, Stephen Colebourne wrote:

On Thu, 14 Mar 2019 at 14:21, Brian Goetz  wrote:

There's a reason it took as long as it did for Stuart to come up with
this proposal; all the options were known for years, they all have
problems, and the net benefit is still relatively narrow, which means we
don't have a lot of budget before the cost-benefit becomes negative.  I
think the option proposed is the least-worst, and people still seem to
really want to be able to foreach over streams.

The cost-benefit is definitely tight, but I think it is positive. As I
said here [1] there is still a case for control abstractions in Java
even when you have lambdas


A new concern from me is that this change would allow Iterable and
Stream to be used in foreach, but not Iterator. This seems like an
odd/sharp conceptual edge.


To be precise: this change would not allow Iterable and Stream to be 
used in foreach. foreach does not change. It would still alow only 
Iterable (and arrays). The Stream would become an Iterable with this change.




Why not make `Iterator` implement `IterableOnce`? The default method
would obviously just return `this`.


This would conflate concepts to much. Iterator *is* an external 
iteration state while Iterable[Once] is a (one-shot) factory for 
Iterator. The specification for IterableOnce could not say that the 
second and subsequent invocations of iterator() method are to throw 
exception if Iterator interface extended IterableOnce (interfaces have 
no state to model this in the default method).





It seems to me that if we are willing to countenance foreach over a
one-shot Stream, it is inappropriate to deny foreach to a one-shot
Iterator.


Again, this proposal does not do anything to foreach specification. It 
just makes Stream be Iterable. The concepts here are more to the point:


Iterable: factory for Iterator
Stream: one-shot factory for Spliterator
Stream extends IterableOnce: one-shot factory for Spliterator and 
one-shot factory for Iterator


I think this makes perfect sense.

Regards, Peter



Re: RFR 8220613: [TEST] java/util/Arrays/TimSortStackSize2.java times out with fastdebug build

2019-03-14 Thread Alan Bateman

On 14/03/2019 14:37, Roger Riggs wrote:

Please review a test configure change to skip running TimSortStackSize2
on the fast debug build. The running time of the test is excessive.
This particular test has been timing out intermittently for a long time.

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-timsort-timeout-8220613/

Issue:
  https://bugs.openjdk.java.net/browse/JDK-8220613
Looks okay but the fastdebug times looks very long compared to other 
tests - do you know if there a specific assertion that is the issue here?


-Alan


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

2019-03-14 Thread Maurizio Cimadamore

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. A trained eye of 
course can quickly scan the line where an exception occurs, and quickly 
isolate the couple of problematic spots which could have caused an 
exception. But it takes time to get to that point, and having some 
helpful messages to help you to get to that point is, IMHO a valuable 
goal in itself.


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. What kind of improvements should we add to 
the NPE exception? What happens if the NPE already has an user-provided 
details message? Should the enhancement make use of optional debugging 
classfile attributes (you touch this in your nice RFE). And again, what 
are the performance considerations we deem important for this work to be 
declared successful? And, maintenance-wise, what is the right way to 
implement the enhancement? Should we implement that as a pure VM 
feature, should we implement it on top of the existing VM, using some 
classfile manipulation (**) ? Or a combination of those?


As you can see, there are so many question this enhancement raises that 
I think going straight for a code review can be a premature move; people 
will be coming at the review table with different answers to the above 
set of questions (and maybe additional questions too :-)), and that 
could make the review process hard and frustrating for all the parties 
involved. So I warmly suggest we take a step back from the code, and 
formulate a proposal for enhancing NPE messages in Java; in this sense, 
a JEP seems to me the most natural way to move forward.


(**) I have a quick and dirty prototype of that built using ASM which 
I'm happy to share, in case you are interested taking a look when 
evaluating alternatives.


Cheers
Maurizio


On 14/03/2019 00:42, Mandy Chung wrote:

Hi Goetz,

Roger, Coleen, Maurizio and I talked about this proposed feature.
We all think that improving NPE message is a useful enhancement for
the platform and helps developers to tell what causes NPE.

This is not a small enhancement.  Diving into a large code review
would not be the best way to move this forward as you can see the
discussion so far.

It would help if you can start with writing down the problem and
the proposal like what improvements are proposed and under what
circumstances may be acceptable that NPE message won't be improved.
This would get the discussion on the proposal feature and then
the discussion of the best way to to implement it in the VM, library,
or combination.  You can consider using the JEP template that gives
you a good structure to follow for the write up.

What do you think?

Mandy


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

2019-03-14 Thread Lindenmaier, Goetz
Hi, 

> Roger, Coleen, Maurizio and I talked about this proposed feature.
> We all think that improving NPE message is a useful enhancement for
> the platform and helps developers to tell what causes NPE.
Thanks for this positive feedback :)!

> This is not a small enhancement.  Diving into a large code review
> would not be the best way to move this forward as you can see the
> discussion so far.
> 
> It would help if you can start with writing down the problem and
> the proposal like what improvements are proposed and under what
> circumstances may be acceptable that NPE message won't be improved.
> This would get the discussion on the proposal feature and then
> the discussion of the best way to to implement it in the VM, library,
> or combination.  You can consider using the JEP template that gives
> you a good structure to follow for the write up.

Yes, I can write down the overall design of this.  This probably is a good
idea.
I already started editing, but please give me a day or two.

The past days I have been working on the C-code. I incorporated 
Coleens comments, but mostly changed the messages to print 
different text as it was proposed in several reviews. 
I'm about to publish a webrev with this new coding. This could 
be a new, better basis for discussing the wording of the message.

Best regards,
  Goetz.


 


> 
> What do you think?
> 
> Mandy


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread Stephen Colebourne
On Thu, 14 Mar 2019 at 14:21, Brian Goetz  wrote:
> There's a reason it took as long as it did for Stuart to come up with
> this proposal; all the options were known for years, they all have
> problems, and the net benefit is still relatively narrow, which means we
> don't have a lot of budget before the cost-benefit becomes negative.  I
> think the option proposed is the least-worst, and people still seem to
> really want to be able to foreach over streams.

The cost-benefit is definitely tight, but I think it is positive. As I
said here [1] there is still a case for control abstractions in Java
even when you have lambdas


A new concern from me is that this change would allow Iterable and
Stream to be used in foreach, but not Iterator. This seems like an
odd/sharp conceptual edge.

Why not make `Iterator` implement `IterableOnce`? The default method
would obviously just return `this`.

It seems to me that if we are willing to countenance foreach over a
one-shot Stream, it is inappropriate to deny foreach to a one-shot
Iterator.

thanks
Stephen

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


Re: RFR 8220613: [TEST] java/util/Arrays/TimSortStackSize2.java times out with fastdebug build

2019-03-14 Thread Lance Andersen
Thank you Roger for addressing this 

Looks good

> On Mar 14, 2019, at 10:37 AM, Roger Riggs  wrote:
> 
> Please review a test configure change to skip running TimSortStackSize2
> on the fast debug build. The running time of the test is excessive.
> This particular test has been timing out intermittently for a long time.
> 
> Webrev:
>   http://cr.openjdk.java.net/~rriggs/webrev-timsort-timeout-8220613/
> 
> Issue:
>   https://bugs.openjdk.java.net/browse/JDK-8220613
> 
> Thanks, Roger
> 

 
  

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





RFR 8220613: [TEST] java/util/Arrays/TimSortStackSize2.java times out with fastdebug build

2019-03-14 Thread Roger Riggs

Please review a test configure change to skip running TimSortStackSize2
on the fast debug build. The running time of the test is excessive.
This particular test has been timing out intermittently for a long time.

Webrev:
  http://cr.openjdk.java.net/~rriggs/webrev-timsort-timeout-8220613/

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

Thanks, Roger



Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread Brian Goetz




It think this alternative is not given fair comparison. 1st this is an 
instance method, so the foreach loop should read:


for (T t : stream.asIterable()) {
    ...
}


Let's keep sight of the goal here, which is: people find it a gap that 
Stream does not play cleanly with foreach.  And the main knock against 
this approach (besides the chorus of "that's not what we wanted" we are 
sure to get) is that it is not really much more discoverable than some 
of the other workarounds (such as casting stream::iterator to Iterable.)





And now for something more controversial...

Is changing the language really out of the picture here?


Yes.  This issue was extensively litigated during JSR-335, and it was 
decided that one language-to-library tunnel (Iterable) here was all we 
wanted.  And there's been no new evidence since then that we want to 
change the language for this.


There's a reason it took as long as it did for Stuart to come up with 
this proposal; all the options were known for years, they all have 
problems, and the net benefit is still relatively narrow, which means we 
don't have a lot of budget before the cost-benefit becomes negative.  I 
think the option proposed is the least-worst, and people still seem to 
really want to be able to foreach over streams.




RFR: Container Fixes (8219652, 8217766, 8212528)

2019-03-14 Thread Bob Vandette
Ping ...


Please review these three fixes for Linux Docker/cgroup container support.

WEBREV:

http://cr.openjdk.java.net/~bobv/8217766/webrev.0

ISSUE1:

https://bugs.openjdk.java.net/browse/JDK-8219562 - Line of code in 
osContainer_linux.cpp#L102 appears unreachable

This change corrects a rarely used hotspot code path making it compatible with 
the Java based Metrics.

ISSUE2:

https://bugs.openjdk.java.net/browse/JDK-8212528 - Wrong cgroup subsystem being 
used for some CPU Container Metrics

Most Linux distros provide symbolic links for cpuacct and cpu controller 
directories.  Docker on the Mac does not.
This causes some of the cpu statistics to be unreported since the directory 
name was incorrect.

ISSUE3:

https://bugs.openjdk.java.net/browse/JDK-8217766 - Container Support doesn't 
work for some Join Controllers combinations

The cgroup identification -implemented by parsing /proc/self/mountinfo
and /proc/self/cgroup- assumed each cgroup controller was mounted
disjoint from the others (except for "cpu" and "cpuacct" controllers).
Which means, we expected one single controller per mountinfo line.

This matches the way most Linux distributions currently configure
cgroupsv1 by default. Yet controllers can be grouped arbitrarily.
For instance, using the JoinControllers systemd directive.
One use case for that is to let Kubernetes' kubelet discover his own
dedicated and reserved cgroup hierarchy. In that situation, the JVM
fails to discover the expected cgroup controllers set, and, when running
containerized, default to a suboptimal understanding of available resources.

Supporting arbitrarily controllers groups per mountpoint actually
allows for simpler and easier to follow code, as we don't need nested
if/else for every controller.

This fix also updates the Containers Metrics, to support joint controllers.


Bob.

Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread Peter Levart

Hi Stuart,

The Alternatives section of the proposal is very thorough and it 
mentions the following alternative:


"
    An alternative adapter is to add a default method to Stream:

    default Iterable asIterable() { return this::iterator; }

    for (T t : asIterable(stream)) {
        ...
    }

    This is slightly better at the call site, and it’s restricted to 
streams. But it’s still not as nice as IterableOnce and it facilitates 
creation of poorly-behaved Iterable instances.

"

It think this alternative is not given fair comparison. 1st this is an 
instance method, so the foreach loop should read:


for (T t : stream.asIterable()) {
    ...
}

...which is better in particular when creation of Stream and consumption 
is performed in single expression (not a good example, since it doesn't 
close the stream):


for (String line : Files.lines(path).asIterable()) {
    ...
}

In addition, why should this be restricted to streams? This could be 
combined with IterableOnce. The method could be called differently and 
specified as BaseStream's terminal operation, returning IterableOnce 
instead of Iterable:


    /**
 * Returns an {@link IterableOnce} for elements of this stream.
 *
 * This is a terminal
 * operation.
 *
 * @return iterable for elements of this stream that may be 
iterated only once

 */
    default IterableOnce iterate() {
    Iterator iterator = iterator();
    return () -> iterator; // this should be a proper IterableOnce 
with exception thrown etc.

    }

So IntStream and friends would get this too, although with necessary 
boxing, which should be easy for JIT to eliminate and without conflicts 
between IntStream.forEach and Iterable.forEach.



What do you think? Is this additional method invocation too much of a 
drawback?



And now for something more controversial...

Is changing the language really out of the picture here?

The Iterator interface has got a new default method called 
forEachRemaining in JDK 8. This method could be seen roughly equivalent 
to one-shot iteration directly from IterableOnce:


IterableOnce.forEach() vs. IterableOnce.iterator().forEachReamining()

Both of above can only be performed once. And once is enough for foreach 
loop. So if foreach loop was retrofitted to also act on Iterator(s) as a 
possible right hand side (arrays, Iterable(s), Iterator(s)), then we 
might not need this IterableOnce at all. You could then just write:


for (T t : stream.iterator()) ...

But that's probably not going to happen right?


Regards, Peter


On 3/12/19 10:59 PM, Stuart Marks wrote:

Hi Stephen,


My slight concern is that the terminal operation is hidden and not
immediately visible, which could be surprising. I do note that streams
throw a clear exception if a terminal operation is called a second
time which mitigates this to some degree.


Yes, this is certainly a possibility.

I'll note that even though my example does it, I think that storing a 
stream in a local variable is a bit of a code smell. It has to be done 
for try-with-resources, but storing the head of a pipeline in a local 
variable so that it can be iterated over does introduce the 
possibility of accidental reuse.


I suspect that most cases (aside from try-with-resources) will call a 
method returning a fresh Stream that's then iterated over. For example,


    for (var x : getSomeStream()) {
   // ...
    }

In this case there's no possibility of accidental reuse.


The IterableOnce class-level Javadoc contradicts the method-level
Javadoc. The class-level say "It is recommended that" whereas the
method-level Javadoc mandates.


Oh yes, thanks. I'll fix this to make the behavior mandatory.

s'marks





Re: RFR 8220252: Fix Headings in java.naming

2019-03-14 Thread Daniel Fuchs

On 13/03/2019 19:13, Lance Andersen wrote:
Yes you are right, I missed that but it is updated at 
http://cr.openjdk.java.net/~lancea/8220252/webrev.01/index.html




Thanks Lance!

The new version looks good.

best regards,

-- daniel



Re: RFR 8220088 : Test "java/lang/annotation/loaderLeak/Main.java" fails with -Xcomp

2019-03-14 Thread Andrew Haley
On 3/14/19 1:08 AM, Martin Buchholz wrote:
> Why not Reference.reachabilityFence ?

Certainly, yes. Apart from anything else, it's much clearer and robust against
changes to HotSpot.

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


Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

2019-03-14 Thread forax
- Mail original -
> De: "Stuart Marks" 
> À: "Remi Forax" 
> Cc: "core-libs-dev" 
> Envoyé: Mardi 12 Mars 2019 22:45:12
> Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams

> Hi Remi,
> 
>> Stream.iterator() can be really really slow, it uses a pull semantics while
>> the whole Stream push values. When designing it, the lambda EG saw it as an
>> "escape hatch" in order to interropt with a legacy code than require an
>> Iterator and not more.
> 
> If Stream.iterator() is slow, then perhaps it needs to be optimized. Tagir had
> some ideas for how to do that. Of course, I don't know if that's exactly the
> right way; some additional investigation should be done. But poor performance
> relative to Spliterator.tryAdvance() or forEachRemaining() shouldn't be an
> argument against doing this. People repeatedly bump into the gap in the
> programming model between streams and the enhanced-for loop, and it's time to
> fill it in.

I'm talking about the perf difference between stream.forEach and for(var 
element: stream), forEachRemaining may be slower because for the VM the ideal 
case is to see the creation of the Stream and the call to the terminal 
operation inside the same inlining horizon so the creation of the Stream itself 
can be elided.

A bit of history: they have been several prototypes of how to implement the 
stream API before the current one, one of them (i think it's the first one) was 
based on iterators and iterators of iterators, one for each step of the Stream. 
The perf of that implementation was good until there was too many intermediary 
ops calls on the Stream and at that point perf were really bad. It's because 
the VM has two way to find the type of something in a generic code, it can 
build a profile by remembering what class was used for a method call or it can 
propagate the type of an argument to the type of the corresponding parameter. 
Because an iterator stores the element to return in a field, you are loosing 
the later way to optimize and the former only work if you have no more than 2 
different classes in the profile.
So while Stream.iterator() may be optimized, it's not that simple.

> 
>> This proposal has the side effect of making Stream more different from its
>> primitive counterpart IntStream, LongStream and DoubleStream which may be
>> problematic because we are trying to introduce reified generics as part of
>> Valhalla (there is a recent mail of Brian about not adding methods to
>> OptionalInt for the same reason).
> 
> Well, yes, I think that it means that Stream evolves somewhat independently of
> Int/Long/DoubleStream, but I don't see that this imposes an impediment on
> generic specialization in Valhalla. In that world, Stream should (mostly)
> just work. It may also be possible in a specialized world to add the specific
> things from IntStream (such as sum() and max()) to Stream.

We may want more here, like having Stream being a subtype of IntStream so 
there is only one implementation for IntStream and Stream.
Thus adding a method that make IntStream and Stream different just make 
this kind of retrofitting more unlikely. 

> 
>> And, the real issue is how to deal with checked exceptions inside the Stream
>> API, i would prefer to fix that issue instead of trying to find a way to
>> workaround it.
> 
> Well I'd like to have a solution for checked exceptions as well, but there
> doesn't appear to be one on the horizon. I mean, there are some ideas floating
> around, but nobody is working on them as far as I know.

as far as i know, there are two of them,
- one is to get ride of checked exception, even Kotlin which tout itself as a 
language that is more safe that Java doesn't have checked exception, basically 
Java is the only language that run of the JVM and have checked exception. 
- the other is to automatically wrap checked exceptions into a corresponding 
unchecked exception by letting the compiler generate the code that users 
currently write when the checked exception appear some context
  by example with the keyword autowrap,
  - you have the autowrap block (syntactically like a synchronized block)
  autowrap {
return Files.newInputStream(path);   // IOException is transformed to 
UncheckedIOException by calling IOException.wrap()
  }
  - you can use autowrap on a method declaration
 void foo(Path path) autowrap {
   return Files.newInputStream(path);   // IOException is transformed to 
UncheckedIOException by calling IOException.wrap()
 }
  - you can use autowrap with a functional interface
 void runBlock(autoWrap Consumer consumer) { ... }
 ...
 runblock(() -> {
   Files.newInputStream(path); // IOException is transformed to 
UncheckedIOException by calling IOException.wrap()
 });


> 
> But checked exceptions aren't the only reason to prefer iteration in some 
> cases;
> loops offer more flexible control flow (break/continue) and easier handling of
> side