Re: 6516099: InputStream.skipFully(int k) to skip exactly k bytes

Hi Sergey, That is rather ugly. Thanks for pointing it out. The previous version which merely repeatedly read and discarded a buffer of bytes did not have this problem, but it also did not take advantage of any performance improvement to be obtained by subclass overriding of skip(). It does

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

Hello, I think I neglected to post the link here before, but the Compatibility and Specification Request (CSR) corresponding to this issue is at [2]. It has a specdiff attached to it which would be helpful in comparing the changes to the javadoc specification of {Double,Float}.toString(). Any

Re: 6516099: InputStream.skipFully(int k) to skip exactly k bytes

Hi, Brian. It looks like the new version will not throw EOFException if the "skip(n)" method will skip more bytes than requested, it is possible for example in FileInputStream.skip(long n); "This method may skip more bytes than what are remaining in the * backing file. This produces no

RE: RFR(s): 8210549: Runtime.exec: in closeDescriptors(), use FD_CLOEXEC instead of close()

Sorry, I meant to say: "Unfortunately, that is only for files... sockets in Linux are file descriptors as well, but OpenJDK is (currently) not opening them with CLOEXEC." -Original Message- From: core-libs-dev On Behalf Of Andrew Luo Sent: Wednesday, October 24, 2018 1:18 PM To:

Re: Review Request JDK-8212948: Remove unused jdk.internal.misc.VMNotification interface

+1 > On Oct 24, 2018, at 6:17 PM, Mandy Chung wrote: > > Remove a leftover file from JDK-8159590 that removed > jdk.internal.misc.VM.registerVMNotification method taking > VMNotification parameter. > > $ hg remove src/java.base/share/classes/jdk/internal/misc/VMNotification.java > > Mandy > >

Review Request JDK-8212948: Remove unused jdk.internal.misc.VMNotification interface

Remove a leftover file from JDK-8159590 that removed jdk.internal.misc.VM.registerVMNotification method taking VMNotification parameter. $ hg remove src/java.base/share/classes/jdk/internal/misc/VMNotification.java Mandy diff --git

Re: RFR: JDK-8212828 Allow POSIX_SPAWN to be used for ProcessImpl on Linux

Hi Roger, On Wed 24. Oct 2018 at 21:39, Roger Riggs wrote: > Hi Thomas, > > Sorry, I had the incantation for multiple tests wrong. > Separate test configurations need to be in separate /* */ comment blocks. > BTW, it creates separate .jtr files for each block. > > diff --git

RE: RFR(s): 8210549: Runtime.exec: in closeDescriptors(), use FD_CLOEXEC instead of close()

By the way, there is some related work on CLOEXEC for networking that I had done. It seems like this work is somewhat similar (although my work addresses a different use case - when JNI code calls exec on its own and doesn't go through and close its inherited FDs):

Re: JDK 12 RFR of JDK-8212880: Cannot access ftp: site for fdlibm.tar

Hi Joe, Looks fine. Brian > On Oct 24, 2018, at 1:11 PM, joe darcy wrote: > > Seems the ftp option for the fdlibm sources has gone away; the URL reference > in the JDK sources in StrictMath should be updated to a live page: > > JDK-8212880: Cannot access ftp: site for fdlibm.tar >

Re: JDK 12 RFR of JDK-8212880: Cannot access ftp: site for fdlibm.tar

looks fine joe > On Oct 24, 2018, at 4:11 PM, joe darcy wrote: > > Hello, > > Seems the ftp option for the fdlibm sources has gone away; the URL reference > in the JDK sources in StrictMath should be updated to a live page: > > JDK-8212880: Cannot access ftp: site for fdlibm.tar > >

Re: JDK 12 RFR of JDK-8212880: Cannot access ftp: site for fdlibm.tar

+1 -- Jon On 10/24/2018 01:11 PM, joe darcy wrote: Hello, Seems the ftp option for the fdlibm sources has gone away; the URL reference in the JDK sources in StrictMath should be updated to a live page:         JDK-8212880: Cannot access ftp: site for fdlibm.tar Patch: diff -r

JDK 12 RFR of JDK-8212880: Cannot access ftp: site for fdlibm.tar

Hello, Seems the ftp option for the fdlibm sources has gone away; the URL reference in the JDK sources in StrictMath should be updated to a live page:         JDK-8212880: Cannot access ftp: site for fdlibm.tar Patch: diff -r 211998500d39

Re: 6516099: InputStream.skipFully(int k) to skip exactly k bytes

Hi Andrew, Thanks for the suggestion. Will consider it for the final check-in. Brian > On Oct 24, 2018, at 12:39 PM, Andrew Luo > wrote: > > I'm not a reviewer/committer but just a suggestion: > > You can use assert to ensure "document" that skip(long) should never return > negative (and

Re: 6516099: InputStream.skipFully(int k) to skip exactly k bytes

Hi Brent, Sadly we had a sort of race condition going on here. Here is one more update which gets rid of excess code, namely the discardNBytes() method: http://cr.openjdk.java.net/~bpb/6516099/webrev.03/ This implementation assumes that in

RE: 6516099: InputStream.skipFully(int k) to skip exactly k bytes

I'm not a reviewer/committer but just a suggestion: You can use assert to ensure "document" that skip(long) should never return negative (and also provide useful debugging when assertions are enabled), for example: 586 long ns = skip(n); assert ns >= 0 : "skip(long) returned

Re: RFR: JDK-8212828 Allow POSIX_SPAWN to be used for ProcessImpl on Linux

Hi Thomas, Sorry, I had the incantation for multiple tests wrong. Separate test configurations need to be in separate /* */ comment blocks. BTW, it creates separate .jtr files for each block. diff --git a/test/jdk/java/lang/ProcessBuilder/Basic.java

Re: Fwd: RFR: JDK-8212694 - Using Raw String Literals with align() and Integer.MIN_VALUE causes out of memory error

looks good. -sherman *From: *Jim Laskey > *Subject: **RFR: JDK-8212694 - Using Raw String Literals with align() and Integer.MIN_VALUE causes out of memory error* *Date: *October 19, 2018 at 2:50:19 PM ADT *To: *core-libs-dev

Re: 6516099: InputStream.skipFully(int k) to skip exactly k bytes

Hi, I think that's looking pretty good. One thing: InputStream.java 515 * @param n the number of bytes to be skipped. How about an @param for throwOnEOF, too? Regarding the tests: NullInputStream.java 237 @Test(groups = "closed") 238 public static void

Re: RFR(s): 8210549: Runtime.exec: in closeDescriptors(), use FD_CLOEXEC instead of close()

"Do we need the fallback on your beloved HP-UX?" We do not ship OpenJDK on HPUX, so I do not really care. And I really do not love it :) ..Thomas On Wed, Oct 24, 2018 at 8:16 PM Thomas Stüfe wrote: > > Hi Martin, > > sorry for letting this rest for so long - I got sidetracked by other issues. >

