Recommend java.lang.Math instead of android.util.FloatMath

Apparently java.lang.Math is faster than FloatMath now with the newer
JITs. This reverses the old lint rule which would recommend using
FloatMath instead of Math with the opposite recommendation, provided
minSdkVersion is Froyo or greater. See issue 36199 for more.

Change-Id: I362cb7da011d39f5620db96e113f8255a828d40b

4 files changed, 56 insertions, 65 deletions

diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/MathDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/MathDetector.java
index 7d6424b..65eff86 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/MathDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/MathDetector.java
@@ -16,9 +16,6 @@
 
 package com.android.tools.lint.checks;
 
-import static com.android.tools.lint.detector.api.LintUtils.getNextOpcode;
-import static com.android.tools.lint.detector.api.LintUtils.getPrevOpcode;
-
 import com.android.annotations.NonNull;
 import com.android.annotations.Nullable;
 import com.android.tools.lint.detector.api.Category;
@@ -29,7 +26,6 @@ import com.android.tools.lint.detector.api.Scope;
 import com.android.tools.lint.detector.api.Severity;
 import com.android.tools.lint.detector.api.Speed;
 
-import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.tree.ClassNode;
 import org.objectweb.asm.tree.MethodInsnNode;
 import org.objectweb.asm.tree.MethodNode;
