[SYSTEMML-1894] Performance log operations (replace FastMath by Math)

SystemML uses commons-math's FastMath instead of Math for certain
builtin operations due to performance and/or accuracy reasons. However,
recent experiments with jdk8 on different platforms showed that for log
operations this is meanwhile actually counter-productive. Hence, this
patch globally replaces FastMath.log with Math.log. 

On a scenario of 100 iterations of log(X), where X is a 10Kx10K dense
matrix, this patch improved performance from 413.1s to 319.8s.

Project: http://git-wip-us.apache.org/repos/asf/systemml/repo
Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/137fbf18
Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/137fbf18
Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/137fbf18

Branch: refs/heads/master
Commit: 137fbf18ab61968fe3af1e386812e6ee51e98084
Parents: 6b1c6f0
Author: Matthias Boehm <[email protected]>
Authored: Thu Sep 7 13:02:10 2017 -0700
Committer: Matthias Boehm <[email protected]>
Committed: Thu Sep 7 13:02:45 2017 -0700

----------------------------------------------------------------------
 .../sysml/hops/codegen/cplan/CNodeBinary.java   |  4 +-
 .../sysml/hops/codegen/cplan/CNodeUnary.java    |  4 +-
 .../runtime/codegen/LibSpoofPrimitives.java     |  8 ++--
 .../sysml/runtime/functionobjects/Builtin.java  | 44 +++++++-------------
 .../runtime/matrix/data/LibMatrixMult.java      | 12 +++---
 5 files changed, 30 insertions(+), 42 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/systemml/blob/137fbf18/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeBinary.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeBinary.java 
b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeBinary.java
index 4bbf205..926dd4d 100644
--- a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeBinary.java
+++ b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeBinary.java
@@ -176,9 +176,9 @@ public class CNodeBinary extends CNode
                                case MAX:
                                        return "    double %TMP% = (%IN1% >= 
%IN2%) ? %IN1% : %IN2%;\n";
                                case LOG:
-                                       return "    double %TMP% = 
FastMath.log(%IN1%)/FastMath.log(%IN2%);\n";
+                                       return "    double %TMP% = 
Math.log(%IN1%)/Math.log(%IN2%);\n";
                                case LOG_NZ:
-                                       return "    double %TMP% = (%IN1% == 0) 
? 0 : FastMath.log(%IN1%)/FastMath.log(%IN2%);\n";      
+                                       return "    double %TMP% = (%IN1% == 0) 
? 0 : Math.log(%IN1%)/Math.log(%IN2%);\n";      
                                case POW:
                                        return "    double %TMP% = 
Math.pow(%IN1%, %IN2%);\n";
                                case MINUS1_MULT:

http://git-wip-us.apache.org/repos/asf/systemml/blob/137fbf18/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeUnary.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeUnary.java 
b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeUnary.java
index 02f00b8..a4aa093 100644
--- a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeUnary.java
+++ b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeUnary.java
@@ -108,7 +108,7 @@ public class CNodeUnary extends CNode
                                case SQRT:
                                        return "    double %TMP% = 
Math.sqrt(%IN1%);\n";
                                case LOG:
-                                       return "    double %TMP% = 
FastMath.log(%IN1%);\n";
+                                       return "    double %TMP% = 
Math.log(%IN1%);\n";
                                case ROUND: 
                                        return "    double %TMP% = 
Math.round(%IN1%);\n";
                                case CEIL:
@@ -122,7 +122,7 @@ public class CNodeUnary extends CNode
                                case SIGMOID:
                                        return "    double %TMP% = 1 / (1 + 
FastMath.exp(-%IN1%));\n";
                                case LOG_NZ:
-                                       return "    double %TMP% = (%IN1%==0) ? 
0 : FastMath.log(%IN1%);\n";
+                                       return "    double %TMP% = (%IN1%==0) ? 
0 : Math.log(%IN1%);\n";
                                        
                                default: 
                                        throw new RuntimeException("Invalid 
unary type: "+this.toString());

http://git-wip-us.apache.org/repos/asf/systemml/blob/137fbf18/src/main/java/org/apache/sysml/runtime/codegen/LibSpoofPrimitives.java
----------------------------------------------------------------------
diff --git 
a/src/main/java/org/apache/sysml/runtime/codegen/LibSpoofPrimitives.java 
b/src/main/java/org/apache/sysml/runtime/codegen/LibSpoofPrimitives.java
index eed8cb3..e061064 100644
--- a/src/main/java/org/apache/sysml/runtime/codegen/LibSpoofPrimitives.java
+++ b/src/main/java/org/apache/sysml/runtime/codegen/LibSpoofPrimitives.java
@@ -832,25 +832,25 @@ public class LibSpoofPrimitives
        
        public static void vectLogAdd(double[] a, double[] c, int ai, int ci, 
int len) {
                for( int j = ai; j < ai+len; j++, ci++)
-                       c[ci] += FastMath.log(a[j]);
+                       c[ci] += Math.log(a[j]);
        }
 
        public static void vectLogAdd(double[] a, double[] c, int[] aix, int 
ai, int ci, int alen, int len) {
                for( int j = ai; j < ai+alen; j++ )
-                       c[ci + aix[j]] += FastMath.log(a[j]);
+                       c[ci + aix[j]] += Math.log(a[j]);
        }
        
        public static double[] vectLogWrite(double[] a, int ai, int len) {
                double[] c = allocVector(len, false);
                for( int j = 0; j < len; j++, ai++)
-                       c[j] = FastMath.log(a[ai]);
+                       c[j] = Math.log(a[ai]);
                return c;
        }
 
        public static double[] vectLogWrite(double[] a, int[] aix, int ai, int 
alen, int len) {
                double[] c = allocVector(len, true, Double.NEGATIVE_INFINITY);
                for( int j = ai; j < ai+alen; j++ )
-                       c[aix[j]] = FastMath.log(a[j]);
+                       c[aix[j]] = Math.log(a[j]);
                return c;
        }
        

http://git-wip-us.apache.org/repos/asf/systemml/blob/137fbf18/src/main/java/org/apache/sysml/runtime/functionobjects/Builtin.java
----------------------------------------------------------------------
diff --git 
a/src/main/java/org/apache/sysml/runtime/functionobjects/Builtin.java 
b/src/main/java/org/apache/sysml/runtime/functionobjects/Builtin.java
index 41388a1..753e494 100644
--- a/src/main/java/org/apache/sysml/runtime/functionobjects/Builtin.java
+++ b/src/main/java/org/apache/sysml/runtime/functionobjects/Builtin.java
@@ -284,12 +284,12 @@ public class Builtin extends ValueFunction
                        case ATAN:   return Math.atan(in); //faster in Math
                        case CEIL:   return FASTMATH ? FastMath.ceil(in) : 
Math.ceil(in);
                        case FLOOR:  return FASTMATH ? FastMath.floor(in) : 
Math.floor(in);
-                       case LOG:    return FASTMATH ? FastMath.log(in) : 
Math.log(in);         
-                       case LOG_NZ: return (in==0) ? 0 : FASTMATH ? 
FastMath.log(in) : Math.log(in);
-                       case ABS:    return Math.abs(in); //no need for 
FastMath                        
-                       case SIGN:       return FASTMATH ? FastMath.signum(in) 
: Math.signum(in);                       
-                       case SQRT:   return Math.sqrt(in); //faster in Math     
        
-                       case EXP:    return FASTMATH ? FastMath.exp(in) : 
Math.exp(in);         
+                       case LOG:    return Math.log(in); //faster in Math
+                       case LOG_NZ: return (in==0) ? 0 : Math.log(in); 
//faster in Math
+                       case ABS:    return Math.abs(in); //no need for FastMath
+                       case SIGN:       return FASTMATH ? FastMath.signum(in) 
: Math.signum(in);
+                       case SQRT:   return Math.sqrt(in); //faster in Math
+                       case EXP:    return FASTMATH ? FastMath.exp(in) : 
Math.exp(in);
                        case ROUND: return Math.round(in); //no need for 
FastMath
                        
                        case PLOGP:
@@ -297,8 +297,8 @@ public class Builtin extends ValueFunction
                                        return 0.0;
                                else if (in < 0)
                                        return Double.NaN;
-                               else
-                                       return (in * (FASTMATH ? 
FastMath.log(in) : Math.log(in)));
+                               else //faster in Math
+                                       return in * Math.log(in);
                        
                        case SPROP:
                                //sample proportion: P*(1-P)
@@ -375,17 +375,11 @@ public class Builtin extends ValueFunction
                        }
                        // *** END HACK ***
                case LOG:
