Re: RFR 8044047: Missing null pointer checks for streams

2014-07-15 Thread Paul Sandoz
Ping.. http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-July/027508.html Paul. On Jul 2, 2014, at 2:01 PM, Paul Sandoz wrote: > Hi, > > Please review this fix for some missing null-checks in stream code: > > http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8044047-null-checks/webrev/

Re: RFR 8044047: Missing null pointer checks for streams

2014-07-15 Thread Daniel Fuchs
On 7/15/14 9:35 AM, Paul Sandoz wrote: Ping.. http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-July/027508.html Paul. Hi Paul, This is not my area of expertise - but I haven't seen anything wrong. However - I applied your patch and ran the tests locally on my machine, and I saw

Re: RFR 8044047: Missing null pointer checks for streams

2014-07-15 Thread Paul Sandoz
On Jul 15, 2014, at 12:38 PM, Daniel Fuchs wrote: > On 7/15/14 9:35 AM, Paul Sandoz wrote: >> Ping.. >> >> http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-July/027508.html >> >> Paul. > > Hi Paul, > > This is not my area of expertise - but I haven't seen anything wrong. > Thanks.

Re: [9] RFR (M): 8050052: Small cleanups in java.lang.invoke code

2014-07-15 Thread Vladimir Ivanov
Another update: http://cr.openjdk.java.net/~vlivanov/8050052//webrev.01/ Tentative diff: http://cr.openjdk.java.net/~vlivanov/8050052/webrev.diff.01-02/ Best regards, Vladimir Ivanov On 7/14/14 5:40 PM, Vladimir Ivanov wrote: Paul, thanks! Small update: http://cr.openjdk.java.net/~vlivanov/80

Re: [9] RFR (M): 8050053: Improve caching of different invokers

2014-07-15 Thread Vladimir Ivanov
Update: http://cr.openjdk.java.net/~vlivanov/8050053/webrev.01 Diff: http://cr.openjdk.java.net/~vlivanov/8050053/webrev.diff.00-01/ Got rid of varargs & spread invokers. Best regards, Vladimir Ivanov On 7/11/14 8:35 PM, Vladimir Ivanov wrote: http://cr.openjdk.java.net/~vlivanov/8050053/webre

Re: RFR JDK-8048175: Remove redundant use of reflection on core classes from JNDI

2014-07-15 Thread Mark Sheppard
Hi Pavel, the changes look ok I've run some relevant jck tests (naming management rmi) and the CORBA regression suite for the changeset, with no perceptible issues regards Mark On 14/07/2014 12:43, Pavel Rappo wrote: Hi everyone, Could you please review my change for JDK-8048175? ht

Re: RFR JDK-8048175: Remove redundant use of reflection on core classes from JNDI

2014-07-15 Thread Vincent Ryan
The LDAP changes look fine to me. Thanks. On 15 Jul 2014, at 15:07, Mark Sheppard wrote: > Hi Pavel, > the changes look ok I've run some relevant jck tests (naming > management rmi) and the CORBA regression suite > for the changeset, with no perceptible issues > > regards > Mark > > On

Re: RFR JDK-8048175: Remove redundant use of reflection on core classes from JNDI

2014-07-15 Thread Pavel Rappo
Thanks! -Pavel On 15 Jul 2014, at 15:41, Vincent Ryan wrote: > The LDAP changes look fine to me. > Thanks. > > On 15 Jul 2014, at 15:07, Mark Sheppard wrote: > >> Hi Pavel, >> the changes look ok I've run some relevant jck tests (naming >> management rmi) and the CORBA regression suit

Re: RFR: 8049892: Replace uses of 'new Integer()' with appropriate alternative across core classes

2014-07-15 Thread Paul Sandoz
On Jul 14, 2014, at 6:58 PM, Phil Race wrote: > I haven't seen any one with jdk9 reviewer status comment on this > And although it looks OK to me I'm not responsible for any of the areas > it covers, so it does need an official nod .. > Thanks Phil, this patch looks OK to me too. Paul.

RFR: 7130985: Four helper classes missing in Sun JDK

