Re: (Round 2) RFR(s): 8150460: (linux|bsd|aix)_close.c: file descriptor table may become large or may not work at all

Hi Roger, Dmitry, May I ask you both to have a last look at this change before I commit? It took a while for this to go through our internal tests, hence the delay. New version: http://cr.openjdk.java.net/~stuefe/webrevs/8150460-linux_close-fdTable/webrev.03/webrev/ Delta to last version: http://

Re: (Round 2) RFR(s): 8150460: (linux|bsd|aix)_close.c: file descriptor table may become large or may not work at all

Thomas, Looks good for me! -Dmitry On 2016-04-13 12:12, Thomas Stüfe wrote: > Hi Roger, Dmitry, > > May I ask you both to have a last look at this change before I commit? > It took a while for this to go through our internal tests, hence the delay. > > New > version: > http://cr.openjdk.java

Re: (Round 2) RFR(s): 8150460: (linux|bsd|aix)_close.c: file descriptor table may become large or may not work at all

Thanks Dmitry! On Wed, Apr 13, 2016 at 12:00 PM, Dmitry Samersoff < dmitry.samers...@oracle.com> wrote: > Thomas, > > Looks good for me! > > -Dmitry > > > On 2016-04-13 12:12, Thomas Stüfe wrote: > > Hi Roger, Dmitry, > > > > May I ask you both to have a last look at this change before I commit?

Re: JDK 9 RFR of JDK-4851642: Add fused mac to Java math library

Joe, Here are my comments. 1) Probably in lines StrictMath:1171, StrictMath:1220, Math:1487, Math:1593 you meant {@code -0.0 * +0.0} {@code -0.0f * +0.0f} 2) Lines Math:1508-1525 could be simpler: double result = a * b + c; return Double.isNaN(result) && Double.isFinite(a) && Doub

Re: RFR 8151705 VarHandle.AccessMode enum names should conform to code style

Hi John, I like the approach to hybrid names. I’ll follow up with another issue. How about we go with: AccessMode.NAME_get; etc.? And for those that think best-practices are rules (style guide IMO are often best practices sprinkled with subjective reasoning) i can add an @apiNote explainin

Re: RFR: 8153293 - Stream API: Preserve SORTED and DISTINCT characteristics for boxed() and asLongStream() operations

> On 12 Apr 2016, at 18:37, Tagir F. Valeev wrote: > > Hello! > > Thank you, Stefan and Paul for review. Here's updated webrev: > > http://cr.openjdk.java.net/~tvaleev/webrev/8153293/r2/ > Looks good. I will push for you this week. Paul.

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

I had planned on using StackWalker to generate the location information for every logging event. It seems that this change would thus cause the creation of a new StackTraceElement for every logger event. That seems wasteful. Log4j is currently in the process of trying to reduce the number of obj

RFR:JDK-8154050:java.time.format.DateTimeFormatter can't parse localized zone-offset

HI all, Bug Id - https://bugs.openjdk.java.net/browse/JDK-8154050 Issue - java.time.format.DateTimeFormatter can't parse localized zone-offset Solution - Corrected the mistake in calculating parse end position and removed an unnecessary nu

RFR(S): 8150824: Exceptions when omitting trailing arguments in cleanup

Dear all, please review this change. Bug: https://bugs.openjdk.java.net/browse/JDK-8150824 Webrev: http://cr.openjdk.java.net/~mhaupt/8150824/webrev.00/ The actual bug was fixed with the push for 8150829; this change merely contributes tests for the issue. Thanks, Michael --

RFR 9: 8086278 : java/lang/ProcessHandle/TreeTest.java failed - ProcessReaper StackOverflowException

Please review an increase in the default stack size of the Process reaper thread to 48K from 32k. Some tests with various VM arguments are failing intermittently. The failures are not reproducible. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-reaper-stack-8086278/ Issue: https://bugs

RFR [9] 8137058: Clear out all non-Critical APIs from sun.reflect and move to jdk.unsupported

