Re: JDK 9 RFR of JDK-4851777 Add BigDecimal sqrt method

2016-05-20 Thread joe darcy
Hi Brian, On 5/20/2016 1:39 PM, Brian Burkhalter wrote: Hi Joe, I’ve a few comments / questions, mostly picayune. On May 19, 2016, at 11:43 PM, joe darcy > wrote: Please review the addition of BigDecimal.sqrt: JDK-4851777 Add

[9] RFR: Static build of libzip is missing JNI_OnLoad_zip entry point

2016-05-20 Thread Naoto Sato
Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8150179 The proposed fix is located at: http://cr.openjdk.java.net/~naoto/8150179/webrev.00/ The fix is to simply bring the JNI OnLoad entry point back from ZipFile.c which was removed. Naoto

Re: JDK 9 RFR of JDK-8157487: Mark ZoneId.java as intermittently failing

2016-05-20 Thread Naoto Sato
Looks fine to me. Naoto On 5/20/16 2:55 PM, Joseph D. Darcy wrote: Hello, The test sun/net/www/protocol/http/ZoneId.java has been seen to intermittently fail (JDK-8085785) and should be marked accordingly. Please review the patch below which does this. Thanks, -Joe ---

JDK 9 RFR of JDK-8157487: Mark ZoneId.java as intermittently failing

2016-05-20 Thread Joseph D. Darcy
Hello, The test sun/net/www/protocol/http/ZoneId.java has been seen to intermittently fail (JDK-8085785) and should be marked accordingly. Please review the patch below which does this. Thanks, -Joe --- a/test/sun/net/www/protocol/http/ZoneId.javaFri May 20 14:41:41 2016 -0700

Re: [PING] [9] RFR of 5100935: No way to access the 64-bit integer multiplication of 64-bit CPUs efficiently

2016-05-20 Thread Brian Burkhalter
Hello Andrew, Thank you for your comments. I went ahead and integrated the existing code as reviewed but created a new issue https://bugs.openjdk.java.net/browse/JDK-8157485 to track the possible improvement you suggested. Thanks for citing the appropriate reference material as well. Cheers,

Re: RFR (JAXP) 8054632: [since-tag]: javadoc for xml classes has invalid @since tag

2016-05-20 Thread huizhe wang
Thanks Lance! Joe On 5/20/2016 11:07 AM, Lance Andersen wrote: looks ok joe On May 20, 2016, at 1:12 PM, huizhe wang > wrote: Hi, This patch applies similar change[1] made to the DOM sources under java.xml for those under jdk.xml.dom,

Re: JDK 9 RFR of JDK-4851777 Add BigDecimal sqrt method

2016-05-20 Thread Brian Burkhalter
Hi Joe, I’ve a few comments / questions, mostly picayune. On May 19, 2016, at 11:43 PM, joe darcy wrote: > Please review the addition of BigDecimal.sqrt: > > JDK-4851777 Add BigDecimal sqrt method > >http://cr.openjdk.java.net/~darcy/4851777.5/ 2023 * The

Re: [9] RFR: 8152207: Perform array bound checks while getting a length of bytecode instructions

2016-05-20 Thread Artem Smotrakov
Hello, Could someone review this fix, please? http://cr.openjdk.java.net/~asmotrak/8152207/hotspot/webrev.01/ http://cr.openjdk.java.net/~asmotrak/8152207/jdk/webrev.00/ Artem On 05/10/2016 01:49 PM, Artem Smotrakov wrote: Hi Christian, Thank you for review. Here is updated webrev

Re: RFR: 8156484: ZipFile retains too much native memory from caching Inflaters

