Re: Code review request

2013-02-25 Thread Remi Forax
On 02/25/2013 04:07 AM, David Holmes wrote: On 23/02/2013 8:51 PM, Remi Forax wrote: On 02/21/2013 08:17 PM, Brian Goetz wrote: At http://cr.openjdk.java.net/~briangoetz/jdk-8008670/webrev/ I've posted a webrev for about half the classes in java.util.stream. None of these are public

Re: RFR: 8007806: Need a Throwables performance counter

2013-02-25 Thread Nils Loodin
On 02/25/2013 12:03 AM, David Holmes wrote: I'm getting very confused regarding the different threads on this. But this webrev: http://cr.openjdk.java.net/~nloodin/exception-tracing/webrev.01/ shows the intrusive event tracing that I'm sure we said we would grudgingly accept for 7u but not for

Re: RFR: 8007806: Need a Throwables performance counter

2013-02-25 Thread Nils Loodin
On 02/24/2013 11:18 PM, David Holmes wrote: We've not-so-slightly hijacked Nils' thread here - apologies for that. David, Peter! Yes you did :) However, feel free to make it up to me by: 1. Suggest a good name for the counter 2. Plz to say yes to adding that counter 3. Profit! Regards, Nils

Re: hg: jdk8/tl/jdk: 8008167: IdentityHashMap.[keySet|values|entrySet].toArray speed-up

2013-02-25 Thread Alan Bateman
On 25/02/2013 04:06, Mike Duigou wrote: I have created an issue for fixing the introduced regression and created a regression test that would have caught the error (fix a bug, write a test...). 8008785: IdentityHashMap.values().toArray(V[]) broken by JDK-8008167

Re: RFR: 8007806: Need a Throwables performance counter

2013-02-25 Thread Alan Bateman
On 24/02/2013 20:57, David Holmes wrote: Does jstat access these values directly or only via the synchronized methods? If the latter then the value can't be torn that way. The sync method will store the value in 2 32-bit registers, and the variable load in jstat will take two instructions,

Re: RFR: 8007806: Need a Throwables performance counter

2013-02-25 Thread Alan Bateman
On 25/02/2013 09:23, Nils Loodin wrote: David et al: No! That showed the intrusive event tracing that we will not implement ever. It also has nothing to do with this code review, which is only to implement a performance counter. Just to confirm, this is the webrev that we should be

Re: RFR: 8007806: Need a Throwables performance counter

2013-02-25 Thread Nils Loodin
On 02/25/2013 11:15 AM, Alan Bateman wrote: On 25/02/2013 09:23, Nils Loodin wrote: David et al: No! That showed the intrusive event tracing that we will not implement ever. It also has nothing to do with this code review, which is only to implement a performance counter. Just to confirm,

Re: RFR: 8007806: Need a Throwables performance counter

2013-02-25 Thread Peter Levart
On 02/25/2013 10:29 AM, Nils Loodin wrote: On 02/24/2013 11:18 PM, David Holmes wrote: We've not-so-slightly hijacked Nils' thread here - apologies for that. David, Peter! Yes you did :) However, feel free to make it up to me by: 1. Suggest a good name for the counter As Jason Mehrens

Re: RFR: 8006039 : test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C

2013-02-25 Thread Kumar Srinivasan
Hi Brent, Ok I tested on Windows with JA installed and your patch works, also tested on Solaris. On other nit, Copyright on I18NJarTest.java needs updating. Looks good, go for it. Thanks Kumar On 2/21/13 7:33 PM, Kumar Srinivasan wrote: A nit - LC_ALL + LC_ALL + \n + + LC_ALL= +

Re: hg: jdk8/tl/jdk: 8008167: IdentityHashMap.[keySet|values|entrySet].toArray speed-up

2013-02-25 Thread Mike Duigou
On Feb 25 2013, at 01:51 , Alan Bateman wrote: On 25/02/2013 04:06, Mike Duigou wrote: I have created an issue for fixing the introduced regression and created a regression test that would have caught the error (fix a bug, write a test...). 8008785: IdentityHashMap.values().toArray(V[])

