Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

2019-05-14 Thread Martin Buchholz
I feel the same as David - reluctant to change anything. Introducing a raw type makes an ugly cast uglier. *From: *David Holmes > > > src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java > > As you note this should be ok'd by jsr166 folk so I've cc'd Martin > Buccholz. I

Re: RFR: 8222276: (zipfs) Refactoring and cleanups to prepare for JDK-8213031

2019-05-14 Thread Lance Andersen
Hi Chistoph, > On May 14, 2019, at 8:38 AM, Langer, Christoph > wrote: > > Hi Lance, > >> Thank you for taking this on. > > Thank you for reviewing! Unfortunately you didn't look at my latest > edition... (http://cr.openjdk.java.net/~clanger/webrevs/876.2/) Sorry, must have clicked on t

RFR: JDK-8223725: j.l.c.MethodHandleDesc::of throws undocumented exception IllegalArgumentException

2019-05-14 Thread Vicente Romero
Please review fix [1] for [2] and the corresponding CSR at [3]. The fix is just adding a missing @throws at method j.l.c.MethodHandleDesc::of. It is a one liner fix, Thanks, Vicente [1] http://cr.openjdk.java.net/~vromero/8223725/webrev.00/ [2] https://bugs.openjdk.java.net/browse/JDK-8223725

Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

2019-05-14 Thread David Holmes
Hi Christoph, I'm very reluctant to see changes like this that the compiler folk have not determined are actually incorrect. That said ... On 15/05/2019 7:03 am, Langer, Christoph wrote: Thanks Daniel. Can anybody help reviewing the changes to: src/java.base/share/classes/java/lang/invoke/Me

RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

2019-05-14 Thread Langer, Christoph
Thanks Daniel. Can anybody help reviewing the changes to: src/java.base/share/classes/java/lang/invoke/MethodHandles.java src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java and src/java.management/share/classes/java/lang/management/ManagementFactory.java ? Thanks Christop

RFR: JDK-8223723: j.l.c.MethodTypeDesc.dropParameterTypes​ throws the undocumented exception: IllegalArgumentException

2019-05-14 Thread Vicente Romero
Please review fix for [1] at [2]. The implementation of method java.lang.constant.MethodTypeDesc::dropParameterTypes was throwing a non specified exception. The proposed fix is synchronizing the implementation with the specification. Please also review the CSR at [3]. Check the problem section

Re: RFR(s): 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-14 Thread Florian Weimer
* Thomas Stüfe: > Right now I am worried more about things I cannot determine yet. Where > before we would wait for the pipe to get broken, now we have a read > call on the parent side, a write call on the child side, which both > must succeed. Could they fail sporadically, e.g. due to EINTR? I kn

Bug in jpackage early access regarding icons with the deb installer type

2019-05-14 Thread Sargun Vohra
I've found a bug in the jpackage tool early access which I obtained from https://jdk.java.net/jpackage/. The issue is that the icon path is incorrect in the generated desktop entry file. If there's a better place to report this, please let me know. Steps to reproduce the bug in Ubuntu are as follow

Re: [13] RFR: JDK-8206879: Currency decimal marker incorrect for Peru

2019-05-14 Thread naoto . sato
There is a typo in the test file name (and other related locations, such as class name, constructor name, etc): "Fomat" -> "Format" Otherwise looks good. Naoto On 5/14/19 4:19 AM, Deepak Kejriwal wrote: Thanks Ramanand and Naoto for review. Please find updated version of webrev:- http://cr.

Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

2019-05-14 Thread Daniel Fuchs
Hi Christoph, That looks much better, thanks! (but still not commenting on the other changes ;-)) best regards, -- daniel On 14/05/2019 13:57, Langer, Christoph wrote: Hi Daniel, unfortunately, your proposed solution does not work with javac. I get this in the build: Oh darn. I should hav

Re: 8223869: Problem list java/awt/FontMetrics/MaxAdvanceIsMax.java on more platforms

2019-05-14 Thread Phil Race
On 5/14/19 6:47 AM, Alan Bateman wrote: On 14/05/2019 14:31, Lindenmaier, Goetz wrote: Hi, I would like to problem list this test on ppc and s390. We see it failing, and it seems to be a Problem not related to the VM, see also the description in JDK-8221305

Re: 8223869: Problem list java/awt/FontMetrics/MaxAdvanceIsMax.java on more platforms

