[cp-patches] Re: String class: hack for ORP 1.0.9
Hi David, On Tue, 2005-07-12 at 08:06 -0400, David P Grove wrote: So, I'm having a hard time seeing how this optimization actually makes the code faster under any reasonable assumptions of what an optimizing JIT is going to do. It seems mostly harmless to have it (although it makes the method larger, and thus a slightly less attractive candidate for inlining), but if it actually buys you any measurable speedup on a high performance VM, then you should really take a hard look at your VM/JIT and find out why they didn't do a good job on the unoptimized version in the first place. clone() on an array is just a short hand for a new followed by an arraycopy, and the new followed by arraycopy idiom shows up all over the place so you need to do a good job on it. I hear you :) Don't do performance hacks without actual benchmarking. I admit to have only done some quick tests on a none-high performance runtime (jamvm). And the numbers seemed to indicate that this special case was triggered a lot, so I just assumed it would be beneficial in general. Unfortunately I didn't have a working setup of jikesRVM and kaffe and gcj both have a slightly different implementation of this method. The reason I thought no reasonable optimizing JIT could actually optimize this even with the call to arraycopy was because it cannot know that when count == value.length then offset == 0 so no bounds-checks are needed. So hand-optimizing that case into a array.clone() seemed like a win. It would be interesting to see the actual code produced by JikesRVM before and after this optimization for this method. Ideally we would have a dedicated benchmark box and run free software benchmarks on it comparing CVS from day to day. I had wanted to set something up like that using DaCapo. Unfortunately the box I wanted to use is actually a multi-user/server machine, so that wouldn't generate reliable results. And DaCapo isn't actually free software, even though it is build from all kinds of real world free software programs and libraries (we had some discussions about that, but when I found out I didn't actually have a box to run it on I dropped the issue.). There is the Ashes Suite Collection though which we could use. Cheers, Mark signature.asc Description: This is a digitally signed message part ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: String class: hack for ORP 1.0.9
Hi David, On Tue, 2005-07-12 at 08:06 -0400, David P Grove wrote: So, I'm having a hard time seeing how this optimization actually makes the code faster under any reasonable assumptions of what an optimizing JIT is going to do. It seems mostly harmless to have it (although it makes the method larger, and thus a slightly less attractive candidate for inlining), but if it actually buys you any measurable speedup on a high performance VM, then you should really take a hard look at your VM/JIT and find out why they didn't do a good job on the unoptimized version in the first place. clone() on an array is just a short hand for a new followed by an arraycopy, and the new followed by arraycopy idiom shows up all over the place so you need to do a good job on it. I hear you :) Don't do performance hacks without actual benchmarking. I admit to have only done some quick tests on a none-high performance runtime (jamvm). And the numbers seemed to indicate that this special case was triggered a lot, so I just assumed it would be beneficial in general. Unfortunately I didn't have a working setup of jikesRVM and kaffe and gcj both have a slightly different implementation of this method. The reason I thought no reasonable optimizing JIT could actually optimize this even with the call to arraycopy was because it cannot know that when count == value.length then offset == 0 so no bounds-checks are needed. So hand-optimizing that case into a array.clone() seemed like a win. It would be interesting to see the actual code produced by JikesRVM before and after this optimization for this method. Ideally we would have a dedicated benchmark box and run free software benchmarks on it comparing CVS from day to day. I had wanted to set something up like that using DaCapo. Unfortunately the box I wanted to use is actually a multi-user/server machine, so that wouldn't generate reliable results. And DaCapo isn't actually free software, even though it is build from all kinds of real world free software programs and libraries (we had some discussions about that, but when I found out I didn't actually have a box to run it on I dropped the issue.). There is the Ashes Suite Collection though which we could use. Cheers, Mark signature.asc Description: This is a digitally signed message part ___ Classpath mailing list Classpath@gnu.org http://lists.gnu.org/mailman/listinfo/classpath
[cp-patches] Re: String class: hack for ORP 1.0.9
So, I'm having a hard time seeing how this optimization actually makes
the code faster under any reasonable assumptions of what an optimizing JIT
is going to do. It seems mostly harmless to have it (although it makes
the method larger, and thus a slightly less attractive candidate for
inlining), but if it actually buys you any measurable speedup on a high
performance VM, then you should really take a hard look at your VM/JIT
and find out why they didn't do a good job on the unoptimized version in
the first place. clone() on an array is just a short hand for a new
followed by an arraycopy, and the new followed by arraycopy idiom shows up
all over the place so you need to do a good job on it.
--dave
[EMAIL PROTECTED] wrote on 07/12/2005 04:54:01
AM:
On Tue, 2005-07-12 at 13:02 +1200, Simon Kitching wrote:
I just wondered if it was time to remove this hack...
Wow, that is a very old workaround. And indeed a nice optimization to
have. A quick startup of eclipse (with just a little project) shows 4642
hits of String.toCharArray() of which 4200 have (count == value.length).
Thanks for finding this.
Committed as:
Reported by Simon Kitching [EMAIL PROTECTED]
* java/lang/String.java (toCharArray): Return value.clone() when
count == value.length.
Cheers,
Mark
diff -u -r1.67 String.java
--- java/lang/String.java 11 Jul 2005 22:30:07 - 1.67
+++ java/lang/String.java 12 Jul 2005 08:48:23 -
@@ -1499,10 +1499,9 @@
*/
public char[] toCharArray()
{
-// XXX ORP 1.0.9 crashes on (char[]) clone() during bootstrap, so
we
-// omit this optimization for now.
-// if (count == value.length)
-// return (char[]) value.clone();
+if (count == value.length)
+ return (char[]) value.clone();
+
char[] copy = new char[count];
VMSystem.arraycopy(value, offset, copy, 0, count);
return copy;
[attachment signature.asc deleted by David P Grove/Watson/IBM]
___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath
___
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: String class: hack for ORP 1.0.9
On Tue, 2005-07-12 at 13:02 +1200, Simon Kitching wrote:
I just wondered if it was time to remove this hack...
Wow, that is a very old workaround. And indeed a nice optimization to
have. A quick startup of eclipse (with just a little project) shows 4642
hits of String.toCharArray() of which 4200 have (count == value.length).
Thanks for finding this.
Committed as:
Reported by Simon Kitching [EMAIL PROTECTED]
* java/lang/String.java (toCharArray): Return value.clone() when
count == value.length.
Cheers,
Mark
diff -u -r1.67 String.java
--- java/lang/String.java 11 Jul 2005 22:30:07 - 1.67
+++ java/lang/String.java 12 Jul 2005 08:48:23 -
@@ -1499,10 +1499,9 @@
*/
public char[] toCharArray()
{
-// XXX ORP 1.0.9 crashes on (char[]) clone() during bootstrap, so we
-// omit this optimization for now.
-// if (count == value.length)
-// return (char[]) value.clone();
+if (count == value.length)
+ return (char[]) value.clone();
+
char[] copy = new char[count];
VMSystem.arraycopy(value, offset, copy, 0, count);
return copy;
signature.asc
Description: This is a digitally signed message part
___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath
Re: String class: hack for ORP 1.0.9
So, I'm having a hard time seeing how this optimization actually makes
the code faster under any reasonable assumptions of what an optimizing JIT
is going to do. It seems mostly harmless to have it (although it makes
the method larger, and thus a slightly less attractive candidate for
inlining), but if it actually buys you any measurable speedup on a high
performance VM, then you should really take a hard look at your VM/JIT
and find out why they didn't do a good job on the unoptimized version in
the first place. clone() on an array is just a short hand for a new
followed by an arraycopy, and the new followed by arraycopy idiom shows up
all over the place so you need to do a good job on it.
--dave
[EMAIL PROTECTED] wrote on 07/12/2005 04:54:01
AM:
On Tue, 2005-07-12 at 13:02 +1200, Simon Kitching wrote:
I just wondered if it was time to remove this hack...
Wow, that is a very old workaround. And indeed a nice optimization to
have. A quick startup of eclipse (with just a little project) shows 4642
hits of String.toCharArray() of which 4200 have (count == value.length).
Thanks for finding this.
Committed as:
Reported by Simon Kitching [EMAIL PROTECTED]
* java/lang/String.java (toCharArray): Return value.clone() when
count == value.length.
Cheers,
Mark
diff -u -r1.67 String.java
--- java/lang/String.java 11 Jul 2005 22:30:07 - 1.67
+++ java/lang/String.java 12 Jul 2005 08:48:23 -
@@ -1499,10 +1499,9 @@
*/
public char[] toCharArray()
{
-// XXX ORP 1.0.9 crashes on (char[]) clone() during bootstrap, so
we
-// omit this optimization for now.
-// if (count == value.length)
-// return (char[]) value.clone();
+if (count == value.length)
+ return (char[]) value.clone();
+
char[] copy = new char[count];
VMSystem.arraycopy(value, offset, copy, 0, count);
return copy;
[attachment signature.asc deleted by David P Grove/Watson/IBM]
___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath
___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath
Re: String class: hack for ORP 1.0.9
Guess I'm showing my bias ;-) It is very easy to get the right thing to happen in Jikes RVM... In general, you are right about native methods being a barrier to JIT optimization (btw there was an interesting paper in VEE'05 last month by Stepanian et al on a system that can inline native methods and their JNI callbacks into JITed code). Arraycopy is just a very special case...it shows up as a bottleneck on enough benchmarks that many optimizing JITs will treat it as an intrinsic function (ie, there is a native method implementation in the VM for use by interpreted code, but the JITed code won't actually call it). In some slightly funky way of looking at the world, native methods like arraycopy are a special case where Java-in-Java and C-based VMs end up in pretty much the same place. It's a native method that the JIT really needs to grok to get good performance. In Java-in-Java that happens without much extra effort. In other VMs, you end up building up support for a set of performance critical intrinsics so that the JIT can completely understand their semantics and then implement them in a more optimal fashion than simply calling the backing native method. I should have made it more clear that I was talking about arraycopy in particular, not native methods in general. --dave Archie Cobbs [EMAIL PROTECTED] wrote on 07/12/2005 10:24:10 AM: David P Grove wrote: So, I'm having a hard time seeing how this optimization actually makes the code faster under any reasonable assumptions of what an optimizing JIT is going to do. It seems mostly harmless to have it (although it makes the method larger, and thus a slightly less attractive candidate for inlining), but if it actually buys you any measurable speedup on a high performance VM, then you should really take a hard look at your VM/JIT and find out why they didn't do a good job on the unoptimized version in the first place. clone() on an array is just a short hand for a new followed by an arraycopy, and the new followed by arraycopy idiom shows up all over the place so you need to do a good job on it. Not all VM's are high performance I guess :-) [I'm sure you know all this already but here goes..] E.g., on many VM's VMSystem.arraycopy() is a native method, and they can't optimize into that method. So all the normal type checking, array bounds checking, and array compatibility checking will be done by that native code in all cases, even though in this case we know it's not necessary. With array clone(), also typically a native method, none of that checking is ever needed. This is a good example of the advandages of a JVM written in Java (a coincidence? :-) There is no optimization barrier into native code like System.arraycopy(). -Archie __ Archie Cobbs *CTO, Awarix* http://www.awarix.com ___ Classpath mailing list Classpath@gnu.org http://lists.gnu.org/mailman/listinfo/classpath
Re: String class: hack for ORP 1.0.9
David P Grove wrote: So, I'm having a hard time seeing how this optimization actually makes the code faster under any reasonable assumptions of what an optimizing JIT is going to do. It seems mostly harmless to have it (although it makes the method larger, and thus a slightly less attractive candidate for inlining), but if it actually buys you any measurable speedup on a high performance VM, then you should really take a hard look at your VM/JIT and find out why they didn't do a good job on the unoptimized version in the first place. clone() on an array is just a short hand for a new followed by an arraycopy, and the new followed by arraycopy idiom shows up all over the place so you need to do a good job on it. Not all VM's are high performance I guess :-) [I'm sure you know all this already but here goes..] E.g., on many VM's VMSystem.arraycopy() is a native method, and they can't optimize into that method. So all the normal type checking, array bounds checking, and array compatibility checking will be done by that native code in all cases, even though in this case we know it's not necessary. With array clone(), also typically a native method, none of that checking is ever needed. This is a good example of the advandages of a JVM written in Java (a coincidence? :-) There is no optimization barrier into native code like System.arraycopy(). -Archie __ Archie Cobbs *CTO, Awarix* http://www.awarix.com ___ Classpath mailing list Classpath@gnu.org http://lists.gnu.org/mailman/listinfo/classpath
Re: String class: hack for ORP 1.0.9
Simon Kitching wrote:
public char[] toCharArray()
{
// XXX ORP 1.0.9 crashes on (char[]) clone() during bootstrap, so we
// omit this optimization for now.
// if (count == value.length)
// return (char[]) value.clone();
char[] copy = new char[count];
VMSystem.arraycopy(value, offset, copy, 0, count);
return copy;
}
I just wondered if it was time to remove this hack...
Sounds good to me... and in any case, ORP should be doing it's
own disoptimization for VM-specific issues like this rather than
forcing it on all Classpath users.
-Archie
__
Archie Cobbs *CTO, Awarix* http://www.awarix.com
___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath
