Numbers.sqrt & co. are slow - benchmark first

[JaroslavTulach]

While playing with IGV to analyze performance of sieve.enso I realized that calling sqrt slows the program by 20%. Simple fix was able to speed the execution from 550ms to 450ms.

The slow down seems to be caused by Java interop. Calling java.lang.Math.sqrt requires a lot of nodes and is far from a simple Math.sqrt invocation. To gain the speed we could rewrite sqrt (and other operations on Numbers) to invoke a builtin.

[Akirathan]

I remember that FastR faced the very same dilemma some time ago, when the developers realized that calling native functions (many math and statics related, like sqrt), from Truffle was too slow, so they ended up reimplementing a lot of these functions in Java. I guess that the overhead of calling a host method as opposed to a native function (via NFI) is different, but comparable.

[JaroslavTulach]

It'd be ideal to have this support working without much of boilerplate code. Something like:

diff --git distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso
index fa6c46b4dd..9fc7478457 100644
--- distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso
+++ distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso
@@ -246,7 +246,8 @@ type Number
 
              8.sqrt
     sqrt : Decimal
-    sqrt self = Math.sqrt self.to_decimal
+    # sqrt self = Math.sqrt self.to_decimal
+    sqrt self = @Builtin_Method "Integer.sqrt"
 
     ## ALIAS Logarithm
 
diff --git engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/number/Integer.java engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/number/Integer.java
index 54fabd914b..58a01cf77b 100644
--- engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/number/Integer.java
+++ engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/number/Integer.java
@@ -14,4 +14,10 @@ public class Integer extends Builtin {
   protected boolean containsValues() {
     return true;
   }
+  
+  
+    @org.enso.interpreter.dsl.Builtin.Method(name = "sqrt")
+    public static double sqrt(double d) {
+        return Math.sqrt(d);
+    }
 }

would be great - but it is not working right now - Methodsqrt of 3 (Integer) could not be found. - I must be doing something wrongly. Can you comment on the approach, @hubertp (feel free to replace my diff with better one)? Btw. the original code was doing .to_decimal - e.g. manual conversion to double. It'd be great to get such a conversion with @Builtin.Method automatically as well.

[JaroslavTulach]

Not sure how relevant this issue still is. Maybe a new measurement and closing the issue is all we need.

[JaroslavTulach]

I have modified a benchmark to measure sqrt speed with following diff:

diff --git distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso
index be15943666..35e8d600bb 100644
--- distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso
+++ distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso
@@ -289,7 +289,8 @@ type Number
 
              8.sqrt
     sqrt : Float
-    sqrt self = Math.sqrt self.to_float
+    sqrt self = @Builtin_Method "Integer.sqrt"
+    #sqrt self = Math.sqrt self.to_float
 
     ## ALIAS logarithm
        GROUP Math
diff --git engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/ArrayProxyBenchmarks.java engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/ArrayProxyBenchmarks.java
index 1327a37cee..4c922c97e5 100644
--- engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/ArrayProxyBenchmarks.java
+++ engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/ArrayProxyBenchmarks.java
@@ -2,6 +2,7 @@ package org.enso.interpreter.bench.benchmarks.semantic;
 
 import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
+
 import org.enso.compiler.benchmarks.Utils;
 import org.graalvm.polyglot.Value;
 import org.openjdk.jmh.annotations.Benchmark;
@@ -44,7 +45,7 @@ public class ArrayProxyBenchmarks {
         import Standard.Base.Data.Array_Proxy.Array_Proxy
         sum arr =
             go acc i = if i >= arr.length then acc else
-                @Tail_Call go (acc + arr.at i) i+1
+                @Tail_Call go (acc + (arr.at i . sqrt)) i+1
             go 0 0
 
         make_vector n =
@@ -116,15 +117,18 @@ public class ArrayProxyBenchmarks {
 
   private void performBenchmark(Blackhole matter) throws AssertionError {
     var resultValue = sum.execute(self, arrayOfNumbers);
-    if (!resultValue.fitsInLong()) {
-      throw new AssertionError("Shall be a long: " + resultValue);
+    if (!resultValue.fitsInDouble()) {
+      throw new AssertionError("Shall be a double: " + resultValue);
     }
+    double result = resultValue.asDouble();
+    /*
     long result = resultValue.asLong();
     long expectedResult = length * 3L + (5L * (length * (length - 1L) / 2L));
     boolean isResultCorrect = result == expectedResult;
     if (!isResultCorrect) {
       throw new AssertionError("Expecting " + expectedResult + " but was " + result);
     }
+    */
     matter.consume(result);
   }
 }
diff --git engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/number/Integer.java engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/number/Integer.java
index 296f7521dc..7afce687bc 100644
--- engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/number/Integer.java
+++ engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/number/Integer.java
@@ -2,6 +2,7 @@ package org.enso.interpreter.node.expression.builtin.number;
 
 import org.enso.interpreter.dsl.BuiltinType;
 import org.enso.interpreter.node.expression.builtin.Builtin;
+import org.enso.interpreter.node.expression.builtin.number.Number;
 
 @BuiltinType(name = "Standard.Base.Data.Numbers.Integer")
 public class Integer extends Builtin {
@@ -14,4 +15,10 @@ public class Integer extends Builtin {
   public boolean containsValues() {
     return true;
   }
+
+
+    @org.enso.interpreter.dsl.Builtin.Method(name = "sqrt")
+    public static double self(Object self) {
+        return Math.sqrt(((java.lang.Number)self).doubleValue());
+    }
 }

There is no difference between builtin version and polyglot java version of sqrt. Both run in 0.211 ms/op.