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

2019-03-15 Thread Stuart Marks
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 Str

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

2019-03-15 Thread Claes Redestad
On 2019-03-15 19:06, Peter Levart wrote: On 3/15/19 5:14 PM, Claes Redestad wrote: Hi Peter, thanks for reviewing! Interning arbitrary attribute names might have some unfortunate side- effects, You mean if the number of different names would get larger and larger on a long-running JVM,

RFR: JDK-8220264: msi installer does not remove some files if app installed into non-default location

2019-03-15 Thread Alexander Matveev
Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). - Problem was that we did not restore APPLICATIONFOLDER correctly and it was set to default value. Fixed by forcing reading registry from 64-bit. [1]

Re: 8219196: DataOutputStream.writeUTF may throw unexpected exceptions

2019-03-15 Thread Martin Buchholz
Looks good to me. On Fri, Mar 15, 2019 at 2:06 PM Brian Burkhalter < brian.burkhal...@oracle.com> wrote: > Updated: http://cr.openjdk.java.net/~bpb/8219196/webrev.01/ > > On Mar 15, 2019, at 1:05 PM, Brian Burkhalter > wrote: > > On Mar 15, 2019, at 12:57 PM, Roger Riggs wrote: > > Looks ok. >

Re: 8219196: DataOutputStream.writeUTF may throw unexpected exceptions

2019-03-15 Thread Brian Burkhalter
Updated: http://cr.openjdk.java.net/~bpb/8219196/webrev.01/ > On Mar 15, 2019, at 1:05 PM, Brian Burkhalter > wrote: > >> On Mar 15, 2019, at 12:57 PM, Roger Riggs > > wrote: >> >> Looks ok. >> >> In the test, you could probably create the string using. >>"\

Re: 8219196: DataOutputStream.writeUTF may throw unexpected exceptions

2019-03-15 Thread Brian Burkhalter
Hi Roger, > On Mar 15, 2019, at 12:57 PM, Roger Riggs wrote: > > Looks ok. > > In the test, you could probably create the string using. > "\u0800".repeat(size); Good idea. > Instead of disabling the test statically, you could make it conditional > on Runtime.maxMemory but the test will fa

Re: 8219196: DataOutputStream.writeUTF may throw unexpected exceptions