2019-05-14 Thread Phil Race
This isn't a core libs test, its a 2D test, so the request should go there. -phil. On 5/14/19 6:31 AM, Lindenmaier, Goetz wrote: Hi, I would like to problem list this test on ppc and s390. We see it failing, and it seems to be a Problem not related to the VM, see also the description in JDK-8

Re: RFR (M): JDK-6394757: rev1: AbstractSet.removeAll semantics are surprisingly dependent on relative sizes

2019-05-14 Thread Alan Snyder
Hi Stuart, I strongly disagree with this change. (Fortunately, I am not a Reviewer, just Opinionated.) Summary: 1. The change to removeAll() is a significant incompatible change because of its performance implications. 2. The benefit of the change to removeAll() is restricted to a few edge

Re: RFR 8223593 : Refactor code for reallocating storage

2019-05-14 Thread Roger Riggs
Hi Ivan, The updated patch looks fine. Strictly speaking, the change to Files.readAllBytes is not indicated by the bug report so please update or comment on the bug report to mention that change. Thanks, Roger On 05/13/2019 10:44 AM, Pavel Rappo wrote: Thanks for updating your patch. The u

Re: 8223869: Problem list java/awt/FontMetrics/MaxAdvanceIsMax.java on more platforms

2019-05-14 Thread Alan Bateman
On 14/05/2019 14:59, Lindenmaier, Goetz wrote: Hi Alan, thanks for looking at this. If the issue on these platforms is different JDK-8221305 then best to create a new issue and use that to exclude the test (you can't used JDK-8223869 as that is the issue you are using to add to the exclude

Re: RFR(s): 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-14 Thread Thomas Stüfe
On Tue, May 14, 2019 at 2:16 PM Florian Weimer wrote: > * David Lloyd: > > > Pipe communication isn't very costly AFAIK. The added complexity > > would be waiting for either a thing to appear on the pipe indicating > > success or else a waitpid/waitid+WNOHANG for exit code 127. But > > writing

RE: 8223869: Problem list java/awt/FontMetrics/MaxAdvanceIsMax.java on more platforms

2019-05-14 Thread Lindenmaier, Goetz
Hi Alan, thanks for looking at this. > If the issue on these platforms is different JDK-8221305 then best to > create a new issue and use that to exclude the test (you can't used > JDK-8223869 as that is the issue you are using to add to the exclude > list). Ah, no I think it's the same issue.

Re: 8223869: Problem list java/awt/FontMetrics/MaxAdvanceIsMax.java on more platforms

2019-05-14 Thread Alan Bateman
On 14/05/2019 14:31, Lindenmaier, Goetz wrote: Hi, I would like to problem list this test on ppc and s390. We see it failing, and it seems to be a Problem not related to the VM, see also the description in JDK-8221305 java/awt/FontMetrics/MaxAd

8223869: Problem list java/awt/FontMetrics/MaxAdvanceIsMax.java on more platforms

2019-05-14 Thread Lindenmaier, Goetz
Hi, I would like to problem list this test on ppc and s390. We see it failing, and it seems to be a Problem not related to the VM, see also the description in JDK-8221305 java/awt/FontMetrics/MaxAdvanceIsMax.java fails on MacOS + Solaris webrev:

Re: RFR(s): 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-14 Thread David Lloyd
Ah I see. I like the idea, it would mean pretty comprehensive error reporting and should work across platforms too I think. On Tue, May 14, 2019 at 7:16 AM Florian Weimer wrote: > > * David Lloyd: > > > Pipe communication isn't very costly AFAIK. The added complexity > > would be waiting for ei

RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

2019-05-14 Thread Langer, Christoph
Hi Daniel, > > unfortunately, your proposed solution does not work with javac. I get this > in the build: > > Oh darn. I should have double checked. > Can we at least reduce the scope of the @SuppressedWarnings by > introducing a private method that just has the return call? Sure, what about thi

RE: RFR: 8222276: (zipfs) Refactoring and cleanups to prepare for JDK-8213031

2019-05-14 Thread Langer, Christoph
Hi Lance, > Thank you for taking this on. Thank you for reviewing! Unfortunately you didn't look at my latest edition... (http://cr.openjdk.java.net/~clanger/webrevs/876.2/) In v2 I took back the modification of the hierarchy of classes IndexNode and Entry so it should be a bit easier to r

Re: RFR(s): 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-14 Thread Florian Weimer
* David Lloyd: > Pipe communication isn't very costly AFAIK. The added complexity > would be waiting for either a thing to appear on the pipe indicating > success or else a waitpid/waitid+WNOHANG for exit code 127. But > writing a thing to the pipe won't help determine if the subsequent > exec s

Re: RFR(s): 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-14 Thread David Lloyd
Pipe communication isn't very costly AFAIK. The added complexity would be waiting for either a thing to appear on the pipe indicating success or else a waitpid/waitid+WNOHANG for exit code 127. But writing a thing to the pipe won't help determine if the subsequent exec succeeded, which is really

RE: [13] RFR: JDK-8206879: Currency decimal marker incorrect for Peru

2019-05-14 Thread Deepak Kejriwal
Thanks Ramanand and Naoto for review. Please find updated version of webrev:- http://cr.openjdk.java.net/~dkejriwal/8206879/webrev.01/ Regards, Deepak -Original Message- From: Ramanand Patil Sent: Monday, May 13, 2019 12:40 PM To: Naoto Sato ; Deepak Kejriwal ; i18n-...@openjdk.java.ne

Re: RFR : 8221696: MappedByteBuffer.force method to specify range

2019-05-14 Thread Andrew Dinn
On 14/05/2019 10:57, Andrew Haley wrote: > On 5/13/19 5:14 PM, Andrew Dinn wrote: >> Thank you for looking at the patch. >> . . . >>firstly, that ps-1 clears the original bit and sets all lower bits; > > I think your core argument fails at this point. You have *already* > moved to considering

Re: RFR : 8221696: MappedByteBuffer.force method to specify range

2019-05-14 Thread Andrew Haley
On 5/13/19 5:14 PM, Andrew Dinn wrote: > Thank you for looking at the patch. > > On 28/04/2019 18:09, Andrew Haley wrote: >> On 4/25/19 5:34 PM, Andrew Dinn wrote: >>>long map_base = (address & ~(ps - 1)); >> >> This looks like a hard way to write >> >>long map_base = address & -ps; > > M

Re: RFR(s): 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-14 Thread Thomas Stüfe
Neat idea! But this incurs costs for this pipe communication on every spawn. What do the others think? Cheers, Thomas On Tue, May 14, 2019 at 10:22 AM Florian Weimer wrote: > * Thomas Stüfe: > > > This is impossible to fix completely - see Martin's comment about the > > impossibility to forese

RE: RFR(S): 8223597: jdk/nio/zipfs/ZipFSTester.java RuntimeException: CHECK_FAILED! (getAttribute.crc failed 6af4413c vs 0 ...)

2019-05-14 Thread Langer, Christoph
Thanks, Lance. From: Lance Andersen Sent: Montag, 13. Mai 2019 19:00 To: Langer, Christoph Cc: core-libs-dev ; nio-dev Subject: Re: RFR(S): 8223597: jdk/nio/zipfs/ZipFSTester.java RuntimeException: CHECK_FAILED! (getAttribute.crc failed 6af4413c vs 0 ...) Hi Christoph, The change seems rea

Re: RFR(s): 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-14 Thread Florian Weimer
* Thomas Stüfe: > This is impossible to fix completely - see Martin's comment about the > impossibility to foresee whether an exec() will succeed without actually > exec()ing. But at least we can test the execute permissions on the > jspawnhelper. Which this fix does. This fixes Ubuntu 16.4 (Now,

RE: RFR(S): 8223597: jdk/nio/zipfs/ZipFSTester.java RuntimeException: CHECK_FAILED! (getAttribute.crc failed 6af4413c vs 0 ...)

2019-05-14 Thread Langer, Christoph
Hi Claes, thanks for reviewing. Yes, seems csize will always be > 0 if the entry got deflated. I don’t know the details of the zip algorithm, but DeflaterOutputStream::finish() always causes 2 bytes to be written when the buffer for the file/channel has 0 bytes. So I think the fix really is co

Re: RFR : 8221696: MappedByteBuffer.force method to specify range

2019-05-14 Thread Andrew Dinn
Hi Mikael, Thanks for looking at this. On 13/05/2019 17:41, Mikael Vidstedt wrote: Would it be worth putting the logic in an aptly named (private) method? Something like: ... private long alignDown(long address, long alignment) { return address & ; } ... long mapBase = alignDown(address,

Re: RFR : 8221696: MappedByteBuffer.force method to specify range

2019-05-14 Thread Andrew Dinn
On 26/04/2019 15:46, Alan Bateman wrote: On 25/04/2019 17:34, Andrew Dinn wrote: Also, here is a new webrev including the updated implementations for mappingAddress/Offset/Length as described below JIRA:   https://bugs.openjdk.java.net/browse/JDK-8221696 webrev: http://cr.openjdk.java.net/~adin