Skip to content

Commit

Permalink
Optimized ShrinkingDistance.forCollection(..)
Browse files Browse the repository at this point in the history
Working towards #435 effects
  • Loading branch information
jlink committed Dec 25, 2022
1 parent 19ae50e commit 664d282
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 21 deletions.
20 changes: 9 additions & 11 deletions api/src/main/java/net/jqwik/api/ShrinkingDistance.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,9 @@ public static ShrinkingDistance of(long... distances) {

@API(status = MAINTAINED, since = "1.0")
public static <T> ShrinkingDistance forCollection(Collection<Shrinkable<T>> elements) {
ShrinkingDistance sumDistanceOfElements = elements
.stream()
.map(Shrinkable::distance)
.reduce(ShrinkingDistance.MIN, ShrinkingDistance::plus);

// This is an optimization to avoid creating temporary arrays, which the old streams-based implementation did.
long[] collectedDistances = sumUp(toDistances(elements));
ShrinkingDistance sumDistanceOfElements = new ShrinkingDistance(collectedDistances);
return ShrinkingDistance.of(elements.size()).append(sumDistanceOfElements);
}

Expand All @@ -41,9 +39,9 @@ public static <T> ShrinkingDistance combine(List<Shrinkable<T>> shrinkables) {
if (shrinkables.isEmpty()) {
throw new IllegalArgumentException("At least one shrinkable is required");
}
// This is an optimization to avoid creating many temporary arrays, which the old streams-based implementation did.
List<long[]> shrinkableDistances = toDistances(shrinkables);
long[] combinedDistances = concatenate(shrinkableDistances);

// This is an optimization to avoid creating temporary arrays, which the old streams-based implementation did.
long[] combinedDistances = concatenate(toDistances(shrinkables));
return new ShrinkingDistance(combinedDistances);
}

Expand All @@ -52,7 +50,7 @@ private ShrinkingDistance(long[] distances) {
}

@Override
public boolean equals(Object o) {
public boolean equals(@Nullable Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;

Expand Down Expand Up @@ -109,7 +107,7 @@ private int compareDimension(ShrinkingDistance other, int i) {

@API(status = INTERNAL)
public ShrinkingDistance plus(ShrinkingDistance other) {
long[] summedUpDistances = sumUp(distances, other.distances);
long[] summedUpDistances = sumUp(Arrays.asList(distances, other.distances));
return new ShrinkingDistance(summedUpDistances);
}

Expand All @@ -120,7 +118,7 @@ public ShrinkingDistance append(ShrinkingDistance other) {
}

@NotNull
private static <T> List<long[]> toDistances(List<Shrinkable<T>> shrinkables) {
private static <T> List<long[]> toDistances(Collection<Shrinkable<T>> shrinkables) {
List<long[]> listOfDistances = new ArrayList<>(shrinkables.size());
for (Shrinkable<?> tShrinkable : shrinkables) {
long[] longs = tShrinkable.distance().distances;
Expand Down
24 changes: 16 additions & 8 deletions api/src/main/java/net/jqwik/api/support/LongArraysSupport.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,24 @@ public static long at(long[] array, int i) {
return array.length > i ? array[i] : 0;
}

public static long[] sumUp(long[] left, long[] right) {
long[] sum = new long[Math.max(left.length, right.length)];
for (int i = 0; i < sum.length; i++) {
long summedValue = at(left, i) + at(right, i);
if (summedValue < 0) {
summedValue = Long.MAX_VALUE;
public static long[] sumUp(List<long[]> listOfArrays) {
int maxDistanceSize = listOfArrays.stream().mapToInt(s -> s.length).max().orElse(0);
long[] summedUpArray = new long[maxDistanceSize];
Arrays.fill(summedUpArray, 0);
for (long[] array : listOfArrays) {
for (int i = 0; i < summedUpArray.length; i++) {
summedUpArray[i] = plusWithoutOverflowAt(summedUpArray, array, i);
}
sum[i] = summedValue;
}
return sum;
return summedUpArray;
}

private static long plusWithoutOverflowAt(long[] left, long[] right, int index) {
long summedValue = at(right, index) + at(left, index);
if (summedValue < 0) {

This comment has been minimized.

Copy link
@vlsi

vlsi Dec 25, 2022

Contributor

This is a wrong way to detect overflow

This comment has been minimized.

Copy link
@jlink

jlink Dec 25, 2022

Author Collaborator

The method name may be wrong. The point is to add up to Long.MAX. And it works for the positive values that a distance can have.

This comment has been minimized.

Copy link
@vlsi

vlsi Dec 25, 2022

Contributor

Please check the way to detect overflow in Java. For instance, check how Math.addExact(long, long) is implemented

return Long.MAX_VALUE;
}
return summedValue;
}

public static long[] concatenate(List<long[]> listOfArrays) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ class ForCollections {
@Example
void emptyCollection() {
ShrinkingDistance distance = ShrinkingDistance.forCollection(Collections.emptyList());
assertThat(distance).isEqualTo(ShrinkingDistance.of(0, 0));
assertThat(distance).isEqualByComparingTo(ShrinkingDistance.MIN);
assertThat(distance).isEqualTo(ShrinkingDistance.MIN);
}

@Example
Expand Down

0 comments on commit 664d282

Please sign in to comment.