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

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

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

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

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

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,

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

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:

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

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

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

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: 

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

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

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

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

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

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

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

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

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,

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

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,

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

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

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

- 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

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

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

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

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

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

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.

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

> 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`?

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

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

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

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,

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

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

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

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)

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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:

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

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

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

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

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

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

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

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

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

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/

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

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

RFR: Container Fixes (8219652, 8217766, 8212528)

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

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

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

Re: RFR 8220252: Fix Headings in java.naming

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

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

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

- 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