This review is for the second significant part of the changes for JEP 260 [1]. When created, the jdk.unsupported module [2] initially contained the sun.misc package. This issue is will move all the non-Critical APIs out of sun.reflect, leaving only the critical ones, at which point sun.reflect can

Re: RFR:JDK-8154050:java.time.format.DateTimeFormatter can't parse localized zone-offset

Hi Nadeesh, The bugfix looks fine. The TODO comment on the "GMT" raises the question (as a separate issue) about implementing the TODO or removing the TODO comment. I'm not sure where the localized string for "GMT" would come from but it might be a useful improvement unless it was judged to a

Re: RFR [9] 8137058: Clear out all non-Critical APIs from sun.reflect and move to jdk.unsupported

On 13/04/2016 16:43, Chris Hegarty wrote: This review is for the second significant part of the changes for JEP 260 [1]. When created, the jdk.unsupported module [2] initially contained the sun.misc package. This issue is will move all the non-Critical APIs out of sun.reflect, leaving only the cr

Re: Review request 8153895 (proxy) redundant read edges to superinterfaces of proxy interfaces

> On Apr 12, 2016, at 11:22 PM, Peter Levart wrote: > > No, not about security. Mainly about binary compatibility. For example: > > - library A v1 defines an interface I with some methods > - library B creates a dynamic proxy implementing I. It depends on library A > and libraries defining typ

Re: RFR 9: 8086278 : java/lang/ProcessHandle/TreeTest.java failed - ProcessReaper StackOverflowException

On 13/04/16 16:42, Roger Riggs wrote: Please review an increase in the default stack size of the Process reaper thread to 48K from 32k. Some tests with various VM arguments are failing intermittently. The failures are not reproducible. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-reaper

Re: RFR [9] 8137058: Clear out all non-Critical APIs from sun.reflect and move to jdk.unsupported

On 13/04/16 16:59, Alan Bateman wrote: On 13/04/2016 16:43, Chris Hegarty wrote: This review is for the second significant part of the changes for JEP 260 [1]. When created, the jdk.unsupported module [2] initially contained the sun.misc package. This issue is will move all the non-Critical APIs

Re: RFR:JDK-8154050:java.time.format.DateTimeFormatter can't parse localized zone-offset

> On Apr 13, 2016, at 11:56 AM, Roger Riggs wrote: > > Hi Nadeesh, > > The bugfix looks fine. > > The TODO comment on the "GMT" raises the question (as a separate issue) > about implementing the TODO or removing the TODO comment. > > I'm not sure where the localized string for "GMT" would com

Re: RFR:JDK-8154050:java.time.format.DateTimeFormatter can't parse localized zone-offset

Hi Lance, Literal strings are interned by the compiler but it would make it clearer that it is the same string everywhere. Though I find it easier to read when the value is inline without having to do the indirection to a constant. And its really not going to change; "GMT" is too deeply embedd

Re: RFR 9: 8086278 : java/lang/ProcessHandle/TreeTest.java failed - ProcessReaper StackOverflowException

Hi Chris, On 4/13/2016 12:02 PM, Chris Hegarty wrote: On 13/04/16 16:42, Roger Riggs wrote: Please review an increase in the default stack size of the Process reaper thread to 48K from 32k. Some tests with various VM arguments are failing intermittently. The failures are not reproducible. Webr

Re: RFR 9: 8086278 : java/lang/ProcessHandle/TreeTest.java failed - ProcessReaper StackOverflowException

On 13/04/16 18:11, Roger Riggs wrote: Hi Chris, On 4/13/2016 12:02 PM, Chris Hegarty wrote: On 13/04/16 16:42, Roger Riggs wrote: Please review an increase in the default stack size of the Process reaper thread to 48K from 32k. Some tests with various VM arguments are failing intermittently.

Re: RFR [9] 8137058: Clear out all non-Critical APIs from sun.reflect and move to jdk.unsupported

> On Apr 13, 2016, at 8:43 AM, Chris Hegarty wrote: > > This review is for the second significant part of the changes for JEP > 260 [1]. When created, the jdk.unsupported module [2] initially > contained the sun.misc package. This issue is will move all the > non-Critical APIs out of sun.reflect

