From 74bcf34d5d4f4d9e3b261ce637864f748b4815a1 Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Wed, 24 Jan 2018 11:47:02 +0100 Subject: [PATCH] Fixed performance bug with not filtering out tiels early --- .../index/curves/SpaceFillingCurve.java | 133 ++++++++++++++++-- .../index/curves/SpaceFillingCurveTest.java | 31 +++- 2 files changed, 149 insertions(+), 15 deletions(-) diff --git a/community/spatial-index/src/main/java/org/neo4j/gis/spatial/index/curves/SpaceFillingCurve.java b/community/spatial-index/src/main/java/org/neo4j/gis/spatial/index/curves/SpaceFillingCurve.java index ccfcdaf562bc7..613048bef7792 100644 --- a/community/spatial-index/src/main/java/org/neo4j/gis/spatial/index/curves/SpaceFillingCurve.java +++ b/community/spatial-index/src/main/java/org/neo4j/gis/spatial/index/curves/SpaceFillingCurve.java @@ -256,15 +256,38 @@ long[] normalizedCoordinateFor( long derivedValue, int level ) * Given an envelope, find a collection of LongRange of tiles intersecting it on maxLevel and merge adjacent ones */ public List getTilesIntersectingEnvelope( Envelope referenceEnvelope ) + { + return getTilesIntersectingEnvelope( referenceEnvelope, new RecursionStats( this.maxLevel ) ); + } + + /** + * Given an envelope, find a collection of LongRange of tiles intersecting it on maxLevel and merge adjacent ones + */ + List getTilesIntersectingEnvelope( Envelope referenceEnvelope, RecursionStats stats ) { SearchEnvelope search = new SearchEnvelope( referenceEnvelope ); - ArrayList results = new ArrayList<>(); + ArrayList results = new ArrayList<>(1000); - addTilesIntersectingEnvelopeAt( search, new SearchEnvelope( 0, this.getWidth(), nbrDim ), rootCurve(), 0, this.getValueWidth(), results ); + addTilesIntersectingEnvelopeAt( stats, 0, search, new SearchEnvelope( 0, this.getWidth(), nbrDim ), rootCurve(), 0, this.getValueWidth(), results ); return results; } - private void addTilesIntersectingEnvelopeAt( SearchEnvelope search, SearchEnvelope currentExtent, CurveRule curve, long left, long right, + public static class RecursionStats + { + int maxDepth=0; + int[] counts; + RecursionStats(int maxLevel) + { + this.counts = new int[maxLevel + 1]; + } + void mark(int depth) + { + maxDepth = Math.max( maxDepth, depth ); + counts[depth] ++; + } + } + + private void addTilesIntersectingEnvelopeAt( RecursionStats stats, int depth, SearchEnvelope search, SearchEnvelope currentExtent, CurveRule curve, long left, long right, ArrayList results ) { if ( right - left == 1 ) @@ -283,16 +306,36 @@ private void addTilesIntersectingEnvelopeAt( SearchEnvelope search, SearchEnvelo results.add( current ); } } + stats.mark( depth ); } else if ( search.intersects( currentExtent ) ) { - long width = (right - left) / quadFactor; - for ( int i = 0; i < quadFactor; i++ ) + double overlap = search.fractionOf( currentExtent ); + if ( overlap >= 0.50 ) + { + // Note that LongRange upper bound is inclusive, hence the '-1' in several places + LongRange current = (results.size() > 0) ? results.get( results.size() - 1 ) : null; + if ( current != null && current.max == left - 1 ) + { + current.expandToMax( right - 1 ); + } + else + { + current = new LongRange( left, right - 1 ); + results.add( current ); + } + stats.mark( depth ); + } + else { - int npoint = curve.npointForIndex( i ); + long width = (right - left) / quadFactor; + for ( int i = 0; i < quadFactor; i++ ) + { + int npoint = curve.npointForIndex( i ); - SearchEnvelope quadrant = currentExtent.quadrant( bitValues( npoint ) ); - addTilesIntersectingEnvelopeAt( search, quadrant, curve.childAt( i ), left + i * width, left + (i + 1) * width, results ); + SearchEnvelope quadrant = currentExtent.quadrant( bitValues( npoint ) ); + addTilesIntersectingEnvelopeAt( stats, depth + 1, search, quadrant, curve.childAt( i ), left + i * width, left + (i + 1) * width, results ); + } } } } @@ -426,8 +469,8 @@ public String toString() */ private class SearchEnvelope { - long[] min; - long[] max; + long[] min; // inclusive lower bounds + long[] max; // exclusive upper bounds int nbrDim; private SearchEnvelope( Envelope referenceEnvelope ) @@ -435,6 +478,10 @@ private SearchEnvelope( Envelope referenceEnvelope ) this.min = getNormalizedCoord( referenceEnvelope.getMin() ); this.max = getNormalizedCoord( referenceEnvelope.getMax() ); this.nbrDim = referenceEnvelope.getDimension(); + for ( int i = 0; i < nbrDim; i++ ) + { + this.max[i] += 1; + } } private SearchEnvelope( long[] min, long[] max ) @@ -475,7 +522,7 @@ private boolean contains( long[] coord ) { for ( int dim = 0; dim < nbrDim; dim++ ) { - if ( coord[dim] < min[dim] || coord[dim] > max[dim] ) + if ( coord[dim] < min[dim] || coord[dim] >= max[dim] ) { return false; } @@ -487,12 +534,74 @@ private boolean intersects( SearchEnvelope other ) { for ( int dim = 0; dim < nbrDim; dim++ ) { - if ( max[dim] < other.min[dim] || other.max[dim] < min[dim] ) + if ( this.max[dim] <= other.min[dim] || other.max[dim] <= this.min[dim] ) { return false; } } return true; } + + /** + * The smallest possible envelope has unit area 1 + */ + public double getArea() + { + double area = 1.0; + for ( int i = 0; i < min.length; i++ ) + { + area *= max[i] - min[i]; + } + return area; + } + + public double fractionOf( SearchEnvelope other ) + { + SearchEnvelope intersection = this.intersection(other); + return intersection == null ? 0.0 : intersection.getArea() / other.getArea(); + } + + /** + * Returns true for the smallest possible envelope, of area 1 + */ + public boolean isUnit() + { + for ( int i = 0; i < min.length; i++ ) + { + if ( max[i] - min[i] > 1 ) + { + return false; + } + } + return true; + } + + public SearchEnvelope intersection( SearchEnvelope other ) + { + if ( nbrDim == other.nbrDim ) + { + long[] i_min = new long[this.min.length]; + long[] i_max = new long[this.min.length]; + boolean result = true; + for ( int i = 0; i < min.length; i++ ) + { + if ( other.min[i] < this.max[i] && this.min[i] < other.max[i] ) + { + i_min[i] = Math.max(this.min[i], other.min[i]); + i_max[i] = Math.min(this.max[i], other.max[i]); + } + else + { + result = false; + } + } + return result ? new SearchEnvelope( i_min, i_max ) : null; + } + else + { + throw new IllegalArgumentException( + "Cannot calculate intersection of Envelopes with different dimensions: " + this.nbrDim + " != " + other.nbrDim ); + } + } } } diff --git a/community/spatial-index/src/test/java/org/neo4j/gis/spatial/index/curves/SpaceFillingCurveTest.java b/community/spatial-index/src/test/java/org/neo4j/gis/spatial/index/curves/SpaceFillingCurveTest.java index 47c2286723447..e3cb8bda97f97 100644 --- a/community/spatial-index/src/test/java/org/neo4j/gis/spatial/index/curves/SpaceFillingCurveTest.java +++ b/community/spatial-index/src/test/java/org/neo4j/gis/spatial/index/curves/SpaceFillingCurveTest.java @@ -334,11 +334,11 @@ public void shouldGet2DHilbertSearchTilesForManyLevels() @Test public void shouldGet2DHilbertSearchTilesForWideRangeAtManyLevels() { + boolean verbose = false; Envelope envelope = new Envelope( -180, 180, -90, 90 ); - for ( int level = 1; level <= 11; level++ ) // 12 takes 6s, 13 takes 25s, 14 takes 100s, 15 takes over 400s + for ( int level = 1; level <= HilbertSpaceFillingCurve2D.MAX_LEVEL; level++ ) // 12 takes 6s, 13 takes 25s, 14 takes 100s, 15 takes over 400s { HilbertSpaceFillingCurve2D curve = new HilbertSpaceFillingCurve2D( envelope, level ); - System.out.print( "Testing hilbert query at level " + level ); double halfTile = curve.getTileWidth( 0, level ) / 2.0; long start = System.currentTimeMillis(); // Bottom left should give 1/4 of all tiles started at index 0 @@ -353,7 +353,32 @@ public void shouldGet2DHilbertSearchTilesForWideRangeAtManyLevels() // Bottom right should give 1/4 of all tiles started at index 3/4 assertTiles( curve.getTilesIntersectingEnvelope( new Envelope( 0, envelope.getMax( 0 ), envelope.getMin( 1 ), 0 - halfTile ) ), new SpaceFillingCurve.LongRange( 3 * curve.getValueWidth() / 4, curve.getValueWidth() - 1 ) ); - System.out.println( ", took " + (System.currentTimeMillis() - start) + "ms" ); + + // Now use a sliding window of moderate size (1/8 width and 1/8 height) to test many non-aligned positions + double eighthWidth = envelope.getWidth( 0 ) / 8.0; + double eighthHeight = envelope.getWidth( 1 ) / 8.0; + double left = envelope.getMin( 0 ); + double bottom = envelope.getMin( 1 ); + while ( left < envelope.getMax( 0 ) - eighthWidth ) + { + Envelope eighth = new Envelope( left, left + eighthWidth, bottom, bottom + eighthHeight ); + SpaceFillingCurve.RecursionStats stats = new SpaceFillingCurve.RecursionStats( curve.getMaxLevel() ); + curve.getTilesIntersectingEnvelope( eighth, stats ); + int expectedMaxDepth = Math.max( 10, level / 1 ); + assertThat( "Expected to not recurse deeper than " + expectedMaxDepth + " for level " + level, stats.maxDepth, + lessThanOrEqualTo( expectedMaxDepth ) ); + if ( verbose ) + { + System.out.println( "Recursed to max-depth: " + stats.maxDepth ); + for ( int i = 0; i <= stats.maxDepth; i++ ) + { + System.out.println( "\t" + i + "\t" + stats.counts[i] ); + } + } + left += eighthWidth / 5.12; + bottom += eighthHeight / 5.12; + } + System.out.println( "Tested hilbert query at level " + level + ", took " + (System.currentTimeMillis() - start) + "ms" ); } }