Don't create more objects than actually needed

We re-use PointError objects instead of sending them to GC. This does require external code to re-use the simplifier itself.

Author: craigtaverner

libs/geo/src/main/java/org/elasticsearch/geometry/simplify/GeometrySimplifier.java

@@ -23,8 +23,10 @@ public abstract class GeometrySimplifier<T extends Geometry> {
  protected final int maxPoints;
  protected final SimplificationErrorCalculator calculator;
  protected final PointError[] points;
+ protected PointError lastRemoved;
  protected final Monitor monitor;
  protected int length;
+ protected int objCount = 0;
  protected String description;
 
  protected final PriorityQueue<PointError> queue = new PriorityQueue<>();
@@ -55,7 +57,7 @@ public void reset() {
  * Consume a single point on the stream of points to be simplified
  */
  public void consume(double x, double y) {
- PointError pointError = new PointError(length, x, y);
+ PointError pointError = makePointErrorFor(length, x, y);
  if (length > 1) {
  // we need at least three points to calculate the error of the middle point
  points[length - 1].error = calculator.calculateError(points[length - 2], points[length - 1], pointError);
@@ -78,8 +80,27 @@ public void consume(double x, double y) {
  */
  public abstract T produce();
 
+ private PointError makePointErrorFor(int index, double x, double y) {
+ if (index == maxPoints) {
+ if (lastRemoved == null) {
+ this.objCount++;
+ return new PointError(index, x, y);
+ } else {
+ return lastRemoved.reset(index, x, y);
+ }
+ } else {
+ if (points[index] == null) {
+ this.objCount++;
+ return new PointError(index, x, y);
+ } else {
+ return points[index].reset(index, x, y);
+ }
+ }
+ }
+
  private void removeAndAdd(int toRemove, PointError pointError) {
  assert toRemove > 0; // priority queue can never include first point as that always has zero error by definition
+ this.lastRemoved = this.points[toRemove];
  // Shift all points to the right of the removed point over it in the array
  System.arraycopy(this.points, toRemove + 1, this.points, toRemove, maxPoints - toRemove - 1);
  // Add the new point to the end of the array
@@ -119,8 +140,8 @@ private void updateErrorAt(int index) {
  */
  public static class PointError implements SimplificationErrorCalculator.PointLike, Comparable<PointError> {
  private int index;
- final double x;
- final double y;
+ private double x;
+ private double y;
  double error = 0;
 
  PointError(int index, double x, double y) {
@@ -148,6 +169,13 @@ public double x() {
  public double y() {
  return y;
  }
+
+ public PointError reset(int index, double x, double y) {
+ this.index = index;
+ this.x = x;
+ this.y = y;
+ return this;
+ }
  }
 
  /**

libs/geo/src/test/java/org/elasticsearch/geometry/simplify/GeometrySimplifierTests.java

@@ -17,6 +17,8 @@
 import org.elasticsearch.geometry.utils.GeometryValidator;
 import org.elasticsearch.geometry.utils.WellKnownText;
 import org.elasticsearch.test.ESTestCase;
+import org.hamcrest.Matcher;
+import org.hamcrest.Matchers;
 
 import java.io.BufferedReader;
 import java.io.FileNotFoundException;
@@ -29,6 +31,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.PriorityQueue;
+import java.util.function.Function;
 import java.util.zip.GZIPInputStream;
 
 import static org.hamcrest.Matchers.closeTo;
@@ -111,36 +114,44 @@ public void endSimplification(String description, List<SimplificationErrorCalcul
  simplifications.computeIfPresent(description, (k, v) -> v.asCompleted());
  }
 
- public void assertCompleted(int simplificationCount, int minAdded, int minRemoved) {
+ public void assertCompleted(int simplificationCount, int shouldHaveAdded, int shouldHaveRemoved) {
+ assertCompleted(simplificationCount, shouldHaveAdded, shouldHaveRemoved, Matchers::equalTo);
+ }
+
+ public void assertCompleted(
+ int simplificationCount,
+ int shouldHaveAdded,
+ int shouldHaveRemoved,
+ Function<Integer, Matcher<Integer>> matcher
+ ) {
  int totalCount = simplifications.values().stream().mapToInt(v -> v.count).sum();
- assertThat(
- "Expected at least " + simplificationCount + " simplifications",
- totalCount,
- greaterThanOrEqualTo(simplificationCount)
- );
+ assertThat("Expected " + simplificationCount + " simplifications", totalCount, matcher.apply(simplificationCount));
  for (TestSimplificationState state : simplifications.values()) {
  assertTrue("Simplification should be completed: " + state.description, state.completed);
  }
  assertThat(
- "Expected at least " + minAdded + " points added when maxPoints was " + maxPoints,
+ "Expected " + shouldHaveAdded + " points added when maxPoints was " + maxPoints,
  added,
- greaterThanOrEqualTo(minAdded)
+ matcher.apply(shouldHaveAdded)
  );
  assertThat(
- "Expected at least " + minRemoved + " points removed when maxPoints was " + maxPoints,
+ "Expected " + shouldHaveRemoved + " points removed when maxPoints was " + maxPoints,
  removed,
- greaterThanOrEqualTo(minRemoved)
+ matcher.apply(shouldHaveRemoved)
  );
  }
  }
 
  public void testShortLine() {
- GeometrySimplifier<Line> simplifier = new GeometrySimplifier.LineStrings(10, calculator());
+ int maxPoints = 10;
+ var monitor = new TestMonitor("ShortLine", maxPoints);
+ GeometrySimplifier<Line> simplifier = new GeometrySimplifier.LineStrings(maxPoints, calculator(), monitor);
  Line line = new Line(new double[] { -1, 0, 1 }, new double[] { -1, 0, 1 });
 
  // Test full geometry simplification
  Line simplified = simplifier.simplify(line);
  assertThat("Same line", simplified, equalTo(line));
+ monitor.assertCompleted(0, 0, 0); // simplification never actually used
 
  // Test streaming simplification
  simplifier.reset();
@@ -149,11 +160,14 @@ public void testShortLine() {
  }
  Line streamSimplified = simplifier.produce();
  assertThat("Same line", streamSimplified, equalTo(simplified));
+ assertThat("Should create exactly the needed number of PointError objects", simplifier.objCount, equalTo(line.length()));
+ monitor.assertCompleted(0, 3, 0); // stream used for less than maxPoints
  }
 
  public void testStraightLine() {
  int maxPoints = 10;
- GeometrySimplifier<Line> simplifier = new GeometrySimplifier.LineStrings(maxPoints, calculator());
+ var monitor = new TestMonitor("StraightLine", maxPoints);
+ GeometrySimplifier<Line> simplifier = new GeometrySimplifier.LineStrings(maxPoints, calculator(), monitor);
  double[] x = new double[100];
  double[] y = new double[100];
  for (int i = 0; i < 100; i++) {
@@ -165,6 +179,7 @@ public void testStraightLine() {
  // Test full geometry simplification
  Line simplified = simplifier.simplify(line);
  assertLineEnds(maxPoints, simplified, line);
+ monitor.assertCompleted(1, line.length(), line.length() - maxPoints);
 
  // Test streaming simplification
  simplifier.reset();
@@ -180,13 +195,16 @@ public void testStraightLine() {
  assertTrue(onLine, pointExistsOnLine(px, py, simplified));
  assertTrue(onLine, pointExistsOnLine(px, py, streamSimplified));
  }
+ assertThat("Should create exactly the needed number of PointError objects", simplifier.objCount, equalTo(maxPoints + 1));
+ monitor.assertCompleted(1, 2 * line.length(), 2 * (line.length() - maxPoints));
  }
 
  public void testZigZagLine() {
  int maxPoints = 10;
  int zigSize = 2;
  int lineLength = (maxPoints - 1) * zigSize + 1; // chosen to get rid of all y-crossings during simplification
- GeometrySimplifier<Line> simplifier = new GeometrySimplifier.LineStrings(maxPoints, calculator());
+ var monitor = new TestMonitor("ZigZagLine", maxPoints);
+ GeometrySimplifier<Line> simplifier = new GeometrySimplifier.LineStrings(maxPoints, calculator(), monitor);
  double[] x = new double[lineLength];
  double[] y = new double[lineLength];
  for (int i = 0; i < lineLength; i++) {
@@ -202,6 +220,7 @@ public void testZigZagLine() {
  Line simplified = simplifier.simplify(line);
  assertLineEnds(maxPoints, simplified, line);
  assertLinePointNeverHave(simplified, 0.0);
+ monitor.assertCompleted(1, line.length(), line.length() - maxPoints);
 
  // Test streaming simplification
  simplifier.reset();
@@ -219,6 +238,8 @@ public void testZigZagLine() {
  assertTrue(onLine, pointExistsOnLine(px, py, streamSimplified));
  }
  assertThat("Same line", streamSimplified, equalTo(simplified));
+ assertThat("Should create exactly the needed number of PointError objects", simplifier.objCount, equalTo(maxPoints + 1));
+ monitor.assertCompleted(1, 2 * line.length(), 2 * (line.length() - maxPoints));
  }
 
  public void testCircularLine() {
@@ -238,14 +259,19 @@ public void testCircularLine() {
 
  // Test streaming simplification
  simplifier.reset();
+ simplifier.notifyMonitorSimplificationStart();
  for (int i = 0; i < line.length(); i++) {
  simplifier.consume(line.getX(i), line.getY(i));
  }
+ simplifier.notifyMonitorSimplificationEnd();
  Line streamSimplified = simplifier.produce();
  assertLineEnds(maxPoints, streamSimplified, line);
  assertPointsOnCircle(centerX, centerY, radius, streamSimplified);
  assertThat("Same line", streamSimplified, equalTo(simplified));
- monitor.assertCompleted(1, maxPoints * countFactor, maxPoints * (countFactor - 1));
+ int shouldHaveAdded = 2 * maxPoints * countFactor;
+ int shouldHaveRemoved = shouldHaveAdded - maxPoints * 2;
+ monitor.assertCompleted(2, shouldHaveAdded, shouldHaveRemoved);
+ assertThat("Should create exactly the needed number of PointError objects", simplifier.objCount, equalTo(maxPoints + 1));
  }
 
  public void testCircularPolygon() {
@@ -275,7 +301,10 @@ public void testCircularPolygon() {
  assertPolygonEnds("Polygon", maxPoints, simplified, polygon);
  assertPointsOnCircle(centerX, centerY, radius, streamSimplified);
  assertThat("Same line", streamSimplified, equalTo(simplified));
- monitor.assertCompleted(2, maxPoints * countFactor, maxPoints * (countFactor - 1));
+ int shouldHaveAdded = 2 * maxPoints * countFactor + 2;
+ int shouldHaveRemoved = shouldHaveAdded - maxPoints * 2;
+ monitor.assertCompleted(2, shouldHaveAdded, shouldHaveRemoved);
+ assertThat("Should create exactly the needed number of PointError objects", simplifier.objCount, equalTo(maxPoints + 1));
  }
 
  public void testLineWithBackPaths() {
@@ -288,6 +317,7 @@ public void testLineWithBackPaths() {
  Line simplified = simplifier.simplify(line);
  assertLineEnds(maxPoints, simplified, line);
  monitor.assertCompleted(1, x.length, x.length - maxPoints);
+ assertThat("Should create exactly the needed number of PointError objects", simplifier.objCount, equalTo(maxPoints + 1));
  }
 
  public void testLineWithBackPaths2() throws IOException, ParseException {
@@ -297,13 +327,16 @@ public void testLineWithBackPaths2() throws IOException, ParseException {
  -0.16 -0.1, -0.17 -0.13, -0.16 -0.16, -0.1 -0.175, 0.008 -0.175, 0.1 -0.15, 0.2 -0.05, 0.3 0.05, 0.4 0.12, 0.5 0.15,
  0.67 0.1, 0.8 0.0, 0.7 -0.07, 0.6 -0.12, 0.7 -0.185, 0.85 -0.18, 0.96 -0.09, 1.0 0.0))""";
  Line line = (Line) WellKnownText.fromWKT(GeometryValidator.NOOP, true, lineString);
- int maxPoints = 15;
- var monitor = new TestMonitor("LineWithBackPaths2", maxPoints);
- GeometrySimplifier.LineStrings simplifier = new GeometrySimplifier.LineStrings(maxPoints, calculator(), monitor);
- simplifier.simplify(line);
- simplifier = new GeometrySimplifier.LineStrings(7, calculator(), monitor);
- simplifier.simplify(line);
- monitor.assertCompleted(2, 2 * line.length(), 2 * (line.length() - maxPoints));
+ int[] maxPointsArray = new int[] { 15, 7 };
+ var monitor = new TestMonitor("LineWithBackPaths2", maxPointsArray[0]);
+ for (int maxPoints : maxPointsArray) {
+ var simplifier = new GeometrySimplifier.LineStrings(maxPoints, calculator(), monitor);
+ simplifier.simplify(line);
+ assertThat("Should create exactly the needed number of PointError objects", simplifier.objCount, equalTo(maxPoints + 1));
+ }
+ int shouldHaveAdded = maxPointsArray.length * line.length();
+ int shouldHaveRemoved = shouldHaveAdded - Arrays.stream(maxPointsArray).sum();
+ monitor.assertCompleted(2, shouldHaveAdded, shouldHaveRemoved);
  }
 
  public void testLineWithNarrowSpikes() {
@@ -328,6 +361,7 @@ public void testLineWithNarrowSpikes() {
  Line simplified = simplifier.simplify(line);
  assertLineWithNarrowSpikes(simplified, count);
  monitor.assertCompleted(1, x.length, x.length - maxPoints);
+ assertThat("Should create exactly the needed number of PointError objects", simplifier.objCount, equalTo(maxPoints + 1));
  }
 
  protected abstract void assertLineWithNarrowSpikes(Line simplified, int spikeCount);
@@ -355,6 +389,7 @@ public void testComplexGeoJsonPolygon() throws IOException, ParseException {
  Polygon simplified = simplifier.simplify(polygon);
  assertPolygonEnds("Polygon", maxPoints, simplified, polygon);
  monitor.assertCompleted(1, polygon.getPolygon().length(), polygon.getPolygon().length() - maxPoints);
+ assertThat("Should create exactly the needed number of PointError objects", simplifier.objCount, equalTo(maxPoints + 1));
  }
  }
 
@@ -370,6 +405,7 @@ public void testComplexGeoJsonPolygons() throws IOException, ParseException {
  Polygon simplified = simplifier.simplify(polygon);
  assertPolygonEnds("Polygon", maxPoints, simplified, polygon);
  monitor.assertCompleted(1, length, length - maxPoints);
+ assertThat("Should create exactly the needed number of PointError objects", simplifier.objCount, equalTo(maxPoints + 1));
  }
  }
  }
@@ -391,7 +427,12 @@ public void testComplexGeoJsonMultiPolygon() throws IOException, ParseException
  int maxPolyPoints = Math.max(4, (int) (simplificationFactor * polygon.getPolygon().length()));
  assertPolygonEnds("Polygon[" + i + "]", maxPolyPoints, simplified, polygon);
  }
- monitor.assertCompleted(simplifiedMultiPolygon.size(), maxPolyLength, maxPolyLength - maxPoints);
+ monitor.assertCompleted(
+ simplifiedMultiPolygon.size(),
+ maxPolyLength,
+ maxPolyLength - maxPoints,
+ Matchers::greaterThanOrEqualTo
+ );
  }
  }