Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

uot; -Ulf Jason > Date: Sun, 28 Feb 2010 17:09:15 +0100 > From: ulf.zi...@gmx.de > To: alan.bate...@sun.com > Subject: Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode) > CC: core-libs-dev@openjdk.java.net; dmitry.nadez...@gmail.

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

2010/2/28 Ulf Zibis : > Am 25.02.2010 23:07, schrieb Alan Bateman: >> >> Kelly O'Hair wrote: >>> >>> Yup.  My eyes must be tired, I didn't see that. :^( >> >> Too many repositories in the air at the same time. The webrev has been >> refreshed. Thanks Ulf. >> >> > > Another thought: > > In the const

RE: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

String.hash should only have two known states, zero and the actual computed hash code. http://bugs.sun.com/view_bug.do?bug_id=6611830 Jason > Date: Sun, 28 Feb 2010 17:09:15 +0100 > From: ulf.zi...@gmx.de > To: alan.bate...@sun.com > Subject: Re: Need reviewer for for

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Am 25.02.2010 23:07, schrieb Alan Bateman: Kelly O'Hair wrote: Yup. My eyes must be tired, I didn't see that. :^( Too many repositories in the air at the same time. The webrev has been refreshed. Thanks Ulf. Another thought: In the constructors of String we could initialize hash = Integ

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Am 26.02.2010 20:32, schrieb Alan Bateman: Ulf Zibis wrote: For these other suggestions it would be great to create micro-benchmarks and try our your changes. For now though, as I said, I'm just fixing the method to avoid the store for the empty string case. I've added benchmark for "same" str

RE: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

that makes version 6 a good candidate to benchmark. Jason Date: Sat, 27 Feb 2010 19:11:05 +0100 From: ulf.zi...@gmx.de To: jason_mehr...@hotmail.com CC: dmitry.nadez...@gmail.com; core-libs-dev@openjdk.java.net Subject: Re: Need reviewer for forward port of 6815768 (File.getXXXSpace

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Am 26.02.2010 16:27, schrieb Dmitry Nadezhin: On Fri, Feb 26, 2010 at 6:19 PM, Ulf Zibis > wrote: Am 26.02.2010 12:52, schrieb Dmitry Nadezhin: I found two alternatives in the link http://mail.openjdk.java.net/pipermail/coin-dev/2009-December/002

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

e: Sat, 27 Feb 2010 17:00:14 +0300 Subject: Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode) From: dmitry.nadez...@gmail.com To: core-libs-dev@openjdk.java.net Ulf, I ran Caliper benchmarks on different variants of hashCode(). http://code.google.com/p/

RE: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

= value; for (int i = 0; i < len; i++) { h = 31*h + val[off++]; } hash = h; } return h; } Jason Date: Sat, 27 Feb 2010 17:00:14 +0300 Subject: Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 68

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Dmitry, thanks for the interesting results. Your results are not as promising as mine. I guess, referring to the HotSpot's dissassembler dump would give more hints. Additionally different string lengths, repeat factors, -Xbatch, and GC settings would help. Good trick to use substring(1) for

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Ulf, I ran Caliper benchmarks on different variants of hashCode(). http://code.google.com/p/caliper/ The results and the Caliper code are below: -Dima - -d32 -clien 0% Scenario{vm=java -d32 -client, benchmark=HashCodeEmpty} 8,85ns; σ=0,06ns @ 3 trials 8% Scenario{vm=java -d32 -client

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Am 26.02.2010 20:32, schrieb Alan Bateman: Ulf Zibis wrote: For these other suggestions it would be great to create micro-benchmarks and try our your changes. For now though, as I said, I'm just fixing the method to avoid the store for the empty string case. Here the benchmark for my different

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Am 26.02.2010 20:32, schrieb Alan Bateman: Ulf Zibis wrote: For these other suggestions it would be great to create micro-benchmarks and try our your changes. For now though, as I said, I'm just fixing the method to avoid the store for the empty string case. I've run a benchmark for your "a

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Ulf Zibis wrote: Am 26.02.2010 16:25, schrieb Ulf Zibis: So optimum could be: I have one more ... Additionally we can save double incrementing of int i and off: public int hashCode() { int h = hash; if (h == 0) { int len = count; if (len> 0) { char[] val = value; // f

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Am 26.02.2010 17:10, schrieb Ulf Zibis: Additionally we can save double incrementing of int i and off: public int hashCode() { int h = hash; if (h == 0) { int len = count; if (len> 0) { char[] val = value; // for (int i = offset, len += i; i< len; ) // would be nice syn

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Additionally I think: char[] val = value; looks better then: char val[] = value; -Ulf Am 25.02.2010 20:08, schrieb Alan Bateman: Kelly O'Hair wrote: Looks fine to me. -kto Thanks Kelly and also to Joe for pointing out that I botched the link to the second one - here is the

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Am 26.02.2010 16:25, schrieb Ulf Zibis: So optimum could be: I have one more ... Additionally we can save double incrementing of int i and off: public int hashCode() { int h = hash; if (h == 0) { int len = count; if (len> 0) { char[] val = value; // for (int i = offset

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

On Fri, Feb 26, 2010 at 6:19 PM, Ulf Zibis wrote: > Am 26.02.2010 12:52, schrieb Dmitry Nadezhin: > > I found two alternatives in the link >> http://mail.openjdk.java.net/pipermail/coin-dev/2009-December/002618.html >> The first alternative >> int equalByHashThreshold = 2; >> >> public boolean

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Am 26.02.2010 13:58, schrieb Alan Bateman: Jason Mehrens wrote: Alan, Shouldn't the loading of 'this.count' into 'len' be only performed if 'h' is zero? Otherwise, when hash is not zero we perform a little unnecessary work every time hashCode is called. Jason That's a good suggestion altho

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Am 26.02.2010 12:52, schrieb Dmitry Nadezhin: I found two alternatives in the link http://mail.openjdk.java.net/pipermail/coin-dev/2009-December/002618.html The first alternative int equalByHashThreshold = 2; public boolean equals(Object anObject) { if (this == anObject) { return tr

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Ulf Zibis wrote: Have you seen, that there are some chances to make String#equals little faster too: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6912520 -Ulf Yes, I have seen it, but for now anyway, I'm not actually doing any work on String but rather just bringing forward fixes that we

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Jason Mehrens wrote: Alan, Shouldn't the loading of 'this.count' into 'len' be only performed if 'h' is zero? Otherwise, when hash is not zero we perform a little unnecessary work every time hashCode is called. Jason That's a good suggestion although it might be hard to measure any differ

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

I found two alternatives in the link http://mail.openjdk.java.net/pipermail/coin-dev/2009-December/002618.html The first alternative int equalByHashThreshold = 2; public boolean equals(Object anObject) { if (this == anObject) { return true; } if (anObject instanceof String) { Stri

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Have you seen, that there are some chances to make String#equals little faster too: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6912520 -Ulf Am 25.02.2010 23:07, schrieb Alan Bateman: Kelly O'Hair wrote: Yup. My eyes must be tired, I didn't see that. :^( Too many repositories in the

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Andrew John Hughes wrote: On 25 February 2010 18:19, Kelly O'Hair wrote: Looks fine to me. -kto On 2/25/10 9:51 AM, Alan Bateman wrote: There are two fixes in Sun's proprietary jdk6 updates that should also be fixed in OpenJDK. The first one is that the File.getXXXSpace methods use

RE: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

t; From: ulf.zi...@gmx.de > To: alan.bate...@sun.com > Subject: Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and > 6815768 (String.hashCode) > CC: core-libs-dev@openjdk.java.net; kelly.oh...@sun.com > > Why don't you use the faster local copy of count for

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Kelly O'Hair wrote: Yup. My eyes must be tired, I didn't see that. :^( Too many repositories in the air at the same time. The webrev has been refreshed. Thanks Ulf.

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Yup. My eyes must be tired, I didn't see that. :^( -kto On 2/25/10 12:17 PM, Ulf Zibis wrote: Why don't you use the faster local copy of count for the junction like: if (h == 0&& len> 0) { ? -Ulf Am 25.02.2010 20:08, schrieb Alan Bateman: Kelly O'Hair wrote: Looks fine to me. -kto Th

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Why don't you use the faster local copy of count for the junction like: if (h == 0&& len> 0) { ? -Ulf Am 25.02.2010 20:08, schrieb Alan Bateman: Kelly O'Hair wrote: Looks fine to me. -kto Thanks Kelly and also to Joe for pointing out that I botched the link to the second one - here

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

DOH! Yeah that one looks better... ;^) Thought they looked the same before. -kto On 2/25/10 11:08 AM, Alan Bateman wrote: Kelly O'Hair wrote: Looks fine to me. -kto Thanks Kelly and also to Joe for pointing out that I botched the link to the second one - here is the right webrev: http://cr.

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Kelly O'Hair wrote: Looks fine to me. -kto Thanks Kelly and also to Joe for pointing out that I botched the link to the second one - here is the right webrev: http://cr.openjdk.java.net/~alanb/6921374/webrev/ -Alan

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

On 25 February 2010 18:19, Kelly O'Hair wrote: > Looks fine to me. > > -kto > > On 2/25/10 9:51 AM, Alan Bateman wrote: >> >> There are two fixes in Sun's proprietary jdk6 updates that should also >> be fixed in OpenJDK. >> >> The first one is that the File.getXXXSpace methods use statvfs instead

Re: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

Looks fine to me. -kto On 2/25/10 9:51 AM, Alan Bateman wrote: There are two fixes in Sun's proprietary jdk6 updates that should also be fixed in OpenJDK. The first one is that the File.getXXXSpace methods use statvfs instead of statvfs64 and so fail on 32-bit Solaris and Linux with very larg