Skip to content

Commit

Permalink
[Truffle] Use ArrayUtils.extractRange for other array types as well.
Browse files Browse the repository at this point in the history
@eregon
eregon committed Jan 13, 2015
1 parent 2247235 commit 2f08382
Showing 5 changed files with 93 additions and 15 deletions.
10 changes: 6 additions & 4 deletions core/src/main/java/org/jruby/truffle/nodes/core/ArrayDropTailNode.java
View file
Original file line number Diff line number Diff line change
@@ -14,9 +14,11 @@
import com.oracle.truffle.api.dsl.NodeChildren;
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.source.SourceSection;

import org.jruby.truffle.nodes.RubyNode;
import org.jruby.truffle.runtime.RubyContext;
import org.jruby.truffle.runtime.core.RubyArray;
import org.jruby.truffle.runtime.util.ArrayUtils;

import java.util.Arrays;

@@ -50,7 +52,7 @@ public RubyArray getHeadIntegerFixnum(RubyArray array) {
if (index >= array.getSize()) {
return new RubyArray(getContext().getCoreLibrary().getArrayClass());
} else {
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), Arrays.copyOfRange((int[]) array.getStore(), 0, array.getSize() - index), array.getSize() - index);
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), ArrayUtils.extractRange((int[]) array.getStore(), 0, array.getSize() - index), array.getSize() - index);
}
}

@@ -62,7 +64,7 @@ public RubyArray geHeadLongFixnum(RubyArray array) {
return new RubyArray(getContext().getCoreLibrary().getArrayClass());
} else {
final int size = array.getSize() - index;
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), Arrays.copyOfRange((long[]) array.getStore(), 0, size), size);
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), ArrayUtils.extractRange((long[]) array.getStore(), 0, size), size);
}
}

@@ -74,7 +76,7 @@ public RubyArray getHeadFloat(RubyArray array) {
return new RubyArray(getContext().getCoreLibrary().getArrayClass());
} else {
final int size = array.getSize() - index;
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), Arrays.copyOfRange((double[]) array.getStore(), 0, size), size);
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), ArrayUtils.extractRange((double[]) array.getStore(), 0, size), size);
}
}

@@ -86,7 +88,7 @@ public RubyArray getHeadObject(RubyArray array) {
return new RubyArray(getContext().getCoreLibrary().getArrayClass());
} else {
final int size = array.getSize() - index;
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), Arrays.copyOfRange((Object[]) array.getStore(), 0, size), size);
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), ArrayUtils.extractRange((Object[]) array.getStore(), 0, size), size);
}
}

10 changes: 6 additions & 4 deletions core/src/main/java/org/jruby/truffle/nodes/core/ArrayGetTailNode.java
View file
Original file line number Diff line number Diff line change
@@ -14,9 +14,11 @@
import com.oracle.truffle.api.dsl.NodeChildren;
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.source.SourceSection;

import org.jruby.truffle.nodes.RubyNode;
import org.jruby.truffle.runtime.RubyContext;
import org.jruby.truffle.runtime.core.RubyArray;
import org.jruby.truffle.runtime.util.ArrayUtils;

import java.util.Arrays;

@@ -50,7 +52,7 @@ public RubyArray getTailIntegerFixnum(RubyArray array) {
if (index >= array.getSize()) {
return new RubyArray(getContext().getCoreLibrary().getArrayClass());
} else {
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), Arrays.copyOfRange((int[]) array.getStore(), index, array.getSize()), array.getSize() - index);
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), ArrayUtils.extractRange((int[]) array.getStore(), index, array.getSize()), array.getSize() - index);
}
}

@@ -61,7 +63,7 @@ public RubyArray getTailLongFixnum(RubyArray array) {
if (index >= array.getSize()) {
return new RubyArray(getContext().getCoreLibrary().getArrayClass());
} else {
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), Arrays.copyOfRange((long[]) array.getStore(), index, array.getSize()), array.getSize() - index);
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), ArrayUtils.extractRange((long[]) array.getStore(), index, array.getSize()), array.getSize() - index);
}
}