Re: RFR [9] 8137058: Clear out all non-Critical APIs from sun.reflect and move to jdk.unsupported

Mandy, On 13/04/16 18:15, Mandy Chung wrote: On Apr 13, 2016, at 8:43 AM, Chris Hegarty wrote: This review is for the second significant part of the changes for JEP 260 [1]. When created, the jdk.unsupported module [2] initially contained the sun.misc package. This issue is will move all the

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

It is good to know Log4J is planning to use StackWalker. Thanks for the feedback. I will reconsider. One thing to mention is the patch went in jdk9/hs-rt that will show up in jdk9/dev some time that changes the implementation to create StackTraceElement to get filename and line number. The ob

Re: RFR: 8148568: LoggerFinder.getLogger and LoggerFinder.getLocalizedLogger should take a Module argument instead of a Class.

Hi Mandy, On 08/04/16 22:26, Mandy Chung wrote: On Apr 8, 2016, at 7:52 AM, Daniel Fuchs wrote: Hi, Please find below a patch that slightly modifies the JEP 264 initial implementation to take advantage of the module system. The change modifies the LoggerFinder abstract service to take the

Re: RFR: 8148568: LoggerFinder.getLogger and LoggerFinder.getLocalizedLogger should take a Module argument instead of a Class.

> On Apr 13, 2016, at 10:28 AM, Daniel Fuchs wrote: > > Hi Mandy, > > On 08/04/16 22:26, Mandy Chung wrote: >> >>> On Apr 8, 2016, at 7:52 AM, Daniel Fuchs wrote: >>> >>> Hi, >>> >>> Please find below a patch that slightly modifies >>> the JEP 264 initial implementation to take advantage >>

Re: RFR [9] 8137058: Clear out all non-Critical APIs from sun.reflect and move to jdk.unsupported

> On Apr 13, 2016, at 10:28 AM, Chris Hegarty wrote: > > Mandy, > > On 13/04/16 18:15, Mandy Chung wrote: >> >>> On Apr 13, 2016, at 8:43 AM, Chris Hegarty wrote: >>> >>> This review is for the second significant part of the changes for JEP >>> 260 [1]. When created, the jdk.unsupported modu

Re: Expecting Integer.valueOf(String) to accept Literal format ...

Thanks Joe, for the bug report! I am a bit unsure about gauging interest in this by looking at the votes on the bug report because I expected this to just work, especially since the API was already there! To me, not having this behavior in the platform is a violation of POLA [1]. I was thinking h

Re: RFR [9] 8137058: Clear out all non-Critical APIs from sun.reflect and move to jdk.unsupported

Since getCallerClass will be removed in 10, @Deprecated should also have its condemned=true Cheers, Paul On Wed, Apr 13, 2016 at 12:43 PM, Mandy Chung wrote: > > > On Apr 13, 2016, at 10:28 AM, Chris Hegarty > wrote: > > > > Mandy, > > > > On 13/04/16 18:15, Mandy Chung wrote: > >> > >>> On Ap

Re: Expecting Integer.valueOf(String) to accept Literal format ...

I think the argument for changing Integer.valueOf(String) hinges on the belief that the method is meant to parse JLS integer syntax. If it is, the Javadocs don't speak to that responsibility. If it's an omission from the docs, I would never have guessed. Regarding how it parses today, I wouldn't b

Re: RFR:JDK-8154050:java.time.format.DateTimeFormatter can't parse localized zone-offset

Hi all, Please see the updated the webrev with a minor doc change - *O 1 appendLocalizedOffsetPrefixed(TextStyle.SHORT); - *4 appendLocalizedOffsetPrefixed(TextStyle.FULL); + *O 1 appendLocalizedOffset(TextStyle.SHORT); + *

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

I did a raw test of StackWalker by itself and the performance was much better than using a Throwable to get the location information. However, I haven’t tested how it will be implemented in Log4j. We still support Java 7 (and will for some time) so we have to find a way to support using StackW

