Skip to content

Commit

Permalink
[Skylark] Make range function lazy.
Browse files Browse the repository at this point in the history
range used to use MutableList which would eagerly allocate an array list with
all range elements, which is not efficient for very large ranges or when only
a small number of its elements are used.

This implementation uses a constant amount of RAM and computes a value for
each requested index.
  • Loading branch information
ttsugriy committed May 29, 2018
1 parent 7036e9d commit 965eee8
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.syntax.Concatable.Concatter;
import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.build.lib.syntax.SkylarkList.MutableListLike;
import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
import com.google.devtools.build.lib.syntax.SkylarkList.TupleLike;
import java.io.IOException;
import java.util.IllegalFormatException;

Expand Down Expand Up @@ -306,6 +308,15 @@ private static Object plus(
}
}

if (lval instanceof MutableListLike && rval instanceof MutableListLike) {
return MutableList.concat(((MutableListLike<?>) lval).toMutableList(),
((MutableListLike<?>) rval).toMutableList(), env.mutability());
}

if (lval instanceof TupleLike && rval instanceof TupleLike) {
return Tuple.concat(((TupleLike<?>) lval).toTuple(), ((TupleLike<?>) rval).toTuple());
}

if (lval instanceof SkylarkDict && rval instanceof SkylarkDict) {
if (env.getSemantics().incompatibleDisallowDictPlus()) {
throw new EvalException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature;
import com.google.devtools.build.lib.syntax.EvalUtils.ComparisonException;
import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.build.lib.syntax.SkylarkList.RangeList;
import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
import com.google.devtools.build.lib.syntax.Type.ConversionException;
import java.util.ArrayDeque;
Expand Down Expand Up @@ -622,7 +623,7 @@ public Integer invoke(String value) throws EvalException {

@SkylarkSignature(
name = "range",
returnType = MutableList.class,
returnType = RangeList.class,
doc =
"Creates a list where items go from <code>start</code> to <code>stop</code>, using a "
+ "<code>step</code> increment. If a single argument is provided, items will "
Expand Down Expand Up @@ -659,7 +660,7 @@ public Integer invoke(String value) throws EvalException {
)
private static final BuiltinFunction range =
new BuiltinFunction("range") {
public MutableList<?> invoke(
public RangeList invoke(
Integer startOrStop, Object stopOrNone, Integer step, Location loc, Environment env)
throws EvalException {
int start;
Expand All @@ -674,19 +675,12 @@ public MutableList<?> invoke(
if (step == 0) {
throw new EvalException(loc, "step cannot be 0");
}
ArrayList<Integer> result = new ArrayList<>(Math.abs((stop - start) / step));
if (step > 0) {
while (start < stop) {
result.add(start);
start += step;
}
} else {
while (start > stop) {
result.add(start);
start += step;
}
// simplify handling of a case like range(-2) or its equivalent range(-2, 0)
// by turning it into range(0, 0)
stop = Math.max(start, stop);
}
return MutableList.wrapUnsafe(env, result);
return RangeList.of(start, stop, step);
}
};

Expand Down
138 changes: 134 additions & 4 deletions src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
import com.google.devtools.build.lib.syntax.SkylarkMutable.BaseMutableList;
import java.util.AbstractList;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -116,8 +117,10 @@ public String toString() {
@Override
public boolean equals(Object object) {
return (this == object)
|| ((object != null) && (this.getClass() == object.getClass())
&& getContentsUnsafe().equals(((SkylarkList) object).getContentsUnsafe()));
|| ((object != null) && (this.getClass() == object.getClass()
|| this instanceof MutableListLike && object instanceof MutableListLike
|| this instanceof TupleLike && object instanceof TupleLike)
&& getContentsUnsafe().equals(((SkylarkList) object).getContentsUnsafe()));
}

@Override
Expand Down Expand Up @@ -201,6 +204,123 @@ public static <E> SkylarkList<E> createImmutable(Iterable<? extends E> contents)
return MutableList.copyOf(Mutability.IMMUTABLE, contents);
}

/** An interface for classes that can be converted to {@link MutableList}. */
public interface MutableListLike<E> {
MutableList<E> toMutableList();
}

/** An interface for classes that can be converted to {@link Tuple}. */
public interface TupleLike<E> {
Tuple<E> toTuple();
}

/**
* A sequence returned by the range function invocation.
*
* Instead of eagerly allocating an array with all elements of the sequence, this class uses
* simple math to compute a value at each index. This is particularly useful when range is huge
* or only a few elements from it are used.
*/
public static final class RangeList extends SkylarkList<Integer> implements
MutableListLike<Integer>, TupleLike<Integer> {

/** Provides access to range elements based on their index. */
private static class RangeListView extends AbstractList<Integer> {
private static int computeSize(int start, int stop, int step) {
final int length = Math.abs(stop - start);
final int absolute_step = Math.abs(step);
// round up (length / absolute_step) without using floats
return 1 + ((length - 1) / absolute_step);
}

private final int start;
private final int stop;
private final int step;
private final int size;

private RangeListView(int start, int stop, int step) {
this.start = start;
this.stop = stop;
this.step = step;
this.size = computeSize(start, stop, step);
}

@Override
public Integer get(int index) {
int value = start + step * index;
if ((step > 0 && value > stop) || (step < 0 && value < stop)) {
throw new ArrayIndexOutOfBoundsException(index);
}
return value;
}

@Override
public int size() {
return size;
}
}

private final AbstractList<Integer> contents;

private RangeList(int start, int stop, int step) {
this.contents = new RangeListView(start, stop, step);
}

@Override
public boolean isTuple() {
return false;
}

@Override
public ImmutableList<Integer> getImmutableList() {
return ImmutableList.copyOf(contents);
}

@Override
public SkylarkList<Integer> getSlice(Object start, Object end, Object step, Location loc,
Mutability mutability) throws EvalException {
List<Integer> sliceIndices = EvalUtils.getSliceIndices(start, end, step, this.size(), loc);
ImmutableList.Builder<Integer> builder = ImmutableList.builderWithExpectedSize(sliceIndices.size());
for (int pos : sliceIndices) {
builder.add(this.get(pos));
}
return MutableList.createImmutable(builder.build());
}

@Override
public SkylarkList<Integer> repeat(int times, Mutability mutability) {
ImmutableList.Builder<Integer> builder = ImmutableList.builderWithExpectedSize(this.size() * times);
for (int i = 0; i < times; i++) {
builder.addAll(this);
}
return MutableList.createImmutable(builder.build());
}

@Override
protected List<Integer> getContentsUnsafe() {
return contents;
}

@Override
public Mutability mutability() {
return Mutability.IMMUTABLE;
}

@Override
public MutableList<Integer> toMutableList() {
return MutableList.copyOf(Mutability.IMMUTABLE, contents);
}

@Override
public Tuple<Integer> toTuple() {
return Tuple.copyOf(contents);
}

public static RangeList of(int start, int stop, int step) {
return new RangeList(start, stop, step);
}
}

/**
* A Skylark list, i.e., the value represented by {@code [1, 2, 3]}. Lists are mutable datatypes.
*/
Expand All @@ -222,7 +342,7 @@ public static <E> SkylarkList<E> createImmutable(Iterable<? extends E> contents)
+ "['a', 'b', 'c', 'd'][3:0:-1] # ['d', 'c', 'b']</pre>"
+ "Lists are mutable, as in Python."
)
public static final class MutableList<E> extends SkylarkList<E> {
public static final class MutableList<E> extends SkylarkList<E> implements MutableListLike<E> {

private final ArrayList<E> contents;

Expand Down Expand Up @@ -565,6 +685,11 @@ public Object pop(Object i, Location loc, Environment env)
remove(index, loc, env.mutability());
return result;
}

@Override
public MutableList<E> toMutableList() {
return this;
}
}

/**
Expand All @@ -589,7 +714,7 @@ public Object pop(Object i, Location loc, Environment env)
+ "('a', 'b', 'c', 'd')[3:0:-1] # ('d', 'c', 'b')</pre>"
+ "Tuples are immutable, therefore <code>x[1] = \"a\"</code> is not supported."
)
public static final class Tuple<E> extends SkylarkList<E> {
public static final class Tuple<E> extends SkylarkList<E> implements TupleLike<E> {

private final ImmutableList<E> contents;

Expand Down Expand Up @@ -698,5 +823,10 @@ public Tuple<E> repeat(int times, Mutability mutability) {
}
return copyOf(builder.build());
}

@Override
public Tuple<E> toTuple() {
return this;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ public void testRange() throws Exception {
.testStatement("str(range(5, 0, -1))", "[5, 4, 3, 2, 1]")
.testStatement("str(range(5, 0, -10))", "[5]")
.testStatement("str(range(0, -3, -2))", "[0, -2]")
.testStatement("str(range(3)[1:2])", "[1]")
.testIfErrorContains("step cannot be 0", "range(2, 3, 0)");
}

Expand Down

0 comments on commit 965eee8

Please sign in to comment.