Re: RFR: 8007806: Need a Throwables performance counter

2013-02-25 Thread Nils Loodin
On 02/25/2013 02:30 PM, Peter Levart wrote: On 02/25/2013 10:29 AM, Nils Loodin wrote: On 02/24/2013 11:18 PM, David Holmes wrote: We've not-so-slightly hijacked Nils' thread here - apologies for that. David, Peter! Yes you did :) However, feel free to make it up to me by: 1. Suggest a good

Re: RFR: 8007806: Need a Throwables performance counter

2013-02-25 Thread Peter Levart
On 02/25/2013 05:30 PM, Nils Loodin wrote: On 02/25/2013 02:30 PM, Peter Levart wrote: On 02/25/2013 10:29 AM, Nils Loodin wrote: On 02/24/2013 11:18 PM, David Holmes wrote: We've not-so-slightly hijacked Nils' thread here - apologies for that. David, Peter! Yes you did :) However, feel

hg: jdk8/tl/jdk: 8008808: Allowed dependencies added by JDK-8008481 no longer required

2013-02-25 Thread alan . bateman
Changeset: 155095c245b4 Author:alanb Date: 2013-02-25 17:17 + URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/155095c245b4 8008808: Allowed dependencies added by JDK-8008481 no longer required Reviewed-by: tbell, chegar ! make/tools/src/build/tools/deps/refs.allowed

Re: Code review request

2013-02-25 Thread Paul Sandoz
Hi Remi, Thanks for the feedback i have addressed some of this, mostly related to inner classes, in following change set to the lambda repo: http://hg.openjdk.java.net/lambda/lambda/jdk/rev/3e50294c68ea We can update the webrev next week. On Feb 23, 2013, at 11:51 AM, Remi Forax

Re: hg: jdk8/tl/jdk: 8008167: IdentityHashMap.[keySet|values|entrySet].toArray speed-up

2013-02-25 Thread Mike Duigou
On Feb 25 2013, at 01:51 , Alan Bateman wrote: On 25/02/2013 04:06, Mike Duigou wrote: I have created an issue for fixing the introduced regression and created a regression test that would have caught the error (fix a bug, write a test...). 8008785: IdentityHashMap.values().toArray(V[])

Re: hg: jdk8/tl/jdk: 8008167: IdentityHashMap.[keySet|values|entrySet].toArray speed-up

2013-02-25 Thread Alan Bateman
On 25/02/2013 15:55, Mike Duigou wrote: : The webrev suggests you've replaced Map/Collisions.java and assume that is not the intention. Otherwise the new test looks okay, an alternative would have been to expand MOAT. Webrev doesn't properly recognize 'hg copy' and sees it as a rename. I

Re: hg: jdk8/tl/jdk: 8008167: IdentityHashMap.[keySet|values|entrySet].toArray speed-up

2013-02-25 Thread Mike Duigou
On Feb 25 2013, at 10:13 , Alan Bateman wrote: On 25/02/2013 15:55, Mike Duigou wrote: : The webrev suggests you've replaced Map/Collisions.java and assume that is not the intention. Otherwise the new test looks okay, an alternative would have been to expand MOAT. Webrev doesn't

Re: RFR: 8006039 : test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C

2013-02-25 Thread Brent Christian
On 2/22/13 10:22 AM, Naoto Sato wrote: check not only C but also other locales that do not use UTF-8 encoding. I would like to make the test more robust (per 8008761), though my focus at the moment is to avoid a test failure on our automated test systems, which all use C. I've made a note

Re: RFR: 8006039 : test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C

2013-02-25 Thread Naoto Sato
Looks good. Thank you for fixing this. Naoto On 2/25/13 10:53 AM, Brent Christian wrote: On 2/22/13 10:22 AM, Naoto Sato wrote: check not only C but also other locales that do not use UTF-8 encoding. I would like to make the test more robust (per 8008761), though my focus at the moment is