2014-07-15 Thread Rob McKenna
Hi folks, Looking for a review for the following: bug: https://bugs.openjdk.java.net/browse/JDK-7130985 webrev: http://cr.openjdk.java.net/~robm/7130985/webrev.01/ As per the bug report: The IDLtoJava spec clearly specifies the need of the helper class (not the Holder) for these files. But the

8050044: (process) Increase reaper thread native stack size by a factor of 2

2014-07-15 Thread Rob McKenna
Hi folks, A very simple change suggested by Martin a while back in another review. I'm just getting around to it now: bug: https://bugs.openjdk.java.net/browse/JDK-8050044 webrev: http://cr.openjdk.java.net/~robm/8050044/webrev.01/ Martins comments: http://mail.openjdk.java.net/pipermail/cor

Re: 8050044: (process) Increase reaper thread native stack size by a factor of 2

2014-07-15 Thread Martin Buchholz
Looks good to me! On Tue, Jul 15, 2014 at 10:46 AM, Rob McKenna wrote: > Hi folks, > > A very simple change suggested by Martin a while back in another review. > I'm just getting around to it now: > > bug: https://bugs.openjdk.java.net/browse/JDK-8050044 > webrev: http://cr.openjdk.java.net/~ro

Re: 8050044: (process) Increase reaper thread native stack size by a factor of 2

2014-07-15 Thread roger riggs
Hi Rob, Is there any evidence that needs more space? Is there any way to tell how much of the existing 32k is being used? The reaper has very limited processing to do and there is one thread for every process spawned. Roger On 7/15/2014 1:46 PM, Rob McKenna wrote: Hi folks, A very simpl

Re: RFR: 8050044: (process) Increase reaper thread native stack size by a factor of 2

2014-07-15 Thread Rob McKenna
And as in that review you have managed to intercept me before I made a small correction. In this case to the subject. I'm beginning to suspect you're a highly sophisticated bot. -Rob On 15/07/14 18:48, Martin Buchholz wrote: Looks good to me! On Tue, Jul 15, 2014 at 10:46 AM, Rob McKenn

Re: 8050044: (process) Increase reaper thread native stack size by a factor of 2

2014-07-15 Thread roger riggs
Hi Martin, Wait a minute, can you provide more motivation, observation. etc. it really does not seem necessary and just because everything is bigger is pretty weak. Thanks, Roger On 7/15/2014 1:48 PM, Martin Buchholz wrote: Looks good to me! On Tue, Jul 15, 2014 at 10:46 AM, Rob McKenna

Re: RFR: 8050044: (process) Increase reaper thread native stack size by a factor of 2

2014-07-15 Thread Rob McKenna
I have not made any empirical measurements. I was following up on Martins suggestion. I'll try to put some time aside for testing later this week though. -Rob On 15/07/14 18:51, roger riggs wrote: Hi Rob, Is there any evidence that needs more space? Is there any way to tell how much of

Re: 8050044: (process) Increase reaper thread native stack size by a factor of 2

2014-07-15 Thread Martin Buchholz
The fear is that stack sizes and alignments have grown over time, and thread stacks are acquiring things like guard pages, and those pages may (incorrectly) end up getting included in the stack size. I'm particularly afraid of the hotspot guard page code. It may be worthwhile seeing how low you c

Re: RFR: 7130985: Four helper classes missing in Sun JDK

2014-07-15 Thread Alan Bateman
On 15/07/2014 18:27, Rob McKenna wrote: Hi folks, Looking for a review for the following: bug: https://bugs.openjdk.java.net/browse/JDK-7130985 webrev: http://cr.openjdk.java.net/~robm/7130985/webrev.01/ As per the bug report: The IDLtoJava spec clearly specifies the need of the helper class

Re: [9] RFR (M): 8050052: Small cleanups in java.lang.invoke code

2014-07-15 Thread John Rose
On Jul 11, 2014, at 10:56 AM, Remi Forax wrote: > > On 07/11/2014 06:18 PM, Vladimir Ivanov wrote: >> http://cr.openjdk.java.net/~vlivanov/8050052/webrev.00 >> https://bugs.openjdk.java.net/browse/JDK-8050052 > > I've found myself writing the very same code as > MethodHandleStatics.uncaughExce

Review request for 8050804: (jdeps) Recommend supported API to replace use of JDK internal API

