LUCENE-8746: Refactor EdgeTree #878

iverase · 2019-09-13T14:57:50Z

Another try in refactoring edge tree. This PR splits Edge Tree class into two and adds a new interface:

Component2D: Interface defining an object that knows its bounding box and can perform some spatial operations.
ComponentTree: An interval tree containing the different components (e.g polygon or line)
EdgeTree: An interval tree containing the edges of a components (polygon or line edges)

Unfortunately the PR touches quite a lots of files but most of them are test files. Running benchmark for points results look good, points shows same performance and there is an increase of performance for shapes (we are not computing the bounding box of the triangle many times).

Approach	Shape	M hits/sec Dev	M hits/sec Base	Diff	QPS Dev	QPS Base	Diff	Hit count Dev	Hit count Base	Diff
points	polyRussia	13.97	13.98	-0%	3.98	3.99	-0%	3508846	3508846	0%
points	poly 10	73.35	71.84	2%	46.39	45.43	2%	355809475	355809475	0%
points	polyMedium	8.73	8.67	1%	106.99	106.24	1%	2693559	2693559	0%
shapes	polyRussia	6.88	5.73	20%	1.96	1.63	20%	3508846	3508846	0%
shapes	poly 10	27.98	27.03	4%	17.69	17.10	4%	355809475	355809475	0%
shapes	polyMedium	2.79	2.75	2%	34.19	33.66	2%	2693559	2693559	0%

nknize · 2019-10-08T16:52:19Z

I checked out the PR and got the following test failures:

NOTE: reproduce with: ant test  -Dtestcase=TestLatLonPointQueries -Dtests.method=testAllLatEqual -Dtests.seed=DF1A4B86CC81C3AB -Dtests.slow=true -Dtests.badapples=true -Dtests.locale=seh-MZ -Dtests.timezone=Antarctica/Syowa -Dtests.asserts=true -Dtests.file.encoding=UTF-8

NOTE: reproduce with: ant test  -Dtestcase=TestLatLonPointQueries -Dtests.method=testSmallSetPolyWholeMap -Dtests.seed=DF1A4B86CC81C3AB -Dtests.slow=true -Dtests.badapples=true -Dtests.locale=seh-MZ -Dtests.timezone=Antarctica/Syowa -Dtests.asserts=true -Dtests.file.encoding=UTF-8

NOTE: reproduce with: ant test  -Dtestcase=TestLatLonPointQueries -Dtests.method=testLowCardinality -Dtests.seed=DF1A4B86CC81C3AB -Dtests.slow=true -Dtests.badapples=true -Dtests.locale=seh-MZ -Dtests.timezone=Antarctica/Syowa -Dtests.asserts=true -Dtests.file.encoding=UTF-8

NOTE: reproduce with: ant test  -Dtestcase=TestLatLonPointQueries -Dtests.method=testAllLonEqual -Dtests.seed=DF1A4B86CC81C3AB -Dtests.slow=true -Dtests.badapples=true -Dtests.locale=seh-MZ -Dtests.timezone=Antarctica/Syowa -Dtests.asserts=true -Dtests.file.encoding=UTF-8

NOTE: reproduce with: ant test  -Dtestcase=TestLatLonPointQueries -Dtests.method=testRandomMedium -Dtests.seed=DF1A4B86CC81C3AB -Dtests.slow=true -Dtests.badapples=true -Dtests.locale=seh-MZ -Dtests.timezone=Antarctica/Syowa -Dtests.asserts=true -Dtests.file.encoding=UTF-8

NOTE: reproduce with: ant test  -Dtestcase=TestLatLonPointQueries -Dtests.method=testRandomTiny -Dtests.seed=DF1A4B86CC81C3AB -Dtests.slow=true -Dtests.badapples=true -Dtests.locale=seh-MZ -Dtests.timezone=Antarctica/Syowa -Dtests.asserts=true -Dtests.file.encoding=UTF-8

iverase · 2019-10-08T17:30:58Z

@nknize totally right, some bug I introduce when trying to improve it. Now it should be fixed.

nknize