Re: RFR: 8006039 : test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C

2013-02-25 Thread Kumar Srinivasan
couple of minor nits I did not see in rev 01. Copyright, -if(C.equals(LC_ALL) || C.equals(LANG)) { +if (C.equals(LC_ALL) || C.equals(LANG)) { Kumar Looks good. Thank you for fixing this. Naoto On 2/25/13 10:53 AM, Brent Christian wrote: On 2/22/13 10:22 AM, Naoto Sato

Re: RFR: 8006039 : test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C

2013-02-25 Thread Brent Christian
Thanks for the review, guys. Think I've got it all in: http://cr.openjdk.java.net/~bchristi/8006039/webrev.02/ For the Copyright, do I just need to change the year(s) to 2012, 2013,? Also, thanks a lot for running the Windows testing, Kumar. Can someone push this for me? I can send a patch

Re: FAILS: test/sun/management/jdp/JdpTest.sh: test: argument expected

2013-02-25 Thread Kelly O'Hair
Bingo. ;^) -kto On Feb 12, 2013, at 2:34 PM, alejandro murillo wrote: Dmitry, you have to use -testset core to run this test via JPRT. It's not run on regular jobs. Alejandro On 2/12/2013 3:18 PM, Dmitry Samersoff wrote: Kelly, Do you have an idea why I didn't see this failure

hg: jdk8/tl/jdk: 8006748: getISO3Country() returns wrong value

2013-02-25 Thread yong . huang
Changeset: ce6a2fcb4c9b Author:yhuang Date: 2013-02-16 21:22 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ce6a2fcb4c9b 8006748: getISO3Country() returns wrong value Reviewed-by: naoto ! src/share/classes/java/util/LocaleISOData.java

Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so

2013-02-25 Thread Kelly O'Hair
On Feb 23, 2013, at 12:12 PM, Alan Bateman wrote: On 23/02/2013 18:06, Martin Buchholz wrote: I am actually encountering this in openjdk7 with the old build system. I can repro the problem in openjdk8 with the old build system, but not the new one. I don't know if you consider this a bug

hg: jdk8/tl/jdk: 2 new changesets

2013-02-25 Thread stefan . karlsson
Changeset: c6d77b2b4478 Author:stefank Date: 2013-01-22 13:53 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c6d77b2b4478 8006506: Add test for redefining methods in backtraces to java/lang/instrument tests Reviewed-by: sspitsyn, coleenp +

Re: RFR: 8005545: Add System property to identify ARCH specific details such as ARM hard-float binaries

2013-02-25 Thread Vladimir Danushevsky
Thanks, webrev updated http://cr.openjdk.java.net/~vladidan/8005545/webrev.01/ On Feb 22, 2013, at 5:57 AM, Alan Bateman wrote: On 21/02/2013 22:02, Vladimir Danushevsky wrote: : Webrev: http://cr.openjdk.java.net/~vladidan/8005545/webrev.00/

Re: FAILS: test/sun/management/jdp/JdpTest.sh: test: argument expected

2013-02-25 Thread Kelly O'Hair
+if [ ! -f ${_testclasses} ] needs to be +if [ ! -d ${_testclasses} ] -kto On Feb 12, 2013, at 9:38 AM, Chris Hegarty wrote: Dmitry, This test is now failing on several platforms, on jdk8 and 7u-dev --- result: Passed. Compilation successful #section:shell

RFR: 8005545: Add System property to identify ARCH specific details such as ARM hard-float binaries

2013-02-25 Thread Vladimir Danushevsky
Please review changes for https://jbs.oracle.com/bugs/browse/JDK-8005545 Add System property to identify ARCH specific details such as ARM hard-float binaries which adds an optional sun.os.abi Java system property indicating an ABI type being used by the JVM: CCC Reguest:

Re: hg: jdk8/tl/jdk: 8008167: IdentityHashMap.[keySet|values|entrySet].toArray speed-up

2013-02-25 Thread Mike Duigou
I made one additional change to the regression test so that the keys and values aren't in the same range. It didn't produce any failures but I think it is a good addition to catch future bugs. Mike diff --git a/test/java/util/Map/ToArray.java b/test/java/util/Map/ToArray.java ---

Re: [PATCH] Sunbug 7131192: Optimize BigInteger.doubleValue(), floatValue()

2013-02-25 Thread Brian Burkhalter
Hi Dima, On Feb 25, 2013, at 1:14 AM, Dmitry Nadezhin wrote: I looked at your fix of sunbug=6358355 Fix Float.parseFloat to round correctly and preserve monotonicity. […] 1) The line in floatValue() method fValue = (float) doubleValue(); fValue can become Float.POSITIVE_INFINITY. It