2014-07-15 Thread Mandy Chung
jdeps -jdkinternals flags use of JDK internal APIs. We have created a wiki page to keep track of the JDK internal APIs and its replacement: https://wiki.openjdk.java.net/display/JDK8/Java+Dependency+Analysis+Tool While this page will be updated when we identify any new ones worthnoting, it'd s

JDK 9 RFR of JDK-8030942: Explicitly state floating-point summation requirements on non-finite inputs

2014-07-15 Thread Joe Darcy
Hello, Please review my changes to address: JDK-8030942: Explicitly state floating-point summation requirements on non-finite inputs http://cr.openjdk.java.net/~darcy/8030942.0/ Patch below. Thanks, -Joe --- old/src/share/classes/java/util/DoubleSummaryStatistics.java 2014-07-15 1

RFR(XS): 8050825: Support running regression tests using jtreg_tests+TESTDIRS from top level

2014-07-15 Thread Mikael Vidstedt
Please review the below change which adds support for running jtreg tests from the top level test/ directory using the 'make TESTDIRS= jtreg_tests' syntax. The TESTDIRS syntax is already used in files like hotspot/test/Makefile and jdk/test/Makefile and allows for selecting which jtreg tests

Re: RFR(XS): 8050825: Support running regression tests using jtreg_tests+TESTDIRS from top level

2014-07-15 Thread Mikael Vidstedt
I suppose a webrev helps: http://cr.openjdk.java.net/~mikael/webrevs/8050825/webrev.00/webrev/ Sorry 'bout that. Cheers, Mikael On 2014-07-15 19:48, Mikael Vidstedt wrote: Please review the below change which adds support for running jtreg tests from the top level test/ directory using the

Re: RFR(XS): 8050825: Support running regression tests using jtreg_tests+TESTDIRS from top level

2014-07-15 Thread Mike Duigou
This looks like a nice improvement and provides a good way to execute specific sub-sets that are smaller than the TEST.groups definitions. I'd like to hook it up to the top level make as an alternative to the current recipe. make test TEST="jdk_core" Perhaps adjust the top level make test targ

Re: RFR(XS): 8050825: Support running regression tests using jtreg_tests+TESTDIRS from top level

2014-07-15 Thread David Holmes
Looks okay to me. To be clear, the format of the "path" is not flexible but must have the form ..//test/... David On 16/07/2014 12:51 PM, Mikael Vidstedt wrote: I suppose a webrev helps: http://cr.openjdk.java.net/~mikael/webrevs/8050825/webrev.00/webrev/ Sorry 'bout that. Cheers, Mikael

Re: RFR(XS): 8050825: Support running regression tests using jtreg_tests+TESTDIRS from top level

2014-07-15 Thread Mikael Vidstedt
On 2014-07-15 20:04, Mike Duigou wrote: This looks like a nice improvement and provides a good way to execute specific sub-sets that are smaller than the TEST.groups definitions. I'd like to hook it up to the top level make as an alternative to the current recipe. make test TEST="jdk_core"

Re: RFR(XS): 8050825: Support running regression tests using jtreg_tests+TESTDIRS from top level

2014-07-15 Thread Mikael Vidstedt
Correct, the path needs to be on that format! Thanks for the review! Thanks, Mikael On 2014-07-15 20:15, David Holmes wrote: Looks okay to me. To be clear, the format of the "path" is not flexible but must have the form ..//test/... David On 16/07/2014 12:51 PM, Mikael Vidstedt wrote:

Re: RFR(XS): 8050825: Support running regression tests using jtreg_tests+TESTDIRS from top level

2014-07-15 Thread Mike Duigou
I will probably try to fix that eventually. I had some shell code which re-resolved a path relative to another path. In particular, for top level make execution the "../" portion would be incorrect. Anyway, this seems good enough for now. We can make it more flexible later. Mike On Jul 15 20

Re: RFR(XS): 8050825: Support running regression tests using jtreg_tests+TESTDIRS from top level

2014-07-15 Thread Alan Bateman
On 16/07/2014 03:48, Mikael Vidstedt wrote: Please review the below change which adds support for running jtreg tests from the top level test/ directory using the 'make TESTDIRS= jtreg_tests' syntax. The TESTDIRS syntax is already used in files like hotspot/test/Makefile and jdk/test/Makefile