2019-03-15 Thread Brian Burkhalter
So like this: static int writeUTF(String str, DataOutput out) throws IOException { int strlen = str.length(); // use charAt instead of copying String to char array int utflen = 0; for (int i = 0; i < strlen && utflen < 65536; i++) { int c = str.char

Re: 8219196: DataOutputStream.writeUTF may throw unexpected exceptions

2019-03-15 Thread Roger Riggs
Hi Brian, Looks ok. In the test, you could probably create the string using.     "\u0800".repeat(size); Instead of disabling the test statically, you could make it conditional on Runtime.maxMemory but the test will fail quickly anyway. DataOutputStream.java: Can you fix the indent lines 387

Re: 8219196: DataOutputStream.writeUTF may throw unexpected exceptions

2019-03-15 Thread Martin Buchholz
On Fri, Mar 15, 2019 at 12:46 PM Brian Burkhalter < brian.burkhal...@oracle.com> wrote: > May I ask “why” if it cannot exceed 65535? > You're right, that was a bad idea ... The repetition of the overflow check bothers me. Another idea: simply move the existing utflen > 65535 into the loop. With

Re: 8219196: DataOutputStream.writeUTF may throw unexpected exceptions

2019-03-15 Thread Brian Burkhalter
May I ask “why” if it cannot exceed 65535? > On Mar 15, 2019, at 12:40 PM, Martin Buchholz wrote: > > Consider changing utflen to a long. > > On Fri, Mar 15, 2019 at 12:25 PM Brian Burkhalter > mailto:brian.burkhal...@oracle.com>> wrote: > For [1] please review the proposed fix [2]. It is poss

Re: 8219196: DataOutputStream.writeUTF may throw unexpected exceptions

2019-03-15 Thread Martin Buchholz
Consider changing utflen to a long. On Fri, Mar 15, 2019 at 12:25 PM Brian Burkhalter < brian.burkhal...@oracle.com> wrote: > For [1] please review the proposed fix [2]. It is possible to preemptively > detect a sufficient condition for when the length of the modified UTF-8 > encoding of the stri

8219196: DataOutputStream.writeUTF may throw unexpected exceptions

2019-03-15 Thread Brian Burkhalter
For [1] please review the proposed fix [2]. It is possible to preemptively detect a sufficient condition for when the length of the modified UTF-8 encoding of the string parameter will exceed the maximum allowed value and thereby avert any numerical overflow in incrementing the accumulator which

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

2019-03-15 Thread Martin Buchholz
There are many nanoTime-based calculations in j.u.concurrent and indeed we use a variable named "deadline" a lot. final long deadline = System.nanoTime() + nanosTimeout; But different classes use nanoTime in subtly different ways so there's no copy/paste idiom. When we updated the spec for nanoT

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

2019-03-15 Thread Mandy Chung
That's a good suggestion. The JDK out-of-the-box case and the complex environment case with instrumentation agents transforming the bytecodes are two different use cases. This brings up one issue that when constructing the enhanced NPE message, a method might become obsolete due to redefintion,

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

2019-03-15 Thread Roger Riggs
Hi Ivan, The updated code looks fine except for style. Please put the return and the if's on different lines (by the style conventions). Line 210 and 211, and 216. Thanks, Roger On 03/15/2019 04:08 AM, Ivan Gerasimov wrote: Hi David! The code could be written like following: long start =

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

2019-03-15 Thread Peter Levart
On 3/15/19 5:14 PM, Claes Redestad wrote: Hi Peter, thanks for reviewing! Interning arbitrary attribute names might have some unfortunate side- effects, You mean if the number of different names would get larger and larger on a long-running JVM, the cache would grow without bounds? I n

Re: jpackage ALL-SYSTEM

2019-03-15 Thread Michael Hall
> On Mar 8, 2019, at 8:57 AM, Michael Hall wrote: > > I have made changes to my application that make it mostly functional with the > exception of JMX pid attach as built by jpackage. > I thought I had this functionality working when I first got the application > to use Java 9 but it no lon

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

2019-03-15 Thread Maurizio Cimadamore
You touch an important point here - who is this feature for? Is it for the casual developer in need of some hand holding? Or is it for diagnosing complex instrumented stacks? I think the two use cases are not necessarily similar and might be served by different sets of enhancements. In the latt

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

2019-03-15 Thread Claes Redestad
Hi Peter, thanks for reviewing! Interning arbitrary attribute names might have some unfortunate side- effects, but a tiny - maybe one or two element - cache might be sufficient to keep the churn low in practice. Name and SHA1-Digest are the most commonly repeated non-constant attribute, generat

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

2019-03-15 Thread Peter Levart
Hi Claes, If you have observed memory churn allocating Name objects when reading jar files, then perhaps we could do something to the logic of Attributes class so that lazily allocated Name(s) would get interned too? As a separate change of course. This looks good as is. Regards, Peter On 3

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

2019-03-15 Thread Alan Bateman
On 15/03/2019 13:45, Claes Redestad wrote: I have a solution to the heap dump issues out for review[1], so I've cleaned up this patch, verified the failing tests pass locally and am running both through tier1-3: http://cr.openjdk.java.net/~redestad/8214712/open.01/ This version looks good

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

2019-03-15 Thread Claes Redestad
Hi, On 2019-03-14 18:20, Claes Redestad wrote: On 2019-03-14 18:13, Alan Bateman wrote: 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 trigge

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

2019-03-15 Thread Lindenmaier, Goetz
Hi Maurizio, thanks for sharing your patch! This is about what I thought about so far, just that it's working already :) It prints the information of the failing bytecode. Adding the dataflow analysis, the other information could be printed as well. The major problem I see is here: +

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

2019-03-15 Thread Alan Bateman
On 15/03/2019 06:43, David Holmes wrote: Yes and on closer examination you will find a lot of inconsistencies. RESTARTABLE goes back a long way and many of the I/O APIs have switched locations over the years. Stuff was copied from the HPI layer into hotspot, then back to the JDK and sometimes

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

2019-03-15 Thread Pavel Rappo
Thanks for the clarifications. The change looks fine now. I felt initial unease because, in my experience, it's so easy to overlook some subtlety when working with overflowing integers. For `System.currentTimeMillis` the code like: stopTime - startTime >= timeout doesn't raise one's eyebrow

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

2019-03-15 Thread Alan Bateman
On 14/03/2019 21:33, Igor Ignatyev wrote: 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. Thanks for checking, in which case I think we need to find a better name for this test as JniI

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

2019-03-15 Thread Maurizio Cimadamore
Hi Goetz, please find the attached ASM-based patch. It is just a PoC, as such it does not provide as fine-grained messages as the one discussed in the RFE/JEP, but can be enhanced to cover custom debugging attribute, I believe. When running this: Object o = null; o.toString(); you get: Exce

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

2019-03-15 Thread Lindenmaier, Goetz
Hi everybody, Mark, I followed your advice and created a JEP: https://bugs.openjdk.java.net/browse/JDK-8220715 Please point me to things I need to improve formally, this is my first JEP. Also feel free to fix the administrative information in the Jira issue if it is wrong. And, naturally, you'r

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

2019-03-15 Thread Ivan Gerasimov
On 3/15/19 1:51 AM, Peter Levart wrote: On 3/15/19 9:38 AM, Ivan Gerasimov wrote: Hi Peter! On 3/15/19 1:24 AM, Peter Levart wrote: On 3/15/19 9:03 AM, fo...@univ-mlv.fr wrote: * @since 13 */ interface Once {} What do you think of that? It's not clear to me if an an

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

2019-03-15 Thread Peter Levart
On 3/15/19 9:38 AM, Ivan Gerasimov wrote: Hi Peter! On 3/15/19 1:24 AM, Peter Levart wrote: On 3/15/19 9:03 AM, fo...@univ-mlv.fr wrote:   * @since 13   */ interface Once {} What do you think of that? It's not clear to me if an annotation, available at runtime, is not a b

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

2019-03-15 Thread Ivan Gerasimov
Hi Peter! On 3/15/19 1:24 AM, Peter Levart wrote: On 3/15/19 9:03 AM, fo...@univ-mlv.fr wrote: * @since 13 */ interface Once {} What do you think of that? It's not clear to me if an annotation, available at runtime, is not a better fit. Anyway, i'm not sure not sure intro

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

2019-03-15 Thread Peter Levart
On 3/15/19 9:24 AM, Peter Levart wrote: On 3/15/19 9:03 AM, fo...@univ-mlv.fr wrote: * @since 13 */     interface Once {} What do you think of that? It's not clear to me if an annotation, available at runtime, is not a better fit. Anyway, i'm not sure not sure introducing s

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

2019-03-15 Thread Peter Levart
On 3/15/19 9:03 AM, fo...@univ-mlv.fr wrote: * @since 13 */     interface Once {} What do you think of that? It's not clear to me if an annotation, available at runtime, is not a better fit. Anyway, i'm not sure not sure introducing such interface/annotation worth its mainten

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

2019-03-15 Thread Ivan Gerasimov
Hi David! The code could be written like following: long start = System.nanoTime(); do { ... long elapsed = System.nanoTime() - start; remainingNanos = timeoutNanos - elapsed; } while (remainingNanos > 0); but then one realizes that this always calculates (start + timeoutNanos), bo

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

2019-03-15 Thread forax
- Mail original - > De: "Peter Levart" > À: "Remi Forax" > Cc: "Brian Goetz" , "Stuart Marks" > , "core-libs-dev" > > Envoyé: Vendredi 15 Mars 2019 08:57:10 > Objet: Re: I don't understand why we need IterableOnce ? Was: Proposal: > JDK-8148917 Enhanced-For Statement Should Allow > Str

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

2019-03-15 Thread Lindenmaier, Goetz
Hi Maurizio, > > 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 Ja

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

2019-03-15 Thread Peter Levart
Hi, On 3/14/19 9:51 PM, Remi Forax wrote: 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

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

2019-03-15 Thread Ivan Gerasimov
Thank you David for the detailed analysis! I'm having an impression that the general direction has been to add the RESTARTABLE loops where sensible. (E.g. a recent fix for JDK-8197498, and lots of older similar bugs.) I understand that the fix only touches a small portion of potentially pro

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

2019-03-15 Thread David Holmes
On 15/03/2019 5:03 pm, Ivan Gerasimov wrote: Thank you David! On 3/14/19 11:01 PM, David Holmes wrote: Hi Ivan, On 15/03/2019 12:02 pm, Ivan Gerasimov wrote: 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 imple

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

2019-03-15 Thread Ivan Gerasimov
Thank you David! On 3/14/19 11:01 PM, David Holmes wrote: Hi Ivan, On 15/03/2019 12:02 pm, Ivan Gerasimov wrote: 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) d

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

2019-03-15 Thread Peter Levart
On 3/15/19 12:16 AM, Stephen Colebourne wrote: 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 th