Re: [PATCH] Sunbug 7131192: Optimize BigInteger.doubleValue(), floatValue()

2013-02-25 Thread Louis Wasserman
In general, the only thing I did was to mirror the double parsing logic as closely as I could manage. If that logic is going to change, then so too should the patch. There's really no reason to preserve the specific code I added in that patch. On Mon, Feb 25, 2013 at 12:50 PM, Brian Burkhalter

Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so

2013-02-25 Thread Martin Buchholz
Kelly, Thanks. I think we agree. I think we have consensus that my change is an improvement and should be committed. Can I haz approval? I'm not brave enough to try to change the default for all mapfiles, although I would support Kelly or anyone else who tries. Martin On Mon, Feb 25, 2013 at

Re: [PATCH] Sunbug 7131192: Optimize BigInteger.doubleValue(), floatValue()

2013-02-25 Thread Brian Burkhalter
Understood. Thanks, Brian On Feb 25, 2013, at 12:53 PM, Louis Wasserman wrote: In general, the only thing I did was to mirror the double parsing logic as closely as I could manage. If that logic is going to change, then so too should the patch. There's really no reason to preserve the

Re: Code review request

2013-02-25 Thread David Holmes
On 26/02/2013 3:31 AM, Paul Sandoz wrote: Hi Remi, Thanks for the feedback i have addressed some of this, mostly related to inner classes, in following change set to the lambda repo: http://hg.openjdk.java.net/lambda/lambda/jdk/rev/3e50294c68ea I see a lot of private things that are now

8006409: ThreadLocalRandom should dropping padding fields from its serialized form

2013-02-25 Thread Chris Hegarty
[ Subject: 8006409: ThreadLocalRandom should drop padding fields from its serialized form (was: Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread) ] I think we are in agreement that the pad fields can safely be removed from the serial form. No problem. To

Re: 8006409: ThreadLocalRandom should dropping padding fields from its serialized form

2013-02-25 Thread Doug Lea
On 02/25/13 16:46, Chris Hegarty wrote: [ Subject: 8006409: ThreadLocalRandom should drop padding fields from its serialized form (was: Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread) ] I think we are in agreement that the pad fields can safely be

Re: 8006409: ThreadLocalRandom should dropping padding fields from its serialized form

2013-02-25 Thread Alan Bateman
On 25/02/2013 21:46, Chris Hegarty wrote: [ Subject: 8006409: ThreadLocalRandom should drop padding fields from its serialized form (was: Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread) ] I think we are in agreement that the pad fields can safely be

Re: RFR: 8006039 : test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C

2013-02-25 Thread Kumar Srinivasan
I can take care of it. Kumar Thanks for the review, guys. Think I've got it all in: http://cr.openjdk.java.net/~bchristi/8006039/webrev.02/ For the Copyright, do I just need to change the year(s) to 2012, 2013,? Also, thanks a lot for running the Windows testing, Kumar. Can someone push

Re: hg: jdk8/tl/jdk: 8008167: IdentityHashMap.[keySet|values|entrySet].toArray speed-up