2016-05-20 Thread Martin Buchholz
Peter, You keep impressing me with your development speed! Looking at ObjectPool ... looking pretty good keepalivetime should be > 0. (also very small keepalive times are a bad idea, but what's "very small"?) I'm thinking one second for ZipFile keep alive time. Thread pools have 60 seconds.

Re: RFR: JDK-8143282: \p{Cn} unassigned code points should be included in \p{C}

2016-05-20 Thread Martin Buchholz
Consider your change reviewed if you fix the duplication here: +(Character.CONTROL == type || Character.CONTROL == type || On Fri, May 20, 2016 at 11:10 AM, Xueming Shen wrote: > it is more like a rfe now :-) Yup, that's a RFE!

Re: RFR: JDK-8143282: \p{Cn} unassigned code points should be included in \p{C}

2016-05-20 Thread Xueming Shen
On 5/20/16 10:13 AM, Martin Buchholz wrote: On Fri, May 20, 2016 at 9:55 AM, Xueming Shen wrote: I expected to see general category Other "C" in Character.java can open a rfe for that if needed. Well, don't we want complete correspondence between Unicode standard,

Re: RFR (JAXP) 8054632: [since-tag]: javadoc for xml classes has invalid @since tag

2016-05-20 Thread Lance Andersen
looks ok joe > On May 20, 2016, at 1:12 PM, huizhe wang wrote: > > Hi, > > This patch applies similar change[1] made to the DOM sources under java.xml > for those under jdk.xml.dom, replacing "@since DOM Level 2" with "@since 1.4, > DOM Level 2", and adding "@since

Re: RFR 8156718: Need tests for IsoFields getFrom for unsupported non-Iso Temporal fields

2016-05-20 Thread Roger Riggs
Hi Bhanu, Looks good. Thanks for including the tests of the values so the tests look complete. Roger On 5/20/2016 5:22 AM, Bhanu Gopularam wrote: Hi Roger, Thanks for the comments. Earlier as part of JDK-8062804 we considered scenarios for support of isofields and non isofields (positive

Re: RFR: JDK-8143282: \p{Cn} unassigned code points should be included in \p{C}

2016-05-20 Thread Martin Buchholz
On Fri, May 20, 2016 at 9:55 AM, Xueming Shen wrote: > On 5/20/16 9:11 AM, Martin Buchholz wrote: >> >> Here is a duplicate check: >> >> 4422 (Character.CONTROL == type || Character.CONTROL >> == type || >> >> I don't see any tests for corresponding

RFR (JAXP) 8054632: [since-tag]: javadoc for xml classes has invalid @since tag

2016-05-20 Thread huizhe wang
Hi, This patch applies similar change[1] made to the DOM sources under java.xml for those under jdk.xml.dom, replacing "@since DOM Level 2" with "@since 1.4, DOM Level 2", and adding "@since 1.4, DOM Level 2" to the ones (under html package) that missed the @since tag. JBS:

Re: RFR: JDK-8143282: \p{Cn} unassigned code points should be included in \p{C}

2016-05-20 Thread Xueming Shen
On 5/20/16 9:11 AM, Martin Buchholz wrote: Here is a duplicate check: 4422 (Character.CONTROL == type || Character.CONTROL == type || I don't see any tests for corresponding p{Cn} It appears we don't have any regex test for the gc in current test cases. Not a surprise

Re: RFR: JDK-8143282: \p{Cn} unassigned code points should be included in \p{C}

2016-05-20 Thread Martin Buchholz
Here is a duplicate check: 4422 (Character.CONTROL == type || Character.CONTROL == type || I don't see any tests for corresponding p{Cn} I expected to see general category Other "C" in Character.java I'd like to see tests that p{C} is the same as p{Other} is the same as

Re: RFR: 8157449: Adjust link-time generated Species classes to match JDK-8148604 usage

2016-05-20 Thread Aleksey Shipilev
On 05/20/2016 05:16 PM, Claes Redestad wrote: > Webrev: http://cr.openjdk.java.net/~redestad/8157449/webrev.00/ Looks good. Thanks, -Aleksey

RFR: JDK-8143282: \p{Cn} unassigned code points should be included in \p{C}

2016-05-20 Thread Xueming Shen
Hi, Please help review the change for 8143282. issue: https://bugs.openjdk.java.net/browse/JDK-8143282 webrev: http://cr.openjdk.java.net/~sherman/8143282 thanks, Sherman

Re: Review Request: 8157391: jdeps left JarFile open

2016-05-20 Thread Mandy Chung
> On May 20, 2016, at 7:24 AM, Mandy Chung wrote: > >> >> On May 19, 2016, at 11:39 PM, Alan Bateman wrote: >> >> >> >> On 20/05/2016 04:55, Mandy Chung wrote: >>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157391/webrev.00/index.html

Re: TreeMap specific override of putIfAbsent/merge

2016-05-20 Thread Paul Sandoz
Hi Tomasz, If there was any rejection it was probably due to time constraints than for any technical reasons. It should be easy to refactor the TreeMap.put implementation into say a private putVal accepting a boolean argument and both put and putIfAbsent call putVal (same pattern as in

Re: Review Request: 8157391: jdeps left JarFile open

2016-05-20 Thread Mandy Chung
> On May 19, 2016, at 11:39 PM, Alan Bateman wrote: > > > > On 20/05/2016 04:55, Mandy Chung wrote: >> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157391/webrev.00/index.html >> >> tools/jdeps/modules/GenModuleInfo.java >> tools/jdeps/modules/TransitiveDeps.java

Re: RFR: 8156484: ZipFile retains too much native memory from caching Inflaters

2016-05-20 Thread Peter Levart
Hi Martin, On 05/20/2016 02:51 AM, Martin Buchholz wrote: On Thu, May 19, 2016 at 7:29 AM, Peter Levart wrote: So Martin, what do you think? I studied your code and we are fundamentally in agreement! ... Except for need for periodic cleanup of the cache... Maybe we

RFR: 8157449: Adjust link-time generated Species classes to match JDK-8148604 usage

2016-05-20 Thread Claes Redestad
Hi, I noticed that the BMH$Species classes generated at link time slightly mismatch the classes generated when concatenating things. Updating this to match reality helps mitigate a small fraction of the startup penalty to enabling the optimal string concatenation strategy: Bug:

TreeMap specific override of putIfAbsent/merge

2016-05-20 Thread Tomasz Kowalczewski
Hi all, TreeMap does not have its own specific overrides of Map methods that were added in java 8. In our specific usage we do what is effective a merge operation: For incoming K and V: 1. get value from map 2. if present - merge with incoming value V 3. if absent - associate K with V in the map

Re: RFR: 8157437 Typos in Stream JavaDoc

2016-05-20 Thread Lance Andersen
+1 > On May 20, 2016, at 7:47 AM, Paul Sandoz wrote: > > Hi, > > I introduced some typos in my haste to push the fix for JDK-8130023 > (API java.util.stream: explicitly specify guaranteed execution of the > pipeline) (I am reminded of the phrase “Go slow and

Re: RFR: 8157437 Typos in Stream JavaDoc

2016-05-20 Thread Alan Bateman
On 20/05/2016 12:47, Paul Sandoz wrote: Hi, I introduced some typos in my haste to push the fix for JDK-8130023 (API java.util.stream: explicitly specify guaranteed execution of the pipeline) (I am reminded of the phrase “Go slow and you’ll get there faster”.) So to slow my self down i did

Re: RFR: 8157437 Typos in Stream JavaDoc

2016-05-20 Thread Aleksey Shipilev
Looks good. Thanks, -Aleksey On 05/20/2016 02:47 PM, Paul Sandoz wrote: > Hi, > > I introduced some typos in my haste to push the fix for JDK-8130023 > (API java.util.stream: explicitly specify guaranteed execution of the > pipeline) (I am reminded of the phrase “Go slow and you’ll get

RFR: 8157437 Typos in Stream JavaDoc

2016-05-20 Thread Paul Sandoz
Hi, I introduced some typos in my haste to push the fix for JDK-8130023 (API java.util.stream: explicitly specify guaranteed execution of the pipeline) (I am reminded of the phrase “Go slow and you’ll get there faster”.) So to slow my self down i did a search for spelling errors in all of the

Re: RFR 8130023 API java.util.stream: explicitly specify guaranteed execution of the pipeline

2016-05-20 Thread Paul Sandoz
> On 19 May 2016, at 12:02, Paul Sandoz wrote: > > >> On 17 May 2016, at 23:15, Claes Redestad wrote: >> >> Hi, >> >> the first block in Stream.java bothers me: >> >> + * A stream implementation is permitted significant latitude in >>

Re: JDK 9 RFR of JDK-8157211: Mark tools/launcher/FXLauncherTest.java as intermittently failing

2016-05-20 Thread Paul Sandoz
> On 20 May 2016, at 07:48, Hamlin Li wrote: > > Would you please help to review the code change? > +1 Paul. > Thank you > -Hamlin > > On 2016/5/18 12:52, Hamlin Li wrote: >> tools/launcher/FXLauncherTest.java >> This test is known to fail intermittently (JDK-8130392

RE: RFR 8156718: Need tests for IsoFields getFrom for unsupported non-Iso Temporal fields

2016-05-20 Thread Bhanu Gopularam
Hi Roger, Thanks for the comments. Earlier as part of JDK-8062804 we considered scenarios for support of isofields and non isofields (positive case to check isofield works and negative case to check UnsupportedTemporalTypeException is thrown only. Here is the updated webrev, I have added data

Re: Review request 8153042: jdeps should continue to report JDK internal APIs that are removed/renamed in JDK 9

2016-05-20 Thread Daniel Fuchs
On 5/20/16 6:21 AM, Mandy Chung wrote: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153042/webrev.00/index.html Several JDK internal APIs have been removed JDK 9. It’d be helpful to continue to flag those internal APIs if they are used by existing libraries. This fix is an interim

RE: RFR(M): 8156915: introduce MethodHandle factory for array length

2016-05-20 Thread Uwe Schindler
Hi, I just noticed that there is the @since missing in javadocs: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/fd39cefc5c8f#l3.13 arrayConstructor has @since 9, but arrayLength() not. Uwe - Uwe Schindler uschind...@apache.org ASF Member, Apache Lucene PMC / Committer Bremen, Germany

JDK 9 RFR of JDK-4851777 Add BigDecimal sqrt method

2016-05-20 Thread joe darcy
Hello, Please review the addition of BigDecimal.sqrt: JDK-4851777 Add BigDecimal sqrt method http://cr.openjdk.java.net/~darcy/4851777.5/ Thanks, -Joe

Re: Review Request: 8157391: jdeps left JarFile open

2016-05-20 Thread Alan Bateman
On 20/05/2016 04:55, Mandy Chung wrote: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157391/webrev.00/index.html tools/jdeps/modules/GenModuleInfo.java tools/jdeps/modules/TransitiveDeps.java tools/jdeps/modules/InverseDeps.java These tests still failed on windows after JDK-8152502. Now

Re: RFR: 8156484: ZipFile retains too much native memory from caching Inflaters

2016-05-20 Thread Alan Bateman
On 20/05/2016 07:25, Peter Levart wrote: Hi Martin, On 05/20/2016 12:04 AM, Martin Buchholz wrote: Hi Peter, I would make e.g. the change to Resource an independent change. I agree. It's just that if we measure the impact of ZipFile changes to class loading, for example, we should

Re: RFR: 8156484: ZipFile retains too much native memory from caching Inflaters

2016-05-20 Thread Peter Levart
Hi Martin, On 05/20/2016 12:04 AM, Martin Buchholz wrote: Hi Peter, I would make e.g. the change to Resource an independent change. I agree. It's just that if we measure the impact of ZipFile changes to class loading, for example, we should measure them with this change included so that