enso-org / enso

Enso Analytics is a self-service data prep and analysis platform designed for data teams.
https://ensoanalytics.com
Apache License 2.0
7.39k stars 323 forks source link

Investigate further performance improvements for `Vector.sort` #6361

Closed hubertp closed 5 months ago

hubertp commented 1 year ago

Discussion on https://github.com/enso-org/enso/pull/6334 (a fix for https://github.com/enso-org/enso/issues/6276) revealed some problems with the implementation of Vector.sort. While the performance is now acceptable, we could potentially do much more in terms of making it more Truffle-friendly.

Main issues:

diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortVectorNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortVectorNode.java
index d23ee524e..a277dd50a 100644
--- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortVectorNode.java
+++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortVectorNode.java
@@ -2,6 +2,7 @@ package org.enso.interpreter.node.expression.builtin.ordering;

 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
+import com.oracle.truffle.api.RootCallTarget;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.GenerateUncached;
 import com.oracle.truffle.api.dsl.Specialization;
@@ -806,7 +807,10 @@ public abstract class SortVectorNode extends Node {
       Object yConverted;
       if (hasCustomOnFunc) {
         // onFunc cannot have `self` argument, we assume it has just one argument.
-        xConverted = callNode.executeDispatch(onFunc.get(x), null, state, new Object[]{x});
+        var resolvedFunc = onFunc.get(x);
+        var callTarget = resolvedFunc.getCallTarget();
+        var args = Function.ArgumentsHelper.buildArguments(resolvedFunc, null, state, new Object[]{x});
+        xConverted = callTarget.call(args);
         yConverted = callNode.executeDispatch(onFunc.get(y), null, state, new Object[]{y});
       } else {
         xConverted = x;
### Tasks
- [x] Use `ArrayLikeAtNode` and `ArrayLikeLengthNode` instead of `InteropLibrary` when dealing with arrays - #7697
- [x] Rewrite function invocation to use `CallTarget`
- [x] Check the speedup
JaroslavTulach commented 6 months ago

I guess I should wait for

before starting the benchmarks.

JaroslavTulach commented 5 months ago

Looks like the sorting code is running fast already. Closing.

enso-bot[bot] commented 5 months ago

Jaroslav Tulach reports a new STANDUP for yesterday (2024-06-02):

Progress: - new Rational benchmark: https://github.com/enso-org/enso/pull/10142#issuecomment-2143321164