#13387 closed defect (fixed)
Java 9: ProjectionRegressionTest fails because it is expecting exact floating point values
Reported by: | michael2402 | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | 17.04 |
Component: | Unit tests | Version: | |
Keywords: | java9 projection | Cc: | bastiK |
Description
Currently, the ProjectionRegressionTest expects the floating point values to match exactly.
This makes the test fail in Java9 for me.
I suggest to either:
- Allow the values to differ slightly
- Make all projection code use
strictfp
Attachments (0)
Change History (31)
comment:1 Changed 21 months ago by
Keywords: | java9 projection added |
---|
comment:2 Changed 21 months ago by
comment:3 Changed 21 months ago by
Cc: | bastiK added |
---|
comment:4 Changed 21 months ago by
Summary: | ProjectionRegressionTest fails because it is expecting exact floating point values → Java 9: ProjectionRegressionTest fails because it is expecting exact floating point values |
---|
comment:5 Changed 21 months ago by
Milestone: | 16.08 → 16.09 |
---|
comment:6 Changed 21 months ago by
I traced it down to the sin/cos/... functions of Math
.
They seem to return results that are slightly off to the ones returned previously.
According to their javadoc, this is fine as long as they are of by at most 1 ULP.
I can't tell for sure but by the looks of it the numbers are off so slightly that they are probably within this range. Now the bug But: The StrictMath class is used internally and it's behavior has changed as well:
e.g. Java 9:
[junit] x: -0.13291887071328434 [junit] sin(x): -0.8197757986736175
Java 8:
[junit] x: -0.13291887071328434 [junit] sin(x): -0.8197757986736174
comment:10 Changed 17 months ago by
Milestone: | 16.12 → 17.01 |
---|
comment:11 Changed 16 months ago by
Milestone: | 17.01 → 17.04 |
---|
comment:12 Changed 13 months ago by
Milestone: | 17.04 → 17.05 |
---|
comment:13 follow-up: 14 Changed 13 months ago by
This is pretty interesting:
# Oracle Java 9 ea166 Custom x=-0.785406479576233 cos(x)=0.707100900735684 Math x=-0.785406479576233 cos(x)=0.707100900735684 StrictMath x=-0.785406479576233 cos(x)=0.7071009007356841
# Openjdk Java 1.8.0_91 & # Oracle 1.8.0_74 Custom x=-0.785406479576233 cos(x)=0.707100900735684 Math x=-0.785406479576233 cos(x)=0.7071009007356841 StrictMath x=-0.785406479576233 cos(x)=0.7071009007356841
In Java 9, Math.cos
has a different value than StrictMath.cos
, although in the Java code it is just a wrapper for StrictMath.cos
.
comment:14 Changed 13 months ago by
Replying to bastiK:
In Java 9,
Math.cos
has a different value thanStrictMath.cos
, although in the Java code it is just a wrapper forStrictMath.cos
.
In Java 9 b74+ following javabug:8076112, the method is now annotated with a new annotation @HotSpotIntrinsicCandidate, maybe this is linked?
comment:15 Changed 13 months ago by
I did a quick performance test and in Java 9, Math.cos
is 3 times faster than StrictMath.cos
. In Java 8 they are about the same (Math.cos
being a bit faster).
So it seems that Math.cos
is intrinsified in Java 9. It may have been before, but for sure there is a new implementation which is much faster and differs slightly in the numerical results. In any case, we should take advantage of that and not use the slower but supposedly more stable StrictMath.cos
.
See also #11889.
comment:16 Changed 13 months ago by
My take on how to handle this in ProjectionRegressionTest
:
- only run the test for Java 8
- when Java 9 is released and every developer can reasonably be expected to run Java 9, update test data and only run it for Java 9 from then on out
comment:17 Changed 13 months ago by
It's intrinsics for a long time, only the annotation is new and make it clear HotSpot won't run any Java code here.
The implementation symbol is _dcos
and defined here.
Here is the real deal.
The file macroAssembler_x86_cos.cpp
is new in Java 9 b120.
The history of previous file (macroAssembler_libm_x86_64.cpp
) shows an interesting javabug:8143353 which appears to be what we're looking for:
JDK-8143353
update for x86 sin and cos in the math lib
This change updates the sin and cos functions with a newer representation which increases performance on x86.
The performance gain is ~4.25x with both sin and cos.
So it appears Java 9 Math routines are based since b105 on highly optimized library provided by Intel: LibM.
comment:18 follow-up: 19 Changed 13 months ago by
Can't we use Math.ulp to allow the values to differ lightly between java 8 and 9?
comment:19 Changed 13 months ago by
Wow, thanks for the links, now we know for sure! Also quite impressive optimization. This will have a noticeable effect for many applications.
Replying to Don-vip:
Can't we use Math.ulp to allow the values to differ lightly between java 8 and 9?
Due to error accumulation, the final value differs by more than a few ULPs. Anyway, as long as the Java 8 test stays intact, I don't mind.
comment:23 Changed 13 months ago by
Milestone: | 17.05 → 17.04 |
---|
comment:24 Changed 13 months ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:25 Changed 13 months ago by
[12013] uses the Java 9 version of Math.toDegrees
and Math.toRadians
in projection code. Maybe the maximum difference can be lowered a bit now.
comment:28 Changed 13 months ago by
Quite nice.
I was interested to see, how the new Math.cos
implementation compares in precision:
It turns out, the value in Java 8 is 3.2 % of the times more than 0.5 ULP away from the true value (i.e. rounded incorrectly) and 0.32 % more than 0.6 ULP away. In Java 9 it is 0.16 % of the times more than 0.5 ULP away from the true value and never more than 0.6 ULP away. So the Java 9 algorithm does much better in terms of precision, also. (Sample is 40000 random values in the range 0 to 22.7.)
comment:31 Changed 8 months ago by
There's a talk from Intel at JavaOne about the Math changes: https://youtu.be/G7pfr2XyLfI?t=35m23s
I tried the first approach already, without success. I'd like to understand why Java 9 differs than 7 and 8 on this subject.