Bug 33741 - Float.compare(float, float) and Double.compare(double, double) are very expensive and in the heart of key loops
Summary: Float.compare(float, float) and Double.compare(double, double) are very expen...
Status: RESOLVED FIXED
Alias: None
Product: classpath
Classification: Unclassified
Component: classpath (show other bugs)
Version: unspecified
: P3 normal
Target Milestone: 0.96
Assignee: Andrew John Hughes
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-10-11 19:59 UTC by Andrew John Hughes
Modified: 2007-10-12 09:54 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-10-11 21:57:51


Attachments
Patch from JikesRVM (526 bytes, patch)
2007-10-11 20:30 UTC, Andrew John Hughes
Details | Diff
Patch committed to Classpath (630 bytes, patch)
2007-10-12 08:53 UTC, Andrew John Hughes
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew John Hughes 2007-10-11 19:59:44 UTC
From RVM-26: http://jira.codehaus.org/browse/RVM-26

In Arrays.qsort the methods Float.compare and Double.compare are used depending on the values in the array. The compare operations perform the following (copied from GNU Classpath): 
if (isNaN)
 return isNaN ? 0 : 1;
 if (isNaN)
 return -1;
 // recall that 0.0 == -0.0, so we convert to infinites and try again
 if (x == 0 && y == 0)
 return (int) (1 / x - 1 / y);
 if (x == y)
 return 0;
 return x > y ? 1 : -1;
In the normal case we're going to hit 6 floating point compares. The case of 0, 0 is common due to using qsort on branch profiles, and this results in 2 divides and 1 subtract. Given we're just comparing to values we should be able to do this substantially cheaper in a VM specific/magic version.

A partial fix to this issue is to use floatToIntBits to inspect the sign bit of the float. By dismissing the case where the two values have identical bit patterns and NaNs, we can do a cheap test of whether the sign bits differ where it doesn't matter whether x and y are 0.0 or not (in the following code this is ix_sign - iy_sign). This code removes one compare from the normal path as well as making 2 of the compares int rather than float comparisons: 
 public static int compare(float x, float y)
 {
 int ix = floatToIntBits;
 int iy = floatToIntBits;
 if (ix == iy) return 0;
 if (isNaN) return 1;
 if (isNaN) return -1;
 int ix_sign = ix>>31;
 int iy_sign = iy>>31;
 if (ix_sign == iy_sign) { return x > y ? 1 : -1; } else { return ix_sign - iy_sign; }
 }
this code speeds up an optimizing compiler build on IA32 by a little under 8%. It slows a baseline compiler build because of the method call overhead. I will make and submit a Classpath patch for this and for the Double case. We're still not fully exploiting the fact that a comparison of x and y would give us NaN information on both of them. NaN's are the uncommon case but they take precedence here in order to maintain correct semantics
Comment 1 Andrew John Hughes 2007-10-11 20:30:59 UTC
Created attachment 14341 [details]
Patch from JikesRVM
Comment 2 Andrew John Hughes 2007-10-11 21:57:51 UTC
I'm committing this, based on the discussion by Ian Rogers and Andrew Haley on the Classpath list.

http://www.nabble.com/Float-Double-compare-tf4006194.html
Comment 3 Andrew John Hughes 2007-10-12 08:53:14 UTC
Created attachment 14344 [details]
Patch committed to Classpath
Comment 4 Andrew John Hughes 2007-10-12 08:53:32 UTC
Patch committed. Closing.