Re: RFR: 8013395 StringBuffer.toString performance regression impacting embedded benchmarks

2013-05-13 Thread David Holmes
Thanks for all the feedback and discussion. I've rewritten the patch to use Peter's suggestion of caching the char[] instead of the actual String: http://cr.openjdk.java.net/~dholmes/8013395/webrev.v3/ I too am concerned that any form of caching that avoids the array-copy (which is the crux

Re: RFR: 8013395 StringBuffer.toString performance regression impacting embedded benchmarks

2013-05-13 Thread Remi Forax
Hi David, On 05/13/2013 09:12 AM, David Holmes wrote: Thanks for all the feedback and discussion. I've rewritten the patch to use Peter's suggestion of caching the char[] instead of the actual String: http://cr.openjdk.java.net/~dholmes/8013395/webrev.v3/ I too am concerned that any form of

Re: RFR: 8013395 StringBuffer.toString performance regression impacting embedded benchmarks

2013-05-13 Thread David Holmes
On 13/05/2013 6:07 PM, Remi Forax wrote: Hi David, On 05/13/2013 09:12 AM, David Holmes wrote: Thanks for all the feedback and discussion. I've rewritten the patch to use Peter's suggestion of caching the char[] instead of the actual String:

Re: RFR: 8013395 StringBuffer.toString performance regression impacting embedded benchmarks

2013-05-13 Thread Peter Levart
On 05/13/2013 10:07 AM, Remi Forax wrote: Hi David, On 05/13/2013 09:12 AM, David Holmes wrote: Thanks for all the feedback and discussion. I've rewritten the patch to use Peter's suggestion of caching the char[] instead of the actual String:

Re: RFR: 8013395 StringBuffer.toString performance regression impacting embedded benchmarks

2013-05-13 Thread David Holmes
On 13/05/2013 6:39 PM, Peter Levart wrote: On 05/13/2013 10:07 AM, Remi Forax wrote: Hi David, On 05/13/2013 09:12 AM, David Holmes wrote: Thanks for all the feedback and discussion. I've rewritten the patch to use Peter's suggestion of caching the char[] instead of the actual String:

Re: RFR: 8013395 StringBuffer.toString performance regression impacting embedded benchmarks

2013-05-13 Thread Florian Weimer
On 05/10/2013 08:03 AM, David Holmes wrote: Short version: Cache the value returned by toString and use it to copy-construct a new String on subsequent calls to toString(). Clear the cache on any mutating operation. webrev: http://cr.openjdk.java.net/~dholmes/8013395/webrev.v2/ Shouldn't you