@@ -45,21 +41,20 @@ public class MathDetector extends Detector implements Detector.ClassScanner {
     /** The main issue discovered by this detector */
     public static final Issue ISSUE = Issue.create(
             "FloatMath", //$NON-NLS-1$
-            "Suggests replacing java.lang.Math calls with android.util.FloatMath to " +
-            "avoid conversions",
+            "Suggests replacing android.util.FloatMath calls with java.lang.Math",
 
-            "On modern hardware, \"double\" is just as fast as \"float\" though of course " +
-            "it takes more memory. However, if you are using floats and you need to compute " +
-            "the sine, cosine or square root, then it is better to use the " +
-            "android.util.FloatMath class instead of java.lang.Math since you can call methods " +
-            "written to operate on floats, so you avoid conversions back and forth to double.",
+            "In older versions of Android, using android.util.FloatMath was recommended " +
+            "for performance reasons when operating on floats. However, on modern hardware " +
+            "doubles are just as fast as float (though they take more memory), and in " +
+            "recent versions of Android, FloatMath is actually slower than using java.lang.Math " +
+            "due to the way the JIT optimizes java.lang.Math. Therefore, you should use " +
+            "Math instead of FloatMath if you are only targeting Froyo and above.",
 
             Category.PERFORMANCE,
             3,
             Severity.WARNING,
             MathDetector.class,
             Scope.CLASS_FILE_SCOPE).setMoreInfo(
-               //"http://developer.android.com/reference/android/util/FloatMath.html"); //$NON-NLS-1$
                "http://developer.android.com/guide/practices/design/performance.html#avoidfloat"); //$NON-NLS-1$
 
     /** Constructs a new {@link MathDetector} check */
@@ -90,25 +85,12 @@ public class MathDetector extends Detector implements Detector.ClassScanner {
             @NonNull MethodNode method, @NonNull MethodInsnNode call) {
         String owner = call.owner;
 
-        if (owner.equals("java/lang/Math")) { //$NON-NLS-1$
-            String name = call.name;
-            boolean paramFromFloat = getPrevOpcode(call) == Opcodes.F2D;
-            boolean returnToFloat = getNextOpcode(call) == Opcodes.D2F;
-            if (paramFromFloat || returnToFloat) {
-                String message;
-                if (paramFromFloat) {
-                    message = String.format(
-                            "Use android.util.FloatMath#%1$s() instead of " +
-                            "java.lang.Math#%1$s to avoid argument float to " +
-                            "double conversion", name);
-                } else {
-                    message = String.format(
-                            "Use android.util.FloatMath#%1$s() instead of " +
-                            "java.lang.Math#%1$s to avoid double to float return " +
-                            "value conversion", name);
-                }
-                context.report(ISSUE, method, context.getLocation(call), message, null /*data*/);
-            }
+        if (owner.equals("android/util/FloatMath")  //$NON-NLS-1$
+                && context.getProject().getMinSdk() >= 8) {
+            String message = String.format(
+                    "Use java.lang.Math#%1$s instead of android.util.FloatMath#%1$s() " +
+                    "since it is faster as of API 8", call.name);
+            context.report(ISSUE, method, context.getLocation(call), message, null /*data*/);
         }
     }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/MathDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/MathDetectorTest.java
index 8ec64ec..da12bda 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/MathDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/MathDetectorTest.java
@@ -27,17 +27,22 @@ public class MathDetectorTest extends AbstractCheckTest {
 
     public void test() throws Exception {
         assertEquals(
-            "src/test/bytecode/MathTest.java:12: Warning: Use android.util.FloatMath#sqrt() instead of java.lang.Math#sqrt to avoid argument float to double conversion [FloatMath]\n" +
-            "  floatResult = (float) Math.sqrt(x);\n" +
-            "                             ~~~~\n" +
-            "src/test/bytecode/MathTest.java:18: Warning: Use android.util.FloatMath#sqrt() instead of java.lang.Math#sqrt to avoid double to float return value conversion [FloatMath]\n" +
-            "  floatResult = (float) Math.sqrt(x);\n" +
-            "                             ~~~~\n" +
-            "src/test/bytecode/MathTest.java:23: Warning: Use android.util.FloatMath#sqrt() instead of java.lang.Math#sqrt to avoid argument float to double conversion [FloatMath]\n" +
-            "  doubleResult = Math.sqrt(x);\n" +
-            "                      ~~~~\n" +
-            "0 errors, 3 warnings\n" +
-            "",
+            "src/test/bytecode/MathTest.java:11: Warning: Use java.lang.Math#cos instead of android.util.FloatMath#cos() since it is faster as of API 8 [FloatMath]\n" +
+            "        floatResult = FloatMath.cos(x);\n" +
+            "                                ~~~\n" +
+            "src/test/bytecode/MathTest.java:12: Warning: Use java.lang.Math#sin instead of android.util.FloatMath#sin() since it is faster as of API 8 [FloatMath]\n" +
+            "        floatResult = FloatMath.sin((float) y);\n" +
+            "                                ~~~\n" +
+            "src/test/bytecode/MathTest.java:13: Warning: Use java.lang.Math#ceil instead of android.util.FloatMath#ceil() since it is faster as of API 8 [FloatMath]\n" +
+            "        floatResult = android.util.FloatMath.ceil((float) y);\n" +
+            "                                             ~~~~\n" +
+            "src/test/bytecode/MathTest.java:14: Warning: Use java.lang.Math#floor instead of android.util.FloatMath#floor() since it is faster as of API 8 [FloatMath]\n" +
+            "        System.out.println(FloatMath.floor(x));\n" +
+            "                                     ~~~~~\n" +
+            "src/test/bytecode/MathTest.java:15: Warning: Use java.lang.Math#sqrt instead of android.util.FloatMath#sqrt() since it is faster as of API 8 [FloatMath]\n" +
+            "        System.out.println(FloatMath.sqrt(z));\n" +
+            "                                     ~~~~\n" +
+            "0 errors, 5 warnings\n",
 
             lintProject(
                     "bytecode/.classpath=>.classpath",
@@ -46,4 +51,17 @@ public class MathDetectorTest extends AbstractCheckTest {
                     "bytecode/MathTest.class.data=>bin/classes/test/bytecode/MathTest.class"
                     ));
     }
+
+    public void testNoWarningsPreFroyo() throws Exception {
+        assertEquals(
+            "No warnings.",
+
+            lintProject(
+                    "bytecode/.classpath=>.classpath",
+                    "apicheck/minsdk2.xml=>AndroidManifest.xml",
+                    "bytecode/MathTest.java.txt=>src/test/bytecode/MathTest.java",
+                    "bytecode/MathTest.class.data=>bin/classes/test/bytecode/MathTest.class"
+                    ));
+    }
+
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/MathTest.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/MathTest.class.data
index 9b419fa..6647f1c 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/MathTest.class.data
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/MathTest.class.data
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/MathTest.java.txt b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/MathTest.java.txt
index 684e654..0193f81 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/MathTest.java.txt
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/MathTest.java.txt
@@ -1,30 +1,21 @@
 package test.bytecode;
 
+import android.util.FloatMath;
+
 //Test data for the MathDetector
 public class MathTest {
-	public float floatResult;
-	public double doubleResult;
-
-	public void floatToFloatTest(float x) {
-		// This forces a cast of the argument
-		// from a float to a double, as well
-		// as a cast of the return code back to a float
-		floatResult = (float) Math.sqrt(x);
-	}
-
-	public void doubleToFloat(double x) {
-		// This forces a cast of the return value
-		// to a float
-		floatResult = (float) Math.sqrt(x);
-	}
+    public float floatResult;
+    public double doubleResult;
 
-	public void floatToDouble(float x) {
-		// This causes a cast of the float argument to a double
-		doubleResult = Math.sqrt(x);
-	}
+    public void floatToFloatTest(float x, double y, int z) {
+        floatResult = FloatMath.cos(x);
+        floatResult = FloatMath.sin((float) y);
+        floatResult = android.util.FloatMath.ceil((float) y);
+        System.out.println(FloatMath.floor(x));
+        System.out.println(FloatMath.sqrt(z));
 
-	public void doubleToDouble(double x) {
-		// No casting
-		doubleResult = Math.sqrt(x);
-	}
+        // No warnings for plain math
+        floatResult = (float) Math.cos(x);
+        floatResult = (float) java.lang.Math.sin(x);
+    }
 }