Re: RFR:JDK-8154050:java.time.format.DateTimeFormatter can't parse localized zone-offset

Hi Roger, > On Apr 13, 2016, at 1:06 PM, Roger Riggs wrote: > > Hi Lance, > > Literal strings are interned by the compiler but it would make it clearer > that it is the same string > everywhere. Understand, I just thought it would be cleaner > Though I find it easier to read when the value

RFR: JDK-8147460: Clean-up jrtfs implementation

Hi, Please hep review the cleanup changes for jrtfs implementation. issue: https://bugs.openjdk.java.net/browse/JDK-8147460 webrev: http://cr.openjdk.java.net/~sherman/8147460/webrev Simple benchmark [1] has been run both on jimage and the "exploded modules" directory, the numbers suggest we al

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

If you record all stack frames, I can believe Throwable is faster because of a recent optimization JDK-8150778 that has been made in jdk9 to improve the Throwable::getStackTraceElements method. Mandy > On Apr 13, 2016, at 11:49 AM, Ralph Goers wrote: > > I did a raw test of StackWalker by its

Re: JDK 9 RFR of JDK-4851642: Add fused mac to Java math library

Joe / Dmitry, On Apr 12, 2016, at 5:21 PM, joe darcy wrote: > Please review the changes for > >JDK-4851642: Add fused mac to Java math library >http://cr.openjdk.java.net/~darcy/4851642.0/ > > Fused mac (multiply-accumulate) is a ternary floating-point operation which > accepts three

Re: RFR 9: 8086278 : java/lang/ProcessHandle/TreeTest.java failed - ProcessReaper StackOverflowException

If you're actually seeing faiures with 32k, then 48k is too conservative. I suggest 128k. I tried once to increase the limit to 64k, but without a failing test it didn't get any support. The presence of native thread local variables is the biggest reason for blowing the stack; no limit is actual

Re: RFR 9: 8086278 : java/lang/ProcessHandle/TreeTest.java failed - ProcessReaper StackOverflowException

The change itself is fine. On Wed, Apr 13, 2016 at 1:08 PM, Martin Buchholz wrote: > If you're actually seeing faiures with 32k, then 48k is too > conservative. I suggest 128k. > > I tried once to increase the limit to 64k, but without a failing test > it didn't get any support. > > The presence

Re: Expecting Integer.valueOf(String) to accept Literal format ...

Am Wed, 13 Apr 2016 13:16:45 -0500 schrieb Paul Benedict : > I think the argument for changing Integer.valueOf(String) hinges on > the belief that the method is meant to parse JLS integer syntax. If > it is, the Javadocs don't speak to that responsibility. If it's an > omission from the docs, I wo

RFR JDK9 (JAXP) 8152527: Relative rewriteSystem with xml:base at group level failed

Hi, Please review a fix to rewrite* matching issue. When the rewrite* entries are in a group entry, the current match with entries in a group entry needs to be recorded just as when they are outside of a group. JBS: https://bugs.openjdk.java.net/browse/JDK-8152527 webrevs: http://cr.openjdk.

Re: RFR 9: 8086278 : java/lang/ProcessHandle/TreeTest.java failed - ProcessReaper StackOverflowException

HI Martin, I thought if I overshot, I'd get some push back on too big stack sizes. I'll use 128k and see. Thanks, Roger On 4/13/2016 4:09 PM, Martin Buchholz wrote: The change itself is fine. On Wed, Apr 13, 2016 at 1:08 PM, Martin Buchholz wrote: If you're actually seeing faiures with 32

Re: RFR [9] 8137058: Clear out all non-Critical APIs from sun.reflect and move to jdk.unsupported

Hi Chris, FWIW changes looks good to me. cheers /Joel On Wed, 13 Apr 2016 at 17:43 Chris Hegarty wrote: > This review is for the second significant part of the changes for JEP > 260 [1]. When created, the jdk.unsupported module [2] initially > contained the sun.misc package. This issue is will