Re: RFR(s): 8210549: Runtime.exec: in closeDescriptors(), use FD_CLOEXEC instead of close()

Hi Martin, sorry for letting this rest for so long - I got sidetracked by other issues. On Wed, Sep 12, 2018 at 3:17 PM Martin Buchholz wrote: > > In 2018, we can assume everyone has implemented FD_CLOEXEC. > > Back in 1970, 20 file descriptors was enough for any program, but > today they're

Re: RFR: JDK-8212828 Allow POSIX_SPAWN to be used for ProcessImpl on Linux

Hi Roger, On Wed, Oct 24, 2018 at 5:23 PM Roger Riggs wrote: > > Hi Thomas, > > Thanks for adding the test. > > There's a feature of jtreg that was exposed a couple of month ago that > can be used to deal with running the test too many times. > > There can be multiple @test blocks with different

RFR 8212129: Remove finalize methods from java.util.zip.ZipFIle/Inflator/Deflator

Hi all, This change removes the finalize methods from java.util.zip.ZipFIle/Inflator/Deflator. These methods were deprecated and marked for removal in JDK 9 The webrev can be found at: http://cr.openjdk.java.net/~lancea/8212129/webrev.00/

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

Hello, Nice to see this finally happening! Are we actually adding a new demo? I thought we were working towards getting rid of the demos completely. CompileJavaModules.gmk: The jdk.packager_CLEAN_FILES could be replaced with a simple "jdk.packager_CLEAN := .properties" unless you actually

Re: [12] RFR of JDK-8210908: Refactor java/util/prefs/PrefsSpi.sh to plain java test

Super - thanks, Amy. -Brent On 10/23/18 7:07 PM, Amy Lu wrote: Thank you Brent for the comments! All fixed in the new webrev: http://cr.openjdk.java.net/~amlu/8210908/webrev.01/ Thanks, Amy On 2018/10/24 4:39 AM, Brent Christian wrote: Hi, Amy I think this looks quite good as it is. 

Re: [12] RFR of JDK-8209768: Refactor java/util/prefs/CheckUserPrefsStorage.sh to plain java test

Looks great. -Brent On 10/24/18 2:04 AM, Amy Lu wrote: Please review the updated version: http://cr.openjdk.java.net/~amlu/8209768/webrev.01/ Thanks, Amy Other opinions? Thanks, -Brent On 10/22/18 8:46 PM, Amy Lu wrote: Please review. Thanks, Amy On 2018/10/11 4:47 PM, Amy Lu

Re: RFR: 8212726: Replace some use of drop- and foldArguments with filtering argument combinator in StringConcatFactory

This would definitely be a useful addition to the public MethodHandles API. Achieving this functionality with currently available means is extremely tricky and certainly does not produce readable or maintainable code. Hannes > Am 22.10.2018 um 14:07 schrieb Jim Laskey : > > Thank you for

