From 664d282a21e1cc52695541f1cdc9bc6e37f0c663 Mon Sep 17 00:00:00 2001
From: Johannes Link <business@johanneslink.net>
Date: Sun, 25 Dec 2022 12:19:37 +0100
Subject: [PATCH] Optimized ShrinkingDistance.forCollection(..)

Working towards https://github.com/jlink/jqwik/pull/435 effects
---
 .../java/net/jqwik/api/ShrinkingDistance.java | 20 +++++++---------
 .../jqwik/api/support/LongArraysSupport.java  | 24 ++++++++++++-------
 .../shrinking/ShrinkingDistanceTests.java     |  3 +--
 3 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/api/src/main/java/net/jqwik/api/ShrinkingDistance.java b/api/src/main/java/net/jqwik/api/ShrinkingDistance.java
index 8768d206c..51594b60c 100644
--- a/api/src/main/java/net/jqwik/api/ShrinkingDistance.java
+++ b/api/src/main/java/net/jqwik/api/ShrinkingDistance.java
@@ -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);
 	}
 
@@ -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);
 	}
 
@@ -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;
 
@@ -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);
 	}
 
@@ -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;
diff --git a/api/src/main/java/net/jqwik/api/support/LongArraysSupport.java b/api/src/main/java/net/jqwik/api/support/LongArraysSupport.java
index 7e6265739..e88c0bd44 100644
--- a/api/src/main/java/net/jqwik/api/support/LongArraysSupport.java
+++ b/api/src/main/java/net/jqwik/api/support/LongArraysSupport.java
@@ -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) {
+			return Long.MAX_VALUE;
+		}
+		return summedValue;
 	}
 
 	public static long[] concatenate(List<long[]> listOfArrays) {
diff --git a/engine/src/test/java/net/jqwik/engine/properties/shrinking/ShrinkingDistanceTests.java b/engine/src/test/java/net/jqwik/engine/properties/shrinking/ShrinkingDistanceTests.java
index a12f434ab..3847f2f86 100644
--- a/engine/src/test/java/net/jqwik/engine/properties/shrinking/ShrinkingDistanceTests.java
+++ b/engine/src/test/java/net/jqwik/engine/properties/shrinking/ShrinkingDistanceTests.java
@@ -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