RFR:JDK-8031085:DateTimeFormatter won't parse dates with custom format "yyyyMMddHHmmssSSS"

HI all, BUG ID : https://bugs.openjdk.java.net/browse/JDK-8031085 Webrev : http://cr.openjdk.java.net/~ntv/8031085/webrev.00/ Issue - Fractional parts of seconds do not participate in the protocol for adjacent value parsing Solution - Changed the FractionPrinterParser to subclass of NumberP

Re: Review request 8153895 (proxy) redundant read edges to superinterfaces of proxy interfaces

On 04/13/2016 06:02 PM, Mandy Chung wrote: On Apr 12, 2016, at 11:22 PM, Peter Levart wrote: No, not about security. Mainly about binary compatibility. For example: - library A v1 defines an interface I with some methods - library B creates a dynamic proxy implementing I. It depends on libra

Re: RFR [9] 8137058: Clear out all non-Critical APIs from sun.reflect and move to jdk.unsupported

Hi, Chris Hegarty wrote: Mandy, On 13/04/16 18:15, Mandy Chung wrote: On Apr 13, 2016, at 8:43 AM, Chris Hegarty wrote: This review is for the second significant part of the changes for JEP 260 [1]. When created, the jdk.unsupported module [2] initially contained the sun.misc package. Th

Re: RFR [9] 8137058: Clear out all non-Critical APIs from sun.reflect and move to jdk.unsupported

> On 13 Apr 2016, at 19:03, Paul Benedict wrote: > > Since getCallerClass will be removed in 10, @Deprecated should also have its > condemned=true `condemned` was renamed to `forRemoval` [1]. getCallerClass, in fact the whole class, will have `forRemoval=true`. -Chris. [1] http://mail.ope

Re: JDK 9 RFR of JDK-4851642: Add fused mac to Java math library

Hi Brian and Dmitry, On 4/13/2016 12:43 PM, Brian Burkhalter wrote: Joe / Dmitry, On Apr 12, 2016, at 5:21 PM, joe darcy > wrote: Please review the changes for JDK-4851642: Add fused mac to Java math library http://cr.openjdk.java.net/~darcy/4851642.0/

Re: RFR JDK9 (JAXP) 8152527: Relative rewriteSystem with xml:base at group level failed

Looks OK Joe > On Apr 13, 2016, at 4:34 PM, huizhe wang wrote: > > Hi, > > Please review a fix to rewrite* matching issue. When the rewrite* entries > are in a group entry, the current match with entries in a group entry needs > to be recorded just as when they are outside of a group. > > JB

Re: JDK 9 RFR of JDK-4851642: Add fused mac to Java math library

Hi Joe, On Apr 13, 2016, at 3:02 PM, Joseph D. Darcy wrote: > On 4/13/2016 12:43 PM, Brian Burkhalter wrote: >> >> A couple of points of curiosity. Firstly, is this not “fused multiply-add” >> rather than “fused multiply-accumulate?” Secondly, why the choice of name >> “fusedMac()” instead of

Re: RFR: JDK-8152690: main thread does not have native thread name

Hi, On 14/04/2016 1:28 AM, Yasumasa Suenaga wrote: Hi David, Thanks for your comment. I exported new JVM function to set native thread name, and JLI uses it in new webrev. First the launcher belongs to another team so core-libs will need to review and approve this (in particular Kumar) - no

RFR(m): 8145468 deprecations for java.lang

Hi all, Please review this first round of deprecation changes for the java.lang package. This changeset includes the following: - a set of APIs being newly deprecated - a set of already-deprecated APIs that are "upgraded" to forRemoval=true - addition of the "since" element to all deprecati

Re: RFR: JDK-8152690: main thread does not have native thread name

Hi, On 2016/04/14 9:34, David Holmes wrote: Hi, On 14/04/2016 1:28 AM, Yasumasa Suenaga wrote: Hi David, Thanks for your comment. I exported new JVM function to set native thread name, and JLI uses it in new webrev. First the launcher belongs to another team so core-libs will need to revie