Re: RFR: JDK-8212828 Allow POSIX_SPAWN to be used for ProcessImpl on Linux

Oops, it dropped the newlines. * @run main/othervm/timeout=300 Basic * @run main/othervm/timeout=300 -Djdk.lang.Process.launchMechanism=fork Basic * * @test * @requires (os.family == "linux") * @run main/othervm/timeout=300 -Djdk.lang.Process.launchMechanism=posix_spawn

Re: RFR: JDK-8212828 Allow POSIX_SPAWN to be used for ProcessImpl on Linux

Hi Thomas, Thanks for adding the test. There's a feature of jtreg that was exposed a couple of month ago that can be used to deal with running the test too many times. There can be multiple @test blocks with different @requires. * @run main/othervm/timeout=300 Basic * @run

Re: RFR: JDK-8212828 Allow POSIX_SPAWN to be used for ProcessImpl on Linux

Hi all, version 2 of Davids patch, with test changes: http://cr.openjdk.java.net/~stuefe/webrevs/JDK-8212828-posix_spawn.patch/webrev.01/webrev/ Executed the test on my local Ubuntu box, works fine. Submit job runs. About the test: I added a new line: * @run main/othervm/timeout=300 Basic

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

On 23/10/2018 16:49, Andy Herrick wrote: This patch implements the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool jpackager is a simple packaging tool, based on the JavaFX |javapackager| tool, that:  * Supports

Re: RFR: JDK-8212828 Allow POSIX_SPAWN to be used for ProcessImpl on Linux

On Wed, Oct 24, 2018 at 3:56 PM David Lloyd wrote: > > On Wed, Oct 24, 2018 at 8:52 AM Thomas Stüfe wrote: > > > > Hi David, > > > > I think we need some form of test, as Alan indicated. > > > > java/lang/ProcessBuilder/Basic.java should run through. Actually, I > > would like it if we would run

Re: RFR: JDK-8212828 Allow POSIX_SPAWN to be used for ProcessImpl on Linux

On Wed, Oct 24, 2018 at 8:52 AM Thomas Stüfe wrote: > > Hi David, > > I think we need some form of test, as Alan indicated. > > java/lang/ProcessBuilder/Basic.java should run through. Actually, I > would like it if we would run this test for all valid enabled launch > mechanisms, regardless which

Re: RFR: JDK-8212828 Allow POSIX_SPAWN to be used for ProcessImpl on Linux

Hi David, I think we need some form of test, as Alan indicated. java/lang/ProcessBuilder/Basic.java should run through. Actually, I would like it if we would run this test for all valid enabled launch mechanisms, regardless which one is default. Can this be done? To save time I will only rerun

Re: RFR: JDK-8212828 Allow POSIX_SPAWN to be used for ProcessImpl on Linux

On Wed, Oct 24, 2018 at 1:05 AM Thomas Stüfe wrote: > Review: > > - copyright dates need updating on the C-sources > > - I opt for "#if defined(__solaris__) || defined(_ALLBSD_SOURCE) || > defined(_AIX) || defined(__linux__)" to be removed completely from > unix-specific source files. The ifdef

Re: [12] RFR of JDK-8209768: Refactor java/util/prefs/CheckUserPrefsStorage.sh to plain java test

On 2018/10/24 2:24 AM, Brent Christian wrote: Hi, Amy So one difference from the original test is that the process that creates the preferences store is still active at the time that the preference is checked.  I don't know if this is material to what is being tested or not. It would be

Re: RFR: JDK-8212828 Allow POSIX_SPAWN to be used for ProcessImpl on Linux

jdk-submit tests were clean. ..Thomas On Wed, Oct 24, 2018 at 7:51 AM Thomas Stüfe wrote: > > For the convenience of the reviewers, here webrev and bug: > > https://bugs.openjdk.java.net/browse/JDK-8212828 > http://cr.openjdk.java.net/~stuefe/webrevs/JDK-8212828-posix_spawn.patch/webrev/ > >

Re: RFR: JDK-8212828 Allow POSIX_SPAWN to be used for ProcessImpl on Linux

On 24/10/2018 06:51, Thomas Stüfe wrote: For the convenience of the reviewers, here webrev and bug: https://bugs.openjdk.java.net/browse/JDK-8212828 http://cr.openjdk.java.net/~stuefe/webrevs/JDK-8212828-posix_spawn.patch/webrev/ submit tests are currently running. Adding the posix_spawn to

Re: RFR: JDK-8212828 Allow POSIX_SPAWN to be used for ProcessImpl on Linux

Hi all, Basic considerations: This patch enables an untested function to the end user. I am unsure about this. I would feel better if we had something like "-XX:+UnlockDiagnosticVMOptions" for the JDK. That said, I can see the merit in having this switch to play around with. It would it make