Modify

Opened 15 months ago

Closed 7 months ago

Last modified 7 weeks ago

#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 15 months ago by michael2402

Keywords: java9 projection added

comment:2 Changed 15 months ago by Don-vip

I tried the first approach already, without success. I'd like to understand why Java 9 differs than 7 and 8 on this subject.

comment:3 Changed 15 months ago by Don-vip

Cc: bastiK added

comment:4 Changed 15 months ago by Don-vip

Summary: ProjectionRegressionTest fails because it is expecting exact floating point valuesJava 9: ProjectionRegressionTest fails because it is expecting exact floating point values

comment:5 Changed 15 months ago by Don-vip

Milestone: 16.0816.09

comment:6 Changed 15 months ago by michael2402

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:7 Changed 14 months ago by simon04

Milestone: 16.0916.10

The old ticket is #11889.

comment:8 Changed 14 months ago by simon04

Milestone: 16.1016.11

Milestone renamed

comment:9 Changed 12 months ago by Don-vip

Milestone: 16.1116.12

Milestone renamed

comment:10 Changed 11 months ago by Don-vip

Milestone: 16.1217.01

comment:11 Changed 10 months ago by Don-vip

Milestone: 17.0117.04

comment:12 Changed 7 months ago by Don-vip

Milestone: 17.0417.05

comment:13 Changed 7 months ago by bastiK

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 in reply to:  13 Changed 7 months ago by Don-vip

Replying to bastiK:

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.

In Java 9 b74+ following javabug:8076112, the method is now annotated with a new annotation @HotSpotIntrinsicCandidate, maybe this is linked?

Last edited 7 months ago by Don-vip (previous) (diff)

comment:15 Changed 7 months ago by bastiK

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 7 months ago by bastiK

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 7 months ago by Don-vip

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.

Last edited 7 months ago by Don-vip (previous) (diff)

comment:18 Changed 7 months ago by Don-vip

Can't we use Math.ulp to allow the values to differ lightly between java 8 and 9?

comment:19 in reply to:  18 Changed 7 months ago by bastiK

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:20 Changed 7 months ago by Don-vip

In 11999/josm:

see #13387, see #11924 - allow a difference of 3500 ULPs for Java 9 ProjectionRegressionTest (due to Math.cos change of behaviour in JDK-8143353)

comment:21 Changed 7 months ago by bastiK

In 12001/josm:

see #13387 - simpler logic

comment:22 Changed 7 months ago by Don-vip

In 12003/josm:

see #13387, see #11924 - raise the allowed difference to 35000 ULPs to catch the worst case (34813 ULP). Logic to remove when JOSM switches to Java 9

comment:23 Changed 7 months ago by Don-vip

Milestone: 17.0517.04

comment:24 Changed 7 months ago by Don-vip

Resolution: fixed
Status: newclosed

comment:25 Changed 7 months ago by bastiK

[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:26 Changed 7 months ago by Don-vip

In 12021/josm:

see #13387 - lower the max difference to 850 ULPs

comment:27 Changed 7 months ago by Don-vip

In 12022/josm:

see #13387 - forgot debug code

comment:28 Changed 7 months ago by bastiK

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:29 Changed 7 months ago by Don-vip

It's great to see Java improve this way :)

comment:30 Changed 6 months ago by Don-vip

In 12131/josm:

see #11889, see #11924, see #13387 - use backported versions of Math.toDegrees/toRadians (more accurate and faster) - to revert when migrating to Java 9

comment:31 Changed 7 weeks ago by Don-vip

There's a talk from Intel at JavaOne about the Math changes: https://youtu.be/G7pfr2XyLfI?t=35m23s

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.