Skip to content

Commit

Permalink
Make ArrayOverBuffer behave like an Array
Browse files Browse the repository at this point in the history
Most of the problems with accessing `ArrayOverBuffer` have been resolved
by using `CoerceArrayNode` (#3817).
In `Array.sort` we still however specialized on Array which wasn't
compatible with `ArrayOverBuffer`.

Added a specialization to `Array.sort` to deal with that case. Because
one cannot use a custom comparator with primitive (Java) arrays there is
a conversion needed. It's a necessary penalty if we want to keep
`ArrayOverBuffer` around.

Also fixed an example in `Array.enso` by providing a default argument.
  • Loading branch information
hubertp committed Jan 9, 2023
1 parent 41b2aac commit 4bd3578
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 16 deletions.
14 changes: 8 additions & 6 deletions distribution/lib/Standard/Base/0.0.0-dev/src/Data/Array.enso
Original file line number Diff line number Diff line change
Expand Up @@ -134,19 +134,21 @@ type Array
length : Integer
length self = @Builtin_Method "Array.length"

## Sorts the this array in place.
## ADVANCED

Arguments:
- comparator: A comparison function that takes two elements and returns
an Ordering that describes how the first element is ordered with
respect to the second.
Sorts this array in place.

Arguments:
- comparator: A comparison function that takes two elements and returns
an Ordering that describes how the first element is ordered with
respect to the second.

> Example
Sorting an array of numbers.

[1,2,3].to_array.sort
sort : (Any -> Any -> Ordering) -> Nothing
sort self comparator = @Builtin_Method "Array.sort"
sort self comparator=(_.compare_to _) = self.sort_builtin comparator

## Identity.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,28 @@
import com.oracle.truffle.api.profiles.BranchProfile;
import java.util.Arrays;
import java.util.Comparator;

import org.apache.commons.lang3.ArrayUtils;
import org.enso.interpreter.dsl.BuiltinMethod;
import org.enso.interpreter.epb.node.CoercePrimitiveNode;
import org.enso.interpreter.node.callable.dispatch.CallOptimiserNode;
import org.enso.interpreter.node.callable.dispatch.SimpleCallOptimiserNode;
import org.enso.interpreter.runtime.EnsoContext;
import org.enso.interpreter.runtime.callable.atom.Atom;
import org.enso.interpreter.runtime.callable.function.Function;
import org.enso.interpreter.runtime.data.Array;
import org.enso.interpreter.runtime.data.ArrayOverBuffer;
import org.enso.interpreter.runtime.error.PanicException;
import org.enso.interpreter.runtime.state.State;

@BuiltinMethod(type = "Array", name = "sort", description = "Sorts a mutable array in place.")
@BuiltinMethod(
type = "Array",
name = "sort_builtin",
description = "Sorts a mutable array in place.")
public abstract class SortNode extends Node {
private @Child CallOptimiserNode callOptimiserNode = SimpleCallOptimiserNode.build();
private @Child InvalidComparisonNode invalidComparisonNode = InvalidComparisonNode.build();
private @Child CoercePrimitiveNode coercePrimitiveNode = CoercePrimitiveNode.build();
private final BranchProfile resultProfile = BranchProfile.create();

abstract Object execute(State state, Object self, Object comparator);
Expand All @@ -34,15 +42,22 @@ static SortNode build() {
@Specialization
Object doSortFunction(State state, Array self, Function comparator) {
EnsoContext context = EnsoContext.get(this);
Comparator<Object> compare = getComparator(comparator, context, state);
return runSort(compare, self, context);
Comparator<Object> compare = getComparator(comparator, context, state, false);
return runSort(compare, self.getItems(), context);
}

@Specialization
Object doAtomThis(State state, Atom self, Object that) {
return EnsoContext.get(this).getBuiltins().nothing();
}

@Specialization
Object doArrayOverBuffer(State state, ArrayOverBuffer self, Function comparator) {
EnsoContext context = EnsoContext.get(this);
Comparator<Object> compare = getComparator(comparator, context, state, true);
return runSort(compare, self.backingArray(), context);
}

@Fallback
Object doOther(State state, Object self, Object comparator) {
CompilerDirectives.transferToInterpreter();
Expand All @@ -52,19 +67,30 @@ Object doOther(State state, Object self, Object comparator) {
this);
}

Object runSort(Comparator<Object> compare, Array self, EnsoContext context) {
doSort(self.getItems(), compare);
LoopNode.reportLoopCount(this, (int) self.length());
Object runSort(Comparator<Object> compare, Object[] self, EnsoContext context) {
doSort(self, compare);
LoopNode.reportLoopCount(this, self.length);
return context.getBuiltins().nothing();
}

Object runSort(Comparator<Object> compare, byte[] self, EnsoContext context) {
Byte[] tmpArray = ArrayUtils.toObject(self);
Object result = runSort(compare, tmpArray, context);
byte[] primitiveTmpArray = ArrayUtils.toPrimitive(tmpArray);
System.arraycopy(primitiveTmpArray, 0, self, 0, self.length);
return result;
}

@TruffleBoundary
void doSort(Object[] items, Comparator<Object> compare) {
Arrays.sort(items, compare);
}

private SortComparator getComparator(Function comp, EnsoContext context, State state) {
return new SortComparator(comp, context, this, state);
private SortComparator getComparator(
Function comp, EnsoContext context, State state, boolean coerce) {
return coerce
? new CoerceSortComparator(comp, context, this, state)
: new SortComparator(comp, context, this, state);
}

private class SortComparator implements Comparator<Object> {
Expand Down Expand Up @@ -105,4 +131,16 @@ private int convertResult(Object res) {
}
}
}

private class CoerceSortComparator extends SortComparator {

CoerceSortComparator(Function compFn, EnsoContext context, SortNode outerThis, State state) {
super(compFn, context, outerThis, state);
}

@Override
public int compare(Object o1, Object o2) {
return super.compare(coercePrimitiveNode.execute(o1), coercePrimitiveNode.execute(o2));
}
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
package org.enso.interpreter.runtime.data;

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.TruffleObject;
import com.oracle.truffle.api.interop.UnsupportedMessageException;
import com.oracle.truffle.api.library.ExportLibrary;
import com.oracle.truffle.api.library.ExportMessage;
import org.enso.interpreter.node.expression.builtin.error.InvalidArrayIndex;
import org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode;
import org.enso.interpreter.runtime.error.WarningsLibrary;

import java.io.PrintWriter;
import java.io.StringWriter;
import java.nio.ByteBuffer;

@ExportLibrary(InteropLibrary.class)
Expand All @@ -16,8 +24,12 @@ private ArrayOverBuffer(ByteBuffer buffer) {
}

@ExportMessage
Object readArrayElement(long index) {
return (long) buffer.get(buffer.position() + Math.toIntExact(index));
Object readArrayElement(long index) throws InvalidArrayIndexException {
try {
return (long) buffer.get(buffer.position() + Math.toIntExact(index));
} catch (IndexOutOfBoundsException e) {
throw InvalidArrayIndexException.create(index);
}
}

@ExportMessage
Expand All @@ -38,4 +50,49 @@ long getArraySize() {
public static ArrayOverBuffer wrapBuffer(ByteBuffer buffer) {
return new ArrayOverBuffer(buffer);
}

@ExportMessage
String toDisplayString(boolean allowSideEffects) {
final InteropLibrary iop = InteropLibrary.getUncached();
StringBuilder sb = new StringBuilder();
try {
sb.append('[');
String sep = "";
long len = getArraySize();
for (long i = 0; i < len; i++) {
sb.append(sep);

Object at = readArrayElement(i);
Object str = showObject(iop, allowSideEffects, at);
if (iop.isString(str)) {
sb.append(iop.asString(str));
} else {
sb.append("_");
}
sep = ", ";
}
sb.append(']');
} catch (InvalidArrayIndexException | UnsupportedMessageException ex) {
StringWriter w = new StringWriter();
ex.printStackTrace(new PrintWriter(w));
sb.append("...\n").append(w.toString());
}
return sb.toString();
}

@CompilerDirectives.TruffleBoundary
private Object showObject(InteropLibrary iop, boolean allowSideEffects, Object child)
throws UnsupportedMessageException {
if (child == null) {
return "null";
} else if (child instanceof Boolean) {
return (boolean) child ? "True" : "False";
} else {
return iop.toDisplayString(child, allowSideEffects);
}
}

public byte[] backingArray() {
return buffer.array();
}
}
18 changes: 18 additions & 0 deletions test/Tests/src/Data/Array_Spec.enso
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,22 @@ spec =
res = Array.new err
res . should_fail_with Illegal_State.Error

Test.specify "should sort in place" <|
arr = make_enso_array [3, 1, 2]
arr.sort
arr . should_equal [1, 2, 3]

Test.group "ArrayOverBuffer" <|
Test.specify "should behave like an Array" <|
array_over_buffer = (File.new (enso_project.data / "sample.txt") . read_last_bytes 10).to_array

case array_over_buffer of
_ : Array -> Nothing
_ -> Test.fail "Expected ArrayOverBuffer to match on Array type"

array_over_buffer.to_text . should_equal "[32, 106, 117, 106, 117, 98, 101, 115, 46, 10]"
array_over_buffer.sort
array_over_buffer.to_text . should_equal "[10, 32, 46, 98, 101, 106, 106, 115, 117, 117]"


main = Test_Suite.run_main spec

0 comments on commit 4bd3578

Please sign in to comment.