-                       //if ( in1 <= 0 )
-                       //      throw new 
DMLRuntimeException("Builtin.execute(): logarithm can be computed only for 
non-negative numbers.");
-                       if( FASTMATH )
-                               return (FastMath.log(in1)/FastMath.log(in2)); 
-                       else
-                               return (Math.log(in1)/Math.log(in2)); 
+                       //faster in Math
+                       return (Math.log(in1)/Math.log(in2)); 
                case LOG_NZ:
-                       if( FASTMATH )
-                               return (in1==0) ? 0 : 
(FastMath.log(in1)/FastMath.log(in2)); 
-                       else
-                               return (in1==0) ? 0 : 
(Math.log(in1)/Math.log(in2)); 
+                       //faster in Math
+                       return (in1==0) ? 0 : (Math.log(in1)/Math.log(in2)); 
                
                        
                default:
@@ -436,17 +430,11 @@ public class Builtin extends ValueFunction
                case MININDEX: return (in1 <= in2) ? 1 : 0;
                
                case LOG:
-                       //if ( in1 <= 0 )
-                       //      throw new 
DMLRuntimeException("Builtin.execute(): logarithm can be computed only for 
non-negative numbers.");
-                       if( FASTMATH )
-                               return (FastMath.log(in1)/FastMath.log(in2));
-                       else
-                               return (Math.log(in1)/Math.log(in2));
+                       //faster in Math
+                       return Math.log(in1)/Math.log(in2);
                case LOG_NZ:
-                       if( FASTMATH )
-                               return (in1==0) ? 0 : 
(FastMath.log(in1)/FastMath.log(in2)); 
-                       else
-                               return (in1==0) ? 0 : 
(Math.log(in1)/Math.log(in2)); 
+                       //faster in Math
+                       return (in1==0) ? 0 : Math.log(in1)/Math.log(in2);
                
                                
                

http://git-wip-us.apache.org/repos/asf/systemml/blob/137fbf18/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java
----------------------------------------------------------------------
diff --git 
a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java 
b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java
index fd9742e..116c8a8 100644
--- a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java
+++ b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java
@@ -2684,7 +2684,7 @@ public class LibMatrixMult
                                                double wij = w[ix+j];
                                                if( wij != 0 ) {
                                                        double uvij = 
dotProduct(u, v, uix, vix, cd);
-                                                       wceval += wij * 
FastMath.log(uvij + eps);
+                                                       wceval += wij * 
Math.log(uvij + eps);
                                                }
                                        }
                }
@@ -2723,7 +2723,7 @@ public class LibMatrixMult
                                        int k = wpos + curk[i-bi];
                                        for( ; k<wpos+wlen && wix[k]<bjmin; k++ 
) {
                                                double uvij = dotProduct(u, v, 
uix, wix[k]*cd, cd);
-                                               wceval += wval[k] * 
FastMath.log(uvij + eps);   
+                                               wceval += wval[k] * 
Math.log(uvij + eps);
                                        }
                                        curk[i-bi] = k - wpos;
                                }
@@ -2752,7 +2752,7 @@ public class LibMatrixMult
                                        double[] wval = w.values(i);
                                        for( int k=wpos; k<wpos+wlen; k++ ) {
                                                double uvij = 
dotProductGeneric(mU, mV, i, wix[k], cd);
-                                               wceval += wval[k] * 
FastMath.log(uvij + eps);   
+                                               wceval += wval[k] * 
Math.log(uvij + eps);
                                        }
                                }       
                }
@@ -2765,7 +2765,7 @@ public class LibMatrixMult
                                        double wij = w[ix];
                                        if( wij != 0 ) {
                                                double uvij = 
dotProductGeneric(mU, mV, i, j, cd);
-                                               wceval += wij * 
FastMath.log(uvij + eps);       
+                                               wceval += wij * Math.log(uvij + 
eps);
                                        }
                                }
                }
@@ -3322,7 +3322,7 @@ public class LibMatrixMult
                                1 / (1 + FastMath.exp(-uvij));
                                
                //compute weighted output
-               return wij * ((flaglog) ? FastMath.log(cval) : cval);
+               return wij * ((flaglog) ? Math.log(cval) : cval);
        }
 
        private static double wsigmoid( final double wij, MatrixBlock u, 
MatrixBlock v, final int uix, final int vix, final boolean flagminus, final 
boolean flaglog, final int len )
@@ -3336,7 +3336,7 @@ public class LibMatrixMult
                                1 / (1 + FastMath.exp(-uvij));
                                
                //compute weighted output
-               return wij * ((flaglog) ? FastMath.log(cval) : cval);
+               return wij * ((flaglog) ? Math.log(cval) : cval);
        }
 
        private static void wdivmm( final double wij, double[] u, double[] v, 
double[] c, final int uix, final int vix, final boolean left, final boolean 
mult, final boolean minus, final int len )

Reply via email to