Move logic calculating the index in Vector.at to a builtin method to make the performance of Vector to be on par with Array

CHANGELOG.md

@@ -395,6 +395,7 @@
 - [By-type pattern matching][3742]
 - [Fix performance of method calls on polyglot arrays][3781]
 - [Missing foreign language generates proper Enso error][3798]
+- [Made Vector performance to be on par with Array][3811]
 
 [3227]: https://github.com/enso-org/enso/pull/3227
 [3248]: https://github.com/enso-org/enso/pull/3248
@@ -449,6 +450,7 @@
 [3742]: https://github.com/enso-org/enso/pull/3742
 [3781]: https://github.com/enso-org/enso/pull/3781
 [3798]: https://github.com/enso-org/enso/pull/3798
+[3811]: https://github.com/enso-org/enso/pull/3811
 
 # Enso 2.0.0-alpha.18 (2021-10-12)
 

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Array.enso

@@ -7,20 +7,23 @@ import Standard.Base.Meta
 ## The type of primitive mutable arrays.
 @Builtin_Type
 type Array
-    ## Gets the element at index in the array this.
+    ## Gets an element from the array at a specified index (0-based).
 
        Arguments:
-       - index: The index to get the element from.
-
-       ? Safety
-         If index < 0 or index >= self.length, then this operation will result
-         in an Invalid_Array_Index_Error exception.
+       - index: The index to get the element from. The index is
+         also allowed be negative, then the elements are indexed from the back
+         of the array, i.e. -1 will correspond to the last element.
 
        > Example
          Get the element at index 1.
 
              [1,2,3].to_array.at 1
-    at : Integer -> Any
+
+       > Example
+         Get the last element of an array.
+
+             [1,2,3].to_array.at -1
+    at : Integer -> Any ! Index_Out_Of_Bounds_Error
     at self index = @Builtin_Method "Array.at"
 
     ## Gets the length of the array this.

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Vector.enso

@@ -146,20 +146,7 @@ type Vector a
 
              [1, 2, 3].at -1 == 3
     at : Integer -> Any ! Index_Out_Of_Bounds_Error
-    at self index =
-        actual_index = if index < 0 then self.length + index else index
-        Panic.catch Invalid_Array_Index_Error_Data (self.unsafe_at actual_index) _->
-            Error.throw (Index_Out_Of_Bounds_Error_Data index self.length)
-
-    ## ADVANCED
-       UNSTABLE
-
-       An unsafe variant of the `at` operation. It only allows non-negative
-       indices and will panic with a raw Java exception on out-of-bounds access.
-       Thus it should only be used when the access is guaranteed to be within
-       bounds or with additional error handling.
-    unsafe_at : Integer -> Any
-    unsafe_at self index = @Builtin_Method "Vector.unsafe_at"
+    at self index = @Builtin_Method "Vector.at"
 
     ## Combines all the elements of the vector, by iteratively applying the
        passed function with next elements of the vector.
@@ -179,7 +166,7 @@ type Vector a
              [0, 1, 2] . fold 0 (+)
     fold : Any -> (Any -> Any -> Any) -> Any
     fold self init function =
-        f = acc -> ix -> function acc (self.unsafe_at ix)
+        f = acc -> ix -> function acc (self.at ix)
         0.up_to self.length . fold init f
 
     ## Combines all the elements of the vector, by iteratively applying the
@@ -213,9 +200,9 @@ type Vector a
     reduce : (Any -> Any -> Any) -> Any ! Empty_Error
     reduce self function =
         case self.not_empty of
-            True -> if self.length == 1 then self.unsafe_at 0 else
+            True -> if self.length == 1 then self.at 0 else
                 f = acc -> ix -> function acc (self.at ix)
-                1.up_to self.length . fold (self.unsafe_at 0) f
+                1.up_to self.length . fold (self.at 0) f
             False -> Error.throw Empty_Error
 
     ## Computes the sum of the values in the vector.
@@ -248,7 +235,7 @@ type Vector a
              [1, 2, 3, 4, 5].exists (> 3)
     exists : (Any -> Boolean) -> Boolean
     exists self predicate =
-        0.up_to self.length . exists (idx -> (predicate (self.unsafe_at idx)))
+        0.up_to self.length . exists (idx -> (predicate (self.at idx)))
 
     ## Returns the first element of the vector that satisfies the predicate or
        if no elements of the vector satisfy the predicate, it throws nothing.
@@ -267,7 +254,7 @@ type Vector a
         len = self.length
         go idx =
             if (idx >= len) then Error.throw Nothing else