2013-02-25 Thread Alan Bateman
On 25/02/2013 20:24, Mike Duigou wrote: I made one additional change to the regression test so that the keys and values aren't in the same range. It didn't produce any failures but I think it is a good addition to catch future bugs. Mike Looks okay. -Alan

Re: 8006409: ThreadLocalRandom should dropping padding fields from its serialized form

2013-02-25 Thread Aleksey Shipilev
On 02/26/2013 01:46 AM, Chris Hegarty wrote: [ Subject: 8006409: ThreadLocalRandom should drop padding fields from its serialized form (was: Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread) ] I think we are in agreement that the pad fields can safely

Re: 8006409: ThreadLocalRandom should dropping padding fields from its serialized form

2013-02-25 Thread Martin Buchholz
Me too!

Re: 8006409: ThreadLocalRandom should dropping padding fields from its serialized form

2013-02-25 Thread Martin Buchholz
Perhaps slightly better is this variant, which adds doc for the @serialField's. /** * @serialField rnd long * seed for random computations * @serialField initialized boolean * always true */ private static final ObjectStreamField[]

hg: jdk8/tl/jdk: 8006039: test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C

2013-02-25 Thread kumar . x . srinivasan
Changeset: 58f829566fe3 Author:bchristi Date: 2013-02-25 14:19 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/58f829566fe3 8006039: test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C Summary: Avoid automated test failure by just exiting when in 'C' locale

hg: jdk8/tl: 8008914: Add nashorn to the tl build

2013-02-25 Thread jonathan . gibbons
Changeset: 5b0b6ef58dbf Author:jjg Date: 2013-02-25 15:08 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/rev/5b0b6ef58dbf 8008914: Add nashorn to the tl build Reviewed-by: mr, tbell, jjh Contributed-by: erik.joels...@oracle.com, james.las...@oracle.com ! Makefile !

hg: jdk8/tl/jdk: 8008914: Add nashorn to the tl build

2013-02-25 Thread jonathan . gibbons
Changeset: 4cf4403c9bf2 Author:jjg Date: 2013-02-25 15:08 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4cf4403c9bf2 8008914: Add nashorn to the tl build Reviewed-by: mr, tbell, jjh Contributed-by: erik.joels...@oracle.com, james.las...@oracle.com ! make/launchers/Makefile

Re: RFR: 8007806: Need a Throwables performance counter

2013-02-25 Thread Mandy Chung
On 2/25/2013 8:30 AM, Nils Loodin wrote: I changed it to sun.throwables.numThrowables in http://cr.openjdk.java.net/~nloodin/8007806/webrev.01/ Is this better? However, bear in mind that it's not exactly specified where this is going to be incremented from yet. Nothing in this change states

Re: 8006409: ThreadLocalRandom should dropping padding fields from its serialized form

2013-02-25 Thread Chris Hegarty
Thanks Martin, good suggestion. -Chris On 25 Feb 2013, at 22:25, Martin Buchholz marti...@google.com wrote: Perhaps slightly better is this variant, which adds doc for the @serialField's. /** * @serialField rnd long * seed for random computations *

Re: [PATCH] Sunbug 7131192: Optimize BigInteger.doubleValue(), floatValue()

2013-02-25 Thread Dmitry Nadezhin
Hi Brian, The reported benchmark results are impressive: http://bugs.sun.com/view_bug.do?bug_id=7032154 Where can we find the 7032154 patch to review it ? Dima On Tue, Feb 26, 2013 at 12:50 AM, Brian Burkhalter brian.burkhal...@oracle.com wrote: Hi Dima, On Feb 25, 2013, at 1:14 AM,

Re: [PATCH] Sunbug 7131192: Optimize BigInteger.doubleValue(), floatValue()

2013-02-25 Thread Brian Burkhalter
Hi Dima, On Feb 25, 2013, at 6:58 PM, Dmitry Nadezhin wrote: The reported benchmark results are impressive: http://bugs.sun.com/view_bug.do?bug_id=7032154 Indeed. Where can we find the 7032154 patch to review it ? The patch appears to be against the state of the repository about 23 months