@@ -72,7 +74,7 @@ public RubyArray getTailFloat(RubyArray array) {
if (index >= array.getSize()) {
return new RubyArray(getContext().getCoreLibrary().getArrayClass());
} else {
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), Arrays.copyOfRange((double[]) array.getStore(), index, array.getSize()), array.getSize() - index);
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), ArrayUtils.extractRange((double[]) array.getStore(), index, array.getSize()), array.getSize() - index);
}
}

@@ -83,7 +85,7 @@ public RubyArray getTailObject(RubyArray array) {
if (index >= array.getSize()) {
return new RubyArray(getContext().getCoreLibrary().getArrayClass());
} else {
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), Arrays.copyOfRange((Object[]) array.getStore(), index, array.getSize()), array.getSize() - index);
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), ArrayUtils.extractRange((Object[]) array.getStore(), index, array.getSize()), array.getSize() - index);
}
}

6 changes: 3 additions & 3 deletions core/src/main/java/org/jruby/truffle/nodes/core/ArrayNodes.java
View file
Original file line number Diff line number Diff line change
@@ -672,7 +672,7 @@ public Object sliceIntegerFixnum(RubyArray array, int start, int length) {
} else {
final int end = Math.min(array.getSize(), normalisedIndex + length);

return new RubyArray(getContext().getCoreLibrary().getArrayClass(), Arrays.copyOfRange((int[]) array.getStore(), normalisedIndex, end), end - normalisedIndex);
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), ArrayUtils.extractRange((int[]) array.getStore(), normalisedIndex, end), end - normalisedIndex);
}
}

@@ -689,7 +689,7 @@ public Object sliceLongFixnum(RubyArray array, int start, int length) {
} else {
final int end = Math.min(array.getSize(), normalisedIndex + length);

return new RubyArray(getContext().getCoreLibrary().getArrayClass(), Arrays.copyOfRange((long[]) array.getStore(), normalisedIndex, end), end - normalisedIndex);
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), ArrayUtils.extractRange((long[]) array.getStore(), normalisedIndex, end), end - normalisedIndex);
}
}

@@ -706,7 +706,7 @@ public Object sliceFloat(RubyArray array, int start, int length) {
} else {
final int end = Math.min(array.getSize(), normalisedIndex + length);

return new RubyArray(getContext().getCoreLibrary().getArrayClass(), Arrays.copyOfRange((double[]) array.getStore(), normalisedIndex, end), end - normalisedIndex);
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), ArrayUtils.extractRange((double[]) array.getStore(), normalisedIndex, end), end - normalisedIndex);
}
}

10 changes: 6 additions & 4 deletions core/src/main/java/org/jruby/truffle/nodes/core/ArraySliceNode.java
View file
Original file line number Diff line number Diff line change
@@ -14,9 +14,11 @@
import com.oracle.truffle.api.dsl.NodeChildren;
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.source.SourceSection;

import org.jruby.truffle.nodes.RubyNode;
import org.jruby.truffle.runtime.RubyContext;
import org.jruby.truffle.runtime.core.RubyArray;
import org.jruby.truffle.runtime.util.ArrayUtils;

import java.util.Arrays;

@@ -56,7 +58,7 @@ public RubyArray sliceIntegerFixnum(RubyArray array) {
if (from >= to) {
return new RubyArray(getContext().getCoreLibrary().getArrayClass());
} else {
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), Arrays.copyOfRange((int[]) array.getStore(), from, to), to - from);
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), ArrayUtils.extractRange((int[]) array.getStore(), from, to), to - from);
}
}

@@ -68,7 +70,7 @@ public RubyArray sliceLongFixnum(RubyArray array) {
if (from >= to) {
return new RubyArray(getContext().getCoreLibrary().getArrayClass());
} else {
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), Arrays.copyOfRange((long[]) array.getStore(), from, to), to - from);
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), ArrayUtils.extractRange((long[]) array.getStore(), from, to), to - from);
}
}

@@ -80,7 +82,7 @@ public RubyArray sliceFloat(RubyArray array) {
if (from >= to) {
return new RubyArray(getContext().getCoreLibrary().getArrayClass());
} else {
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), Arrays.copyOfRange((double[]) array.getStore(), from, to), to - from);
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), ArrayUtils.extractRange((double[]) array.getStore(), from, to), to - from);
}
}

