Re: Review request for 7034570 - java.lang.Runtime.exec(String[] cmd, String[] env) can not work properly if SystemRoot not inherited

2011-04-18 Thread Michael McMahon
David Schlosnagle wrote: On Sun, Apr 17, 2011 at 1:27 PM, Ulf Zibis ulf.zi...@gmx.de wrote: Am 16.04.2011 16:45, schrieb David Schlosnagle: One minor nit in ProcessEnvironment.java 336 private static void addToEnv(StringBuilder sb, String name, String val) { 337

Re: Fwd: [concurrency-interest] ConcurrentHashMap footprint and contention improvements

2011-04-18 Thread Doug Lea
On 04/13/11 11:58, Chris Hegarty wrote: This change does appear worthwhile. We have a narrowing window of opportunity for JDK7. I spoke to Alan Bateman about this and we feel that if we could finalize the JDK7 changes by next Monday (that is, get them in for JDK7 b140), this would give us a

Re: Fwd: [concurrency-interest] ConcurrentHashMap footprint and contention improvements

2011-04-18 Thread Chris Hegarty
On 04/18/11 11:18 AM, Doug Lea wrote: On 04/13/11 11:58, Chris Hegarty wrote: This change does appear worthwhile. We have a narrowing window of opportunity for JDK7. I spoke to Alan Bateman about this and we feel that if we could finalize the JDK7 changes by next Monday (that is, get them in

Re: Review request for 7034570 - java.lang.Runtime.exec(String[] cmd, String[] env) can not work properly if SystemRoot not inherited

2011-04-18 Thread Ulf Zibis
Hi David, much thanks for your detailed effort. -Ulf Am 17.04.2011 22:50, schrieb David Schlosnagle: On Sun, Apr 17, 2011 at 1:27 PM, Ulf Zibisulf.zi...@gmx.de wrote: Am 16.04.2011 16:45, schrieb David Schlosnagle: One minor nit in ProcessEnvironment.java 336 private static void

Re: Suspected regression: fix for 6735255 causes delay in GC of ZipFile InputStreams, increase in heap demand

2011-04-18 Thread Neil Richards
On Thu, 2011-04-14 at 14:48 -0700, Xueming Shen wrote: Opinion? anything I overlooked/missed? Hi Sherman, Thanks once more for all your help and advice on this - I'm in favour of almost all of what you suggest. :-) I think it's worthwhile trying to clear 'streams' entries from a finalize method

Re: Review request for 7034570 - java.lang.Runtime.exec(String[] cmd, String[] env) can not work properly if SystemRoot not inherited

2011-04-18 Thread Ulf Zibis
Am 16.04.2011 12:48, schrieb Alan Bateman: Michael McMahon wrote: I have incorporated much of Ulf's approach into this version. I agree with Alan about the spec. We could find other variables in the future that need to be set, but can have alternative values other than the default. I think

Re: Review request for 7034570 - java.lang.Runtime.exec(String[] cmd, String[] env) can not work properly if SystemRoot not inherited

2011-04-18 Thread Michael McMahon
Ulf Zibis wrote: Am 16.04.2011 12:48, schrieb Alan Bateman: Michael McMahon wrote: I have incorporated much of Ulf's approach into this version. I agree with Alan about the spec. We could find other variables in the future that need to be set, but can have alternative values other than the

Re: Review request for 7034570 - java.lang.Runtime.exec(String[] cmd, String[] env) can not work properly if SystemRoot not inherited

2011-04-18 Thread Ulf Zibis
Am 15.04.2011 15:43, schrieb Michael McMahon: I have incorporated much of Ulf's approach into this version. I agree with Alan about the spec. We could find other variables in the future that need to be set, but can have alternative values other than the default. I think this approach is the

hg: jdk7/tl/jdk: 7036559: ConcurrentHashMap footprint and contention improvements

2011-04-18 Thread chris . hegarty
Changeset: 005c0c85b0de Author:dl Date: 2011-04-18 16:10 +0100 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/005c0c85b0de 7036559: ConcurrentHashMap footprint and contention improvements Reviewed-by: chegar ! src/share/classes/java/util/concurrent/ConcurrentHashMap.java

Re: Review needed for 7037085 : Add hashCode() to Timestamp to address Findbugs warning

2011-04-18 Thread Lance Andersen - Oracle
Thank you Eamonn. I fixed the typo in the class description as well so they both reference hashCode Regards Lance On Apr 18, 2011, at 6:34 AM, Eamonn McManus wrote: OK, in that case you can add me (emcmanus) as a reviewer. I'd just suggest fixing the case of {@code hashcode} in the doc

Re: Suspected regression: fix for 6735255 causes delay in GC of ZipFile InputStreams, increase in heap demand

2011-04-18 Thread Xueming Shen
On 4/18/2011 5:33 AM, Neil Richards wrote: On Thu, 2011-04-14 at 14:48 -0700, Xueming Shen wrote: Opinion? anything I overlooked/missed? Hi Sherman, Thanks once more for all your help and advice on this - I'm in favour of almost all of what you suggest. :-) I think it's worthwhile trying to

Re: Suspected regression: fix for 6735255 causes delay in GC of ZipFile InputStreams, increase in heap demand

2011-04-18 Thread Xueming Shen
Hi Neil, All tests passed. I'm starting to push your last patch. I generated the webrev at http://cr.openjdk.java.net/~sherman/7031076/webrev/ It should be exactly the same as your last patch. Thanks, Sherman On 4/18/2011 9:49 AM, Xueming Shen wrote: On 4/18/2011 5:33 AM, Neil Richards

Re: Review request for 7034570 - java.lang.Runtime.exec(String[] cmd, String[] env) can not work properly if SystemRoot not inherited

2011-04-18 Thread Ulf Zibis
Am 15.04.2011 15:43, schrieb Michael McMahon: I have incorporated much of Ulf's approach into this version. I agree with Alan about the spec. We could find other variables in the future that need to be set, but can have alternative values other than the default. I think this approach is the

hg: jdk7/tl/jdk: 7031076: Retained ZipFile InputStreams increase heap demand

2011-04-18 Thread xueming . shen
Changeset: 98688c4be64b Author:sherman Date: 2011-04-18 10:51 -0700 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/98688c4be64b 7031076: Retained ZipFile InputStreams increase heap demand Summary: Allow unreferenced ZipFile InputStreams to be finalized, GC'd Reviewed-by: sherman,

Re: Suspected regression: fix for 6735255 causes delay in GC of ZipFile InputStreams, increase in heap demand

2011-04-18 Thread Neil Richards
On 18 April 2011 17:49, Xueming Shen xueming.s...@oracle.com wrote: If you are now explicitly synchronize on streams everywhere, I don't think we even need a copy at close(). I thought that the copy is needed so that the Map being iterated over (in close()) is not able to be modified by the

Review Request : 7035160 : Disable broken test cases

2011-04-18 Thread Mike Duigou
Hello All; I would like a quick review for a small patch which disables certain test cases for test/java/lang/reflect/Generics/Probe.java This test is failing due to recent changes from CR # 6312706. The root cause is another issue, CR # 6476261, which currently has no fix available. The

Re: Review request for 7034570 - java.lang.Runtime.exec(String[] cmd, String[] env) can not work properly if SystemRoot not inherited

2011-04-18 Thread David Schlosnagle
On Mon, Apr 18, 2011 at 1:40 PM, Ulf Zibis ulf.zi...@gmx.de wrote: Am 15.04.2011 15:43, schrieb Michael McMahon: I have incorporated much of Ulf's approach into this version. I agree with Alan about the spec. We could find other variables in the future that need to be set, but can have

Re: Review Request : 7035160 : Disable broken test cases

2011-04-18 Thread Alan Bateman
Mike Duigou wrote: Hello All; I would like a quick review for a small patch which disables certain test cases for test/java/lang/reflect/Generics/Probe.java This test is failing due to recent changes from CR # 6312706. The root cause is another issue, CR # 6476261, which currently has no fix

hg: jdk7/tl/jdk: 2 new changesets

2011-04-18 Thread mike . duigou
Changeset: 60a457a944c4 Author:mduigou Date: 2011-04-18 11:31 -0700 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/60a457a944c4 7035160: Disable broken test cases for test/java/lang/reflect/Generics/Probe.java Reviewed-by: alanb ! test/java/lang/reflect/Generics/Probe.java

Re: Review request for 7034570 - java.lang.Runtime.exec(String[] cmd, String[] env) can not work properly if SystemRoot not inherited

2011-04-18 Thread Ulf Zibis
Am 18.04.2011 15:43, schrieb Michael McMahon: Ulf Zibis wrote: I think, the comment in lines 295..297 more belongs to the code (lines 312..315 + 318..320 ) rather than to the constant SYSTEMROOT. Why defining it as class member, as it is only locally referred? It makes no difference to the

Re: Review request for 7034570 - java.lang.Runtime.exec(String[] cmd, String[] env) can not work properly if SystemRoot not inherited

2011-04-18 Thread Ulf Zibis
Am 15.04.2011 15:43, schrieb Michael McMahon: I have incorporated much of Ulf's approach into this version. I agree with Alan about the spec. We could find other variables in the future that need to be set, but can have alternative values other than the default. I think this approach is the

Re: Review request for 7034570 - java.lang.Runtime.exec(String[] cmd, String[] env) can not work properly if SystemRoot not inherited

2011-04-18 Thread Ulf Zibis
Oops, I was irritated by the internal int variables, but yes, they reside on stack, so no worry about thread-safety. -Ulf Am 18.04.2011 20:16, schrieb David Schlosnagle: On Mon, Apr 18, 2011 at 1:40 PM, Ulf Zibisulf.zi...@gmx.de wrote: Am 15.04.2011 15:43, schrieb Michael McMahon: I have

Review (Updated) : 4884238 : Constants for Standard Charsets

2011-04-18 Thread Mike Duigou
Hello All; I have updated the webrev for the standard Charset constants based upon feedback received from the earlier reviews. The constants class is now named StandardCharset rather than Charsets in mimicry of the class naming used by NIO.2 (JSR 203). According to the conventions used by JSR

hg: jdk7/tl/langtools: 6758050: javadoc handles nested generic types incorrectly

2011-04-18 Thread bhavesh . patel
Changeset: bbd053476ec3 Author:bpatel Date: 2011-04-18 15:39 -0700 URL: http://hg.openjdk.java.net/jdk7/tl/langtools/rev/bbd053476ec3 6758050: javadoc handles nested generic types incorrectly Reviewed-by: jjg !

Review : 7030579 : Correct confusing documentation in ListIterator.add()

2011-04-18 Thread Mike Duigou
Hello All; A very simple patch to remove two instances of the unnecessary and potentially confusing word next from the documentation of ListIterator.add() http://cr.openjdk.java.net/~mduigou/7030579/0/webrev/ Mike

Re: Review : 7030579 : Correct confusing documentation in ListIterator.add()

2011-04-18 Thread David Holmes
Looks good to me Mike! Reads much clearer now. David Mike Duigou said the following on 04/19/11 10:16: Hello All; A very simple patch to remove two instances of the unnecessary and potentially confusing word next from the documentation of ListIterator.add()