Re: Review Request: BigInteger patch for efficient multiplication and division (#4837946)

2013-05-13 Thread Alan Eliasen
On 05/10/2013 03:19 PM, Brian Burkhalter wrote: On May 9, 2013, at 5:28 PM, Brian Burkhalter wrote: I just went ahead and filed 8014319 and 8014320 for phases 3 and 4, respectively. They will show up onhttp://bugs.sun.com/bugdatabase after some unspecified delay, probably about a day.

Re: RFC 7038914: VM could throw uncaught OOME in ReferenceHandler thread

2013-05-13 Thread Thomas Schatzl
Hi all, On Fri, 2013-05-10 at 12:52 +0200, Peter Levart wrote: Hi David, Thomas, I think I understand you now, David. You want to minimize the possibility of vacuously passing the test. So here's my attempt at that: public class OOMEInReferenceHandler { static Object[] fillHeap()

Re: Please review changes for JDK-8012975: Remove rhino from jdk8

2013-05-13 Thread A. Sundararajan
Incorporated changes as suggested. Uploaded webrev for historical purpose: http://cr.openjdk.java.net/~sundar/8012975/webrev.02/ PS. I am going ahead with push.. Thanks -Sundar On Friday 10 May 2013 06:17 PM, A. Sundararajan wrote: Okay, thanks. com.sun.script.util is not supported API (no

Re: RFR [7021870] GzipInputStream closes underlying stream during reading

2013-05-13 Thread Ivan Gerasimov
Hello Sherman! Thank you for the review! While I agree with you that current implementation of ZipInputStream.available() is not what it's supposed to be, I doubt it is the only cause of the issue. Please consider the following example. Zip archive (z.zip) consists of two entries: f1 and f2.

Re: Please review changes for JDK-8012975: Remove rhino from jdk8

2013-05-13 Thread Alan Bateman
On 13/05/2013 13:14, A. Sundararajan wrote: Incorporated changes as suggested. Uploaded webrev for historical purpose: http://cr.openjdk.java.net/~sundar/8012975/webrev.02/ PS. I am going ahead with push.. Thanks for fixing up refs.allowed, that one looks fine okay. Did you decide to keep

hg: jdk8/tl/jdk: 8005535: SSLSessionImpl should have protected finalize()

2013-05-13 Thread xuelei . fan
Changeset: 76998d11a643 Author:xuelei Date: 2013-05-13 05:41 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/76998d11a643 8005535: SSLSessionImpl should have protected finalize() Reviewed-by: weijun, wetmore ! src/share/classes/sun/security/ssl/SSLSessionImpl.java

hg: jdk8/tl/jdk: 8005598: (reopened) Need to clone array of input/output parameters

2013-05-13 Thread xuelei . fan
Changeset: 46db0e633240 Author:xuelei Date: 2013-05-13 06:05 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/46db0e633240 8005598: (reopened) Need to clone array of input/output parameters Reviewed-by: weijun ! src/share/classes/com/sun/jndi/dns/DnsContext.java !

test/java/util/Collection/ListDefaults.java updates

2013-05-13 Thread Chris Hegarty
Akil, Mike, I've recently been diagnosing failures with ListDefaults.java, when working on a separate issue. I found it difficult to determine which collection type was having problems, in some failure cases. The diffs below are what I had to implement in my local repo to help identify the

Re: RFR: JDK-8011136 - FileInputStream.available and skip inconsistencies

2013-05-13 Thread Alan Bateman
On 10/05/2013 22:20, Dan Xu wrote: Hi, The FileInputStream.available() method can return a negative value when the file position is beyond the endof afile. This is an unspecified behaviour that has the potential to break applications that expect available to return a value of 0 or greater.

Re: RFR [7021870] GzipInputStream closes underlying stream during reading

2013-05-13 Thread Ivan Gerasimov
A small correction to the recipe for failure. The failure is reproduced when a SINGLE byte is appended to the gzip archive. In the scenario below the echo command appends TWO bytes. These two bytes are interpreted as a wrong gzip header and get successfully ignored. On 13.05.2013 16:23, Ivan

Code review request, JDK-8010815, some constructors issues in com.sun.jndi.toolkit

2013-05-13 Thread Xuelei Fan
Hi, There is some constructors issues about mutable objects in com.sun.jndi.toolkit. webrev: http://cr.openjdk.java.net/~xuelei/8010815/webrev.00/ Thanks, Xuelei

RFR: JDK-8013900: More warnings compiling jaxp.

2013-05-13 Thread Daniel Fuchs
This is a fix for JDK-8013900: More warnings compiling jaxp. http://cr.openjdk.java.net/~dfuchs/JDK-8013900/webrev.00/ Although the title might suggest a trivial fix, it goes a bit beyond a simple warning fix because the root cause of those warnings is that some classes redefine equals without

Re: Review request: JDK-7147084 (process) appA hangs when read output stream of appB which starts appC that runs forever

2013-05-13 Thread Alexey Utkin
Thanks, Martin! You are right. The approach private static String JAVA_EXE = System.getProperty(java.home) + File.separator + bin + File.separator + java; private static String[] getCommandArray(String processName) { String[] cmdArray = { JAVA_EXE,

Re: Code review request, JDK-8010815, some constructors issues in com.sun.jndi.toolkit

2013-05-13 Thread Wang Weijun
I'll read them tomorrow. One question: are all of them designed to be call-by-value instead of shareable? Thanks Max On May 13, 2013, at 10:05 PM, Xuelei Fan xuelei@oracle.com wrote: Hi, There is some constructors issues about mutable objects in com.sun.jndi.toolkit. webrev:

Re: Please review changes for JDK-8012975: Remove rhino from jdk8

2013-05-13 Thread A. Sundararajan
I agree that it is better to remove com.sun.script.util package as well. It is not used elsewhere and was never declared as supported API. I've removed the same and re-generated webrev. http://cr.openjdk.java.net/~sundar/8012975/webrev.03/ I ran NEWBUILD=false as well as NEWBUILD=true to make

Re: Please review changes for JDK-8012975: Remove rhino from jdk8

2013-05-13 Thread Alan Bateman
On 13/05/2013 16:40, A. Sundararajan wrote: I agree that it is better to remove com.sun.script.util package as well. It is not used elsewhere and was never declared as supported API. I've removed the same and re-generated webrev. http://cr.openjdk.java.net/~sundar/8012975/webrev.03/ I ran

RFR 8005698 : Handle Frequent HashMap Collisions with Balanced Trees

2013-05-13 Thread Brent Christian
Hi, Please review my changes for 8005698 : Handle Frequent HashMap Collisions with Balanced Trees. For HashMap and LinkedHashMap, we would like to duplicate the technique (and some of the code) that the latest ConcurrentHashMap[1] uses for handling hash collisions: use of balanced trees to

Re: test/java/util/Collection/ListDefaults.java updates

2013-05-13 Thread Akhil Arora
Looks good to me. I have had to add similar print statements when debugging, but I deleted them before committing, to keep the default test output clean. Minor - it would be good to have a little more consistency... some tests print the class name always, some only on failure. Also would be

Re: test/java/util/Collection/ListDefaults.java updates

2013-05-13 Thread Chris Hegarty
Thanks Akhil, let me flesh it out a little more and send a formal webrev. -Chris. On 05/13/2013 05:58 PM, Akhil Arora wrote: Looks good to me. I have had to add similar print statements when debugging, but I deleted them before committing, to keep the default test output clean. Minor - it

Re: test/java/util/Collection/ListDefaults.java updates

2013-05-13 Thread Mike Duigou
These changes look OK. I've been adding a description in my test providers which provides the test method with two params, a printable description and the test data. This has worked reasonably well and has the advantage that it can provide more information than just the class name. Mike On

Re: Review Request: BigInteger patch for efficient multiplication and division (#4837946)

2013-05-13 Thread Brian Burkhalter
On May 13, 2013, at 4:26 AM, Alan Eliasen wrote: Another minor note is that you might want to correct reminder to remainder in MutableBigInteger.java . I can revise the patches if you want. Thanks, Alan, I will take care of it. Brian

Re: Review Request: BigInteger patch for efficient multiplication and division (#4837946)

2013-05-13 Thread Brian Burkhalter
On May 11, 2013, at 8:35 PM, Alan Eliasen wrote: On 05/09/2013 03:02 PM, Brian Burkhalter wrote: First you have: /** * The threshold value for using 3-way Toom-Cook multiplication. * If the number of ints in both mag arrays are greater than this number, * then Toom-Cook

Re: RFR: 8013395 StringBuffer.toString performance regression impacting embedded benchmarks

2013-05-13 Thread Alan Bateman
On 13/05/2013 08:12, David Holmes wrote: Thanks for all the feedback and discussion. I've rewritten the patch to use Peter's suggestion of caching the char[] instead of the actual String: http://cr.openjdk.java.net/~dholmes/8013395/webrev.v3/ I too am concerned that any form of caching that

Re: RFR 8012326: Deadlock occurs when Charset.availableCharsets() is called by several threads at the same time

2013-05-13 Thread Xueming Shen
Thanks Mandy for reviewing. Webrev has been updated to (1) added the sync back to charset/deleteCharset(). As you suggested, there is race condition risk that the map is being accessed while the system initialization completes. (2) added the holder pattern for msiso2022_jp and iso2022_jp_2, as

Re: RFR: 8013395 StringBuffer.toString performance regression impacting embedded benchmarks

2013-05-13 Thread Peter Levart
On 05/13/2013 09:05 PM, Alan Bateman wrote: On 13/05/2013 08:12, David Holmes wrote: Thanks for all the feedback and discussion. I've rewritten the patch to use Peter's suggestion of caching the char[] instead of the actual String: http://cr.openjdk.java.net/~dholmes/8013395/webrev.v3/ I

hg: jdk8/tl/langtools: 8012929: Trees.getElement should work not only for declaration trees, but also for use-trees

2013-05-13 Thread jonathan . gibbons
Changeset: 8dd528992c15 Author:jlahoda Date: 2013-05-10 15:15 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/8dd528992c15 8012929: Trees.getElement should work not only for declaration trees, but also for use-trees Reviewed-by: jjg Contributed-by: Dusan Balek

Re: RFR: 8013395 StringBuffer.toString performance regression impacting embedded benchmarks

2013-05-13 Thread Peter Levart
On 05/14/2013 12:09 AM, Peter Levart wrote: I noticed a synchronization bug in String.contentEquals method. If called with a StringBuffer argument while concurrent thread is modifying the StringBuffer, the method can either throw ArrayIndexOutOfBoundsException or return true even though the

Re: Review Request: BigInteger patch for efficient multiplication and division (#4837946)

2013-05-13 Thread Brian Burkhalter
On May 13, 2013, at 11:57 AM, Brian Burkhalter wrote: On May 11, 2013, at 8:35 PM, Alan Eliasen wrote: On 05/09/2013 03:02 PM, Brian Burkhalter wrote: First you have: /** * The threshold value for using 3-way Toom-Cook multiplication. * If the number of ints in both mag arrays are

RFR: 7038105 - File.isHidden() should return true for pagefile.sys and hiberfil.sys

2013-05-13 Thread Rob McKenna
Hi folks, Looking for a review of this suggested fix from Fujitsu. GetFileAttributesExW() returns ERROR_SHARING_VIOLATION for pagefile.sys and hiberfi.sys so we're falling back to FindFirstFile instead: http://cr.openjdk.java.net/~robm/7038105/webrev.01/ -Rob

Re: RFR 8012326: Deadlock occurs when Charset.availableCharsets() is called by several threads at the same time

2013-05-13 Thread Xueming Shen
Hi Mandy, Here is the updated simple version as we chatted. I will leave the ExtendedCharsets rewrite for next round. http://cr.openjdk.java.net/~sherman/8012326/webrev/ Thanks! -Sherman On 5/13/13 3:01 PM, Xueming Shen wrote: Thanks Mandy for reviewing. Webrev has been updated to (1)

hg: jdk8/tl/jdk: 8013386: (tz) Support tzdata2013c

2013-05-13 Thread xueming . shen
Changeset: ae35fdbab949 Author:sherman Date: 2013-05-13 20:35 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ae35fdbab949 8013386: (tz) Support tzdata2013c Summary: updated tz data to version 2013c Reviewed-by: peytoia, okutsu ! make/sun/javazic/tzdata/VERSION !

Re: Code review request for JDK-8014365 Restore Objects.requireNonNull(T, SupplierString)

2013-05-13 Thread Mike Duigou
Looks fine to me. Welcome back Objects.requireNonNull(obj, Supplier)! Mike On May 10 2013, at 14:01 , Joe Darcy wrote: Hello, Please (re)review this change to introduce Objects.requireNonNull(T, SupplierString): http://cr.openjdk.java.net/~darcy/8014365.0/ The original change had

Re: RFR: 8013395 StringBuffer.toString performance regression impacting embedded benchmarks

2013-05-13 Thread David Holmes
On 14/05/2013 5:05 AM, Alan Bateman wrote: On 13/05/2013 08:12, David Holmes wrote: Thanks for all the feedback and discussion. I've rewritten the patch to use Peter's suggestion of caching the char[] instead of the actual String: http://cr.openjdk.java.net/~dholmes/8013395/webrev.v3/ I too

Re: Code review request for JDK-8014365 Restore Objects.requireNonNull(T, SupplierString)

2013-05-13 Thread Joe Darcy
Thanks for the review Mike, -Joe On 05/13/2013 10:06 PM, Mike Duigou wrote: Looks fine to me. Welcome back Objects.requireNonNull(obj, Supplier)! Mike On May 10 2013, at 14:01 , Joe Darcy wrote: Hello, Please (re)review this change to introduce Objects.requireNonNull(T, SupplierString):

Race in String.contentEquals ( was Re: RFR: 8013395 StringBuffer.toString performance regression impacting embedded benchmarks)

2013-05-13 Thread David Holmes
Thanks Peter! I've filed 8014477 Race condition in String.contentEquals when comparing with StringBuffer On 14/05/2013 8:34 AM, Peter Levart wrote: On 05/14/2013 12:09 AM, Peter Levart wrote: I noticed a synchronization bug in String.contentEquals method. If called with a StringBuffer

Re: RFR [7021870] GzipInputStream closes underlying stream during reading

2013-05-13 Thread Xueming Shen
Ivan, It's a fair assessment to say ZIS.available() is not the only cause... though I don't think it's that easy to really craft a real test case to demonstrate the failure with a malformed gzip entry inside a zip file. At least I don't think I can create one without sitting down with some real

Re: Race in String.contentEquals ( was Re: RFR: 8013395 StringBuffer.toString performance regression impacting embedded benchmarks)

2013-05-13 Thread Peter Levart
Right, sb.length() should be used. I will correct that as soon as I get to the keyboard. Regards, Peter On May 14, 2013 7:22 AM, David Holmes david.hol...@oracle.com wrote: Thanks Peter! I've filed 8014477 Race condition in String.contentEquals when comparing with StringBuffer On