Gave it a first go and have a few thoughts / suggestions...

nknize · 2019-10-08T16:40:53Z

lucene/core/src/java/org/apache/lucene/document/LatLonPointInPolygonQuery.java

          }
        };
  }

  @Override
  public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException {

+    final Component2D tree = Polygon2D.create(polygons);


Can we push these variables into getIntersectVisitor? That would delay creating the bounding box, Component2D, and PolygonPredicate objects until the visitor is needed and save unnecessary computation when no docs contain point fields.

See above suggestion: I think it makes the code here a bit cleaner?

Not sure if we should do that. I think we create one ScorerSupplier per segment so moving that logic to the IntersectVisitor means that we will creating this objects per segment.

👍 Thanks for reminding me. I ran into that same problem ages ago as well.

nknize · 2019-10-08T16:41:33Z

lucene/core/src/java/org/apache/lucene/document/LatLonPointInPolygonQuery.java

@@ -84,7 +84,7 @@ public void visit(QueryVisitor visitor) {
    }
  }

-  private IntersectVisitor getIntersectVisitor(DocIdSetBuilder result, Polygon2D tree, GeoEncodingUtils.PolygonPredicate polygonPredicate,
+  private IntersectVisitor getIntersectVisitor(DocIdSetBuilder result, Component2D tree, GeoEncodingUtils.PolygonPredicate polygonPredicate,


Suggested change

private IntersectVisitor getIntersectVisitor(DocIdSetBuilder result, Component2D tree, GeoEncodingUtils.PolygonPredicate polygonPredicate,

private IntersectVisitor getIntersectVisitor(DocIdSetBuilder result) {

nknize · 2019-10-08T16:42:27Z

lucene/core/src/java/org/apache/lucene/document/LatLonPointInPolygonQuery.java

@@ -84,7 +84,7 @@ public void visit(QueryVisitor visitor) {
    }
  }

-  private IntersectVisitor getIntersectVisitor(DocIdSetBuilder result, Polygon2D tree, GeoEncodingUtils.PolygonPredicate polygonPredicate,
+  private IntersectVisitor getIntersectVisitor(DocIdSetBuilder result, Component2D tree, GeoEncodingUtils.PolygonPredicate polygonPredicate,
                                               byte[] minLat, byte[] maxLat, byte[] minLon, byte[] maxLon) {


Suggested change

byte[] minLat, byte[] maxLat, byte[] minLon, byte[] maxLon) {

final Component2D tree = Polygon2D.create(polygons);

final GeoEncodingUtils.PolygonPredicate polygonPredicate = GeoEncodingUtils.createComponentPredicate(tree);

// bounding box over all polygons, this can speed up tree intersection/cheaply improve approximation for complex multi-polygons

final byte minLat[] = new byte[Integer.BYTES];

final byte maxLat[] = new byte[Integer.BYTES];

final byte minLon[] = new byte[Integer.BYTES];

final byte maxLon[] = new byte[Integer.BYTES];

NumericUtils.intToSortableBytes(encodeLatitude(tree.getMinY()), minLat, 0);

NumericUtils.intToSortableBytes(encodeLatitude(tree.getMaxY()), maxLat, 0);

NumericUtils.intToSortableBytes(encodeLongitude(tree.getMinX()), minLon, 0);

NumericUtils.intToSortableBytes(encodeLongitude(tree.getMaxX()), maxLon, 0);

lucene/core/src/java/org/apache/lucene/geo/Component2D.java

lucene/core/src/java/org/apache/lucene/geo/EdgeTree.java

iverase · 2019-10-14T16:46:39Z

@nkine do you think we can move this forward?

nknize

LGTM. Thank you @iverase

Introduce a Component tree that represents the tree of components (e.g polygons). Edge tree is now just a tree of edges.

iverase added 3 commits September 13, 2019 14:19

Refactor of EdgeTree into an Rtree of components

e68f8e6

Some formatting

44b970e

Make building of the component tree cheaper

4db9057

iverase mentioned this pull request Sep 13, 2019

LUCENE-8746: Make EdgeTree (aka ComponentTree) support different type of components #627

Closed

iverase added 9 commits September 14, 2019 11:44

javadocs

b42c8de

make properties final

49f4b75

formatting

a870d5a

make class protected and final

6cbd1fc

make class protected and final

4c9fcf8

Polygon2D needs to be public

352db49

Line2D needs to be public

fcb7608

Polygon2D cannot be final

f3ab127

Merge branch 'master' into componet2Dv2

e0e7c1f

fix issue checking disjoint between box and point

2c3a9cc

iverase added 3 commits October 8, 2019 19:35

Add static containsPoint method to Component2D

0e48f00

clean left-overs

5c0dfe0

clean left-overs

004ebe3

nknize requested changes Oct 10, 2019

View reviewed changes

iverase added 2 commits October 10, 2019 22:50

address review comments

3d9a451

Merge branch 'master' into componet2Dv2

483f2c3

nknize approved these changes Oct 14, 2019

View reviewed changes

iverase added 3 commits October 14, 2019 19:10

Merge branch 'master' into componet2dV2

8dbcb45

Add entry in CHANGES.txt

87eb828

fix typo

50e59c0

iverase merged commit 68a3886 into apache:master Oct 14, 2019

asfgit pushed a commit that referenced this pull request Oct 14, 2019

LUCENE-8746: Refactor EdgeTree (#878)

d82682c

Introduce a Component tree that represents the tree of components (e.g polygons). Edge tree is now just a tree of edges.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LUCENE-8746: Refactor EdgeTree #878

LUCENE-8746: Refactor EdgeTree #878

iverase commented Sep 13, 2019

nknize commented Oct 8, 2019

iverase commented Oct 8, 2019

nknize left a comment

nknize Oct 8, 2019

nknize Oct 8, 2019

iverase Oct 10, 2019

nknize Oct 14, 2019

nknize Oct 8, 2019

nknize Oct 8, 2019

iverase commented Oct 14, 2019

nknize left a comment

	private IntersectVisitor getIntersectVisitor(DocIdSetBuilder result, Component2D tree, GeoEncodingUtils.PolygonPredicate polygonPredicate,
	private IntersectVisitor getIntersectVisitor(DocIdSetBuilder result) {

-                                               byte[] minLat, byte[] maxLat, byte[] minLon, byte[] maxLon) {
+    final Component2D tree = Polygon2D.create(polygons);
+    final GeoEncodingUtils.PolygonPredicate polygonPredicate = GeoEncodingUtils.createComponentPredicate(tree);
+    // bounding box over all polygons, this can speed up tree intersection/cheaply improve approximation for complex multi-polygons
+    final byte minLat[] = new byte[Integer.BYTES];
+    final byte maxLat[] = new byte[Integer.BYTES];
+    final byte minLon[] = new byte[Integer.BYTES];
+    final byte maxLon[] = new byte[Integer.BYTES];
+    NumericUtils.intToSortableBytes(encodeLatitude(tree.getMinY()), minLat, 0);
+    NumericUtils.intToSortableBytes(encodeLatitude(tree.getMaxY()), maxLat, 0);
+    NumericUtils.intToSortableBytes(encodeLongitude(tree.getMinX()), minLon, 0);
+    NumericUtils.intToSortableBytes(encodeLongitude(tree.getMaxX()), maxLon, 0);

LUCENE-8746: Refactor EdgeTree #878

LUCENE-8746: Refactor EdgeTree #878

Conversation

iverase commented Sep 13, 2019

nknize commented Oct 8, 2019

iverase commented Oct 8, 2019

nknize left a comment

Choose a reason for hiding this comment

nknize Oct 8, 2019

Choose a reason for hiding this comment

nknize Oct 8, 2019

Choose a reason for hiding this comment

iverase Oct 10, 2019

Choose a reason for hiding this comment

nknize Oct 14, 2019

Choose a reason for hiding this comment

nknize Oct 8, 2019

Choose a reason for hiding this comment

nknize Oct 8, 2019

Choose a reason for hiding this comment

iverase commented Oct 14, 2019

nknize left a comment

Choose a reason for hiding this comment