@@ -92,7 +94,7 @@ public RubyArray sliceObject(RubyArray array) {
if (from >= to) {
return new RubyArray(getContext().getCoreLibrary().getArrayClass());
} else {
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), Arrays.copyOfRange((Object[]) array.getStore(), from, to), to - from);
return new RubyArray(getContext().getCoreLibrary().getArrayClass(), ArrayUtils.extractRange((Object[]) array.getStore(), from, to), to - from);
}
}

72 changes: 72 additions & 0 deletions core/src/main/java/org/jruby/truffle/runtime/util/ArrayUtils.java
View file
Original file line number Diff line number Diff line change
@@ -18,6 +18,78 @@

public abstract class ArrayUtils {

/**
* Extracts part of an array into a newly allocated Object[] array. Does not perform safety checks on parameters.
* @param source the source array whose values should be extracted
* @param start the start index, must be >= 0 and < source.length
* @param end the end index (exclusive), must be >= 0 and <= source.length and >= start
* @return a newly allocated array with the extracted elements and length (end - start)
*/
public static Object[] extractRange(int[] source, int start, int end) {
assert checkExtractRangeArgs(source, start, end);
int length = end - start;
Object[] result = new Object[length];
System.arraycopy(source, start, result, 0, length);
return result;
}

private static boolean checkExtractRangeArgs(int[] source, int start, int end) {
assert source != null;
assert start >= 0;
assert start <= source.length;
assert end >= start;
assert end <= source.length;
return true;
}

/**
* Extracts part of an array into a newly allocated Object[] array. Does not perform safety checks on parameters.
* @param source the source array whose values should be extracted
* @param start the start index, must be >= 0 and < source.length
* @param end the end index (exclusive), must be >= 0 and <= source.length and >= start
* @return a newly allocated array with the extracted elements and length (end - start)
*/
public static Object[] extractRange(long[] source, int start, int end) {
assert checkExtractRangeArgs(source, start, end);
int length = end - start;
Object[] result = new Object[length];
System.arraycopy(source, start, result, 0, length);
return result;
}

private static boolean checkExtractRangeArgs(long[] source, int start, int end) {
assert source != null;
assert start >= 0;
assert start <= source.length;
assert end >= start;
assert end <= source.length;
return true;
}

/**
* Extracts part of an array into a newly allocated Object[] array. Does not perform safety checks on parameters.
* @param source the source array whose values should be extracted
* @param start the start index, must be >= 0 and < source.length
* @param end the end index (exclusive), must be >= 0 and <= source.length and >= start
* @return a newly allocated array with the extracted elements and length (end - start)
*/
public static Object[] extractRange(double[] source, int start, int end) {
assert checkExtractRangeArgs(source, start, end);
int length = end - start;
Object[] result = new Object[length];
System.arraycopy(source, start, result, 0, length);
return result;
}

private static boolean checkExtractRangeArgs(double[] source, int start, int end) {

This comment has been minimized.

Sign in to view

Copy link
@thomaswue

thomaswue Jan 13, 2015

Contributor

You can have only a single check method if you pass in the array as an object parameter and use Arrays.getLength.

This comment has been minimized.

Sign in to view

Copy link
@thomaswue

thomaswue Jan 13, 2015

Contributor

We should maybe also rename the method and have the prefix convention "assert" instead of "check" so people are not confused and use the return value of the method for purposes other than assert.

This comment has been minimized.

Sign in to view

Copy link
@eregon

eregon Jan 13, 2015

Author Member

Thanks, I did not know about Array.getLength. The fact it is in java.lang.reflect is not a problem? Is it as efficient for us as calling some_typed_array.length?

So, void assertExtractRangeArgs? Or do you want to keep the return value here?

This comment has been minimized.

Sign in to view

Copy link
@thomaswue

thomaswue Jan 13, 2015

Contributor

It is probably a lot less efficient (unless the JIT compiler has some clever special handling), but this is assertion code and can be therefore slow. No, make it boolean assertExtractRangeArgs and return true like now. It is important to return the value and test it via an assert to make sure the JIT compilers are not tempted to inline the method.
assert source != null;
assert start >= 0;
assert start <= source.length;
assert end >= start;
assert end <= source.length;
return true;
}

/**
* Extracts part of an array into a newly allocated Object[] array. Does not perform safety checks on parameters.
* @param source the source array whose values should be extracted

0 comments on commit 2f08382

Please sign in to comment.