-                elem = self.unsafe_at idx
+                elem = self.at idx
                 if predicate elem then elem else
                     @Tail_Call go idx+1
         go 0
@@ -435,7 +422,7 @@ type Vector a
              [1, 2, 3] . map +1
     map : (Any -> Any) -> Vector Any
     map self function =
-        new self.length i-> function (self.unsafe_at i)
+        new self.length i-> function (self.at i)
 
     ## Applies a function to each element of the vector, returning the vector
        that contains all results concatenated.
@@ -483,7 +470,7 @@ type Vector a
 
              [1, 2, 3].map_with_index (+)
     map_with_index : (Integer -> Any -> Any) -> Vector Any
-    map_with_index self function = new self.length i-> function i (self.unsafe_at i)
+    map_with_index self function = new self.length i-> function i (self.at i)
 
     ## Applies a function to each element of the vector.
 
@@ -500,7 +487,7 @@ type Vector a
     each : (Any -> Any) -> Nothing
     each self f =
         0.up_to self.length . each ix->
-            f (self.unsafe_at ix)
+            f (self.at ix)
 
     ## Applies a function to each element of the vector.
 
@@ -520,7 +507,7 @@ type Vector a
     each_with_index : (Integer -> Any -> Any) -> Nothing
     each_with_index self f =
         0.up_to self.length . each ix->
-            f ix (self.unsafe_at ix)
+            f ix (self.at ix)
 
     ## Reverses the vector, returning a vector with the same elements, but in
        the opposite order.
@@ -530,7 +517,7 @@ type Vector a
 
              [1, 2].reverse
     reverse : Vector Any
-    reverse self = new self.length (i -> self.unsafe_at (self.length - (1 + i)))
+    reverse self = new self.length (i -> self.at (self.length - (1 + i)))
 
     ## Generates a human-readable text representation of the vector.
 
@@ -579,7 +566,7 @@ type Vector a
     == : Vector -> Boolean
     == self that = case that of
         _ : Vector ->
-            eq_at i = self.unsafe_at i == that.unsafe_at i
+            eq_at i = self.at i == that.at i
             if self.length == that.length then 0.up_to self.length . all eq_at else False
         _ : Array ->
             eq_at i = self.at i == that.at i
@@ -643,8 +630,8 @@ type Vector a
     join : Text -> Text -> Text -> Text
     join self separator="" prefix="" suffix="" =
         if self.is_empty then prefix+suffix else
-            if self.length == 1 then prefix + self.unsafe_at 0 + suffix else
-                prefix + self.unsafe_at 0 + (1.up_to self.length . fold "" acc-> i-> acc + separator + self.unsafe_at i) + suffix
+            if self.length == 1 then prefix + self.at 0 + suffix else
+                prefix + self.at 0 + (1.up_to self.length . fold "" acc-> i-> acc + separator + self.at i) + suffix
 
     ## PRIVATE
        Creates a new vector with the skipping elements until `start` and then
@@ -716,7 +703,7 @@ type Vector a
     zip : Vector Any -> (Any -> Any -> Any) -> Vector Any
     zip self that function=[_,_] =
         len = Math.min self.length that.length
-        new len i-> function (self.unsafe_at i) (that.unsafe_at i)
+        new len i-> function (self.at i) (that.at i)
 
     ## Extend `self` vector to the length of `n` appending elements `elem` to
        the end.
@@ -759,7 +746,7 @@ type Vector a
 
              [1, 2, 3, 4].head
     head : Any ! Empty_Error
-    head self = if self.length >= 1 then self.unsafe_at 0 else Error.throw Empty_Error
+    head self = if self.length >= 1 then self.at 0 else Error.throw Empty_Error
 
     ## Get all elements in the vector except the first.
 
@@ -788,7 +775,7 @@ type Vector a
 
              [1, 2, 3, 4].last
     last : Vector ! Empty_Error
-    last self = if self.length >= 1 then self.unsafe_at (self.length-1) else
+    last self = if self.length >= 1 then self.at (self.length-1) else
         Error.throw Empty_Error
 
     ## Get the first element from the vector, or an `Empty_Error` if the vector
@@ -1133,7 +1120,7 @@ slice_ranges vector ranges =
     if ranges.length == 0 then [] else
         if ranges.length != 1 then slice_many_ranges vector ranges else
             case ranges.first of
-                _ : Integer -> [vector.unsafe_at ranges.first]
+                _ : Integer -> [vector.at ranges.first]
                 Range_Data start end step -> case step == 1 of
                     True -> vector.slice start end
                     False -> slice_many_ranges vector ranges
@@ -1147,11 +1134,11 @@ slice_many_ranges vector ranges =
     builder = new_builder new_length
     ranges.each descriptor-> case descriptor of
         _ : Integer ->
-            builder.append (vector.unsafe_at descriptor)
+            builder.append (vector.at descriptor)
         Range_Data start end step -> case step == 1 of
             True ->
                 builder.append_vector_range vector start end
             False ->
                 descriptor.each ix->
-                    builder.append (vector.unsafe_at ix)
+                    builder.append (vector.at ix)
     builder.to_vector

distribution/lib/Standard/Base/0.0.0-dev/src/Error/Common.enso

@@ -218,6 +218,7 @@ from project.Error.Common.Index_Out_Of_Bounds_Error export all
    Arguments:
    - index: The requested index.
    - length: The length of the collection.
+@Builtin_Type
 type Index_Out_Of_Bounds_Error
     Index_Out_Of_Bounds_Error_Data index length
 

docs/runtime-guide.md

@@ -94,6 +94,29 @@ order: 7
    `truffle-api`, you've done something wrong. Similarly, if you ever import
    anything from `org.graalvm.polyglot` in the language code, you're doing
    something wrong.
+10. Avoid
+    [deoptimizations](https://www.graalvm.org/22.2/graalvm-as-a-platform/language-implementation-framework/Optimizing/#debugging-deoptimizations).
+    Understanding IGV graphs can be a very time-consuming and complex process.
+    Sometimes it is sufficient to only look at the compilation traces to
+    discover repeated or unnecessary deoptimizations which can significantly
+    affect overall performance of your program. You can tell runner to generate
+    compilation traces via additional options:
+    ```
+    JAVA_OPTS="-Dpolygot.engine.TracePerformanceWarnings=all -Dpolyglot.engine.TraceTransferToInterpreter=true -Dpolyglot.engine.TraceDeoptimizeFrame=true -Dpolyglot.engine.TraceCompilation=true -Dpolyglot.engine.TraceCompilationDetails=true"
+    ```
+    Make sure you print trace logs by using `--log-level TRACE`.
+11. Occasionally a piece of code runs slower than we anticipated. Analyzing
+    Truffle inlining traces may reveal locations that one thought would be
+    inlined but Truffle decided otherwise. Rewriting such locations to builtin
+    methods or more inliner-friendly representation can significantly improve
+    the performance. You can tell runner to generate inlining traces via
+    additional options:
+    ```
+    JAVA_OPTS="-Dpolyglot.engine.TraceInlining=true -Dpolyglot.engine.TraceInliningDetails=true"
+    ```
+    Make sure you print trace logs by using `--log-level TRACE`. See
+    [documentation](https://www.graalvm.org/22.2/graalvm-as-a-platform/language-implementation-framework/Inlining/#call-tree-states)
+    for the explanation of inlining decisions.
 
 ## Code & Internal Documentation Map
 

engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/error/IndexOutOfBoundsError.java

@@ -0,0 +1,14 @@
+package org.enso.interpreter.node.expression.builtin.error;
+
+import org.enso.interpreter.dsl.BuiltinType;
+import org.enso.interpreter.node.expression.builtin.UniquelyConstructibleBuiltin;
+
+import java.util.List;
+
+@BuiltinType
+public class IndexOutOfBoundsError extends UniquelyConstructibleBuiltin {
+  @Override
+  protected List<String> getConstructorParamNames() {
+    return List.of("index", "length");
+  }
+}

engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/immutable/UnsafeAtVectorNode.java → engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/immutable/AtVectorNode.java

@@ -1,5 +1,6 @@
 package org.enso.interpreter.node.expression.builtin.immutable;
 
+import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.interop.InteropLibrary;
 import com.oracle.truffle.api.interop.InvalidArrayIndexException;
 import com.oracle.truffle.api.interop.UnsupportedMessageException;
@@ -8,25 +9,33 @@
 import org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode;
 import org.enso.interpreter.runtime.Context;
 import org.enso.interpreter.runtime.data.Vector;
-import org.enso.interpreter.runtime.error.PanicException;
+import org.enso.interpreter.runtime.error.DataflowError;
 
 @BuiltinMethod(
     type = "Vector",
-    name = "unsafe_at",
+    name = "at",
     description = "Returns an element of Vector at the specified index.")
-public class UnsafeAtVectorNode extends Node {
+public class AtVectorNode extends Node {
   private @Child InteropLibrary interop = InteropLibrary.getFactory().createDispatched(3);
-  private @Child HostValueToEnsoNode toEnso = HostValueToEnsoNode.build();
+  private @Child HostValueToEnsoNode convert = HostValueToEnsoNode.build();
 
   Object execute(Vector self, long index) {
     try {
-      return self.readArrayElement(index, interop, toEnso);
+      return readElement(self, index);
+    } catch (UnsupportedMessageException e) {
+      CompilerDirectives.transferToInterpreter();
+      throw new IllegalStateException(e);
+    }
+  }
+
+  private Object readElement(Vector self, long index) throws UnsupportedMessageException {
+    try {
+      long actualIndex = index < 0 ? index + self.length(interop) : index;
+      return self.readArrayElement(actualIndex, interop, convert);
     } catch (InvalidArrayIndexException e) {
       Context ctx = Context.get(this);
-      throw new PanicException(
-          ctx.getBuiltins().error().makeInvalidArrayIndexError(self, index), this);
-    } catch (UnsupportedMessageException e) {
-      throw new PanicException(e.getMessage(), this);
+      return DataflowError.withoutTrace(
+          ctx.getBuiltins().error().makeIndexOutOfBoundsError(index, self.length(interop)), this);
     }
   }
 }

engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/mutable/ArrayAtNode.java

@@ -8,25 +8,31 @@
 import org.enso.interpreter.dsl.BuiltinMethod;
 import org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode;
 import org.enso.interpreter.runtime.Context;
-import org.enso.interpreter.runtime.builtin.Builtins;
-import org.enso.interpreter.runtime.error.PanicException;
+import org.enso.interpreter.runtime.error.DataflowError;
 
 @BuiltinMethod(type = "Array", name = "at", description = "Get element of a polyglot array")
 public class ArrayAtNode extends Node {
   @Child InteropLibrary iop = InteropLibrary.getFactory().createDispatched(3);
   @Child HostValueToEnsoNode convert = HostValueToEnsoNode.build();
 
-  Object execute(Object self, long at) {
+  Object execute(Object self, long index) {
     try {
-      var r = iop.readArrayElement(self, at);
-      return convert.execute(r);
+      return readElement(self, index);
     } catch (UnsupportedMessageException ex) {
       CompilerDirectives.transferToInterpreter();
       throw new IllegalStateException(ex);
-    } catch (InvalidArrayIndexException ex) {
+    }
+  }
+
+  private Object readElement(Object self, long index) throws UnsupportedMessageException {
+    try {
+      long actualIndex = index < 0 ? index + iop.getArraySize(self) : index;
+      var element = iop.readArrayElement(self, actualIndex);
+      return convert.execute(element);
+    } catch (InvalidArrayIndexException e) {
       Context ctx = Context.get(this);
-      Builtins builtins = ctx.getBuiltins();
-      throw new PanicException(builtins.error().makeInvalidArrayIndexError(self, at), this);
+      return DataflowError.withoutTrace(
+          ctx.getBuiltins().error().makeIndexOutOfBoundsError(index, iop.getArraySize(self)), this);
     }
   }
 }

engine/runtime/src/main/java/org/enso/interpreter/runtime/builtin/Error.java

@@ -20,6 +20,7 @@ public class Error {
   private final SyntaxError syntaxError;
   private final TypeError typeError;
   private final CompileError compileError;
+  private final IndexOutOfBoundsError indexOutOfBoundsError;
   private final InexhaustivePatternMatchError inexhaustivePatternMatchError;
   private final UninitializedState uninitializedState;
   private final NoSuchMethodError noSuchMethodError;
@@ -50,6 +51,7 @@ public Error(Builtins builtins, Context context) {
     syntaxError = builtins.getBuiltinType(SyntaxError.class);
     typeError = builtins.getBuiltinType(TypeError.class);
     compileError = builtins.getBuiltinType(CompileError.class);
+    indexOutOfBoundsError = builtins.getBuiltinType(IndexOutOfBoundsError.class);
     inexhaustivePatternMatchError = builtins.getBuiltinType(InexhaustivePatternMatchError.class);
     uninitializedState = builtins.getBuiltinType(UninitializedState.class);
     noSuchMethodError = builtins.getBuiltinType(NoSuchMethodError.class);
@@ -76,6 +78,10 @@ public Atom makeCompileError(Object message) {
     return compileError.newInstance(message);
   }
 
+  public Atom makeIndexOutOfBoundsError(long index, long length) {
+    return indexOutOfBoundsError.newInstance(index, length);
+  }
+
   public Atom makeInexhaustivePatternMatchError(Object message) {
     return inexhaustivePatternMatchError.newInstance(message);
   }