add support for STR bulk packing #67

ambling · 2017-03-03T08:03:37Z

For Issue #45

add support of Rtree creation through STR packing, according to the paper:
STR: a simple and efficient algorithm for R-tree packing

add a recursive function in RTree.Builder and set the default filling factor as 0.7 (1.0 is better for readonly RTree while 0.7 reserves space for insertion).

note that this implementation is only for 2D RTree.

…time

ambling · 2017-03-03T08:34:59Z

The CI checks failed with a buffer overflow on Java6

However, all tests passed after a re-run...

codecov-io · 2017-03-03T08:45:46Z

Codecov Report

Merging #67 into master will increase coverage by 0.42%.
The diff coverage is 95.83%.

@@             Coverage Diff              @@
##             master      #67      +/-   ##
============================================
+ Coverage      77.9%   78.33%   +0.42%     
- Complexity      640      641       +1     
============================================
  Files            57       57              
  Lines          1824     1869      +45     
  Branches        241      248       +7     
============================================
+ Hits           1421     1464      +43     
- Misses          355      357       +2     
  Partials         48       48

Impacted Files	Coverage Δ	Complexity Δ
...c/main/java/com/github/davidmoten/rtree/RTree.java	`97.95% <95.83%> (-0.72%)`	`65 <1> (+1)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23fad39...e2e170c. Read the comment docs.

davidmoten · 2017-03-03T20:31:26Z

Thanks for the PR, looks interesting! I'll review this weekend.

ambling · 2017-03-04T07:38:04Z

I fixed a bug but the CI failed again with openjdk6.

After trying the updated workaround on travis-ci/travis-ci#5227 it seems to be working.

davidmoten · 2017-03-04T09:40:09Z

What's your performance conclusion about using bulk loading?

…

On Sat, 4 Mar 2017, 18:38 Ambling ***@***.***> wrote: I fixed a bug but the CI failed again with openjdk6. After trying the updated workaround on travis-ci/travis-ci#5227 <travis-ci/travis-ci#5227> it seems to be working. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#67 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AATa6_db_RUuBX_YXmLI8Bxp8cBa7JY9ks5riRTdgaJpZM4MR-A7> .

ambling · 2017-03-04T10:02:31Z

@davidmoten
the results on index creation time in the benchmark on my laptop:

Benchmark                                                                               Mode  Cnt        Score       Error  Units
BenchmarksRTree.bulkLoadingFullRTreeCreation010                                        thrpt   10       73.747 ±     3.650  ops/s
BenchmarksRTree.bulkLoadingRTreeCreation010                                            thrpt   10       86.520 ±     1.905  ops/s
BenchmarksRTree.defaultRTreeCreation010                                                thrpt   10        6.506 ±     0.062  ops/s
BenchmarksRTree.flatBufferRTreeCreation010                                             thrpt   10     3818.918 ±    65.382  ops/s
BenchmarksRTree.starRTreeCreation010                                                   thrpt   10        3.141 ±     0.047  ops/s

The bulk loading is about 10x faster than the default RTree creation and about 20x faster than the R*-tree creation.

The FlatBuffers-based RTree is much faster is because the creation process just loads the basic context info and one root node.

Besides, it is interesting in the results that bulk packing with 70% full nodes is a little faster than that with 100% full nodes, while in fact there should be less nodes created if the nodes are 100% full. I may dig this deeper for the reason.

As for the insertion/deletion/search performance on the STR-RTree, it would be reasonably worse than that of the R*-tree, but I haven't check this yet.

And for geometry types other than the point, the performance of STR-RTree maybe even more worse, because the packing is based on just the central position, leading to more overlapping among nodes.

So actually I still recommend the R*-tree implementation in most cases, but for very large dataset indexing or readonly applications the STR-Rtree can be offered as a good option.

davidmoten · 2017-03-06T01:47:36Z

src/main/java/com/github/davidmoten/rtree/RTree.java

+            return packingSTR(nodes, false, size, context);
+        }
+
+        private class MidComparator implements Comparator<HasGeometry> {


make static final

davidmoten · 2017-03-06T01:47:46Z

src/main/java/com/github/davidmoten/rtree/RTree.java

+        }
+
+        private class MidComparator implements Comparator<HasGeometry> {
+            private short dimension; // leave space for multiple dimensions, 0 for x, 1 for y, ...


davidmoten · 2017-03-06T01:48:32Z

src/main/java/com/github/davidmoten/rtree/RTree.java

+        /**
+         * Create an RTree by bulk loading, using the STR method.
+         * STR: a simple and efficient algorithm for R-tree packing
+         * http://ieeexplore.ieee.org/abstract/document/582015/


add javadoc comment about the fact that the method mutates the input list

davidmoten · 2017-03-06T01:56:19Z

src/test/java/com/github/davidmoten/rtree/RTreeTest.java

@@ -713,25 +755,33 @@ public void testStarTreeReturnsSameAsStandardRTree() {
                rectangle(1, 0.252, 50, 69.23), rectangle(13.12, 23.123, 50.45, 80.9),
                rectangle(10, 10, 50, 50) };

+        List<Entry<Integer, Geometry>> entris = new ArrayList<Entry<Integer, Geometry>>(10000);


davidmoten · 2017-03-06T02:22:15Z

src/main/java/com/github/davidmoten/rtree/RTree.java

+            }
+
+            @Override
+            public boolean equals(Object obj) {


why are you implementing equals? I'd say remove.

ambling · 2017-03-06T03:14:34Z

Thanks for the review!

I have made the changes, please have a look at that.

davidmoten · 2017-03-06T08:23:08Z

Thanks @ambling. So I suppose the remaining task is to come up with some reasonably comprehensive advice about the pros and cons of using the STR bulk load. Would you be able to do a bit more perf testing so that we have something concrete to advise users?

ambling · 2017-03-06T13:53:12Z

Yes @davidmoten . I think maybe I can add one or more test datasets and more benchmarking cases to verify the performance. I may finish the task in several days.

davidmoten · 2017-03-06T21:28:42Z

Thanks @ambling. Let's merge this now and when you have more detail we'll add it the docs. Thanks very much for your work!

add support for STR bulk packing and benchmarking the index creation …

c840231

…time

ambling closed this Mar 3, 2017

ambling reopened this Mar 3, 2017

ambling added 2 commits March 4, 2017 15:02

fix a bug of factor type, add a benchmark about full node bulk packing

164e343

try the updated workaround for buffer overflow with openjdk6

a78748e

davidmoten requested changes Mar 6, 2017

View reviewed changes

add a javadoc note about the input mutation, add finals and fix a typo

e2e170c

davidmoten approved these changes Mar 6, 2017

View reviewed changes

davidmoten added the enhancement label Mar 6, 2017

davidmoten merged commit f0609d5 into davidmoten:master Mar 6, 2017

ambling deleted the str-bulk-loading branch March 7, 2017 03:12

david-bouyssie mentioned this pull request Mar 22, 2017

Bulk loading (with STR) #45

Closed

davidmoten mentioned this pull request Jul 11, 2019

rtree2 #95

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for STR bulk packing #67

add support for STR bulk packing #67

ambling commented Mar 3, 2017

ambling commented Mar 3, 2017 •

edited

Loading

codecov-io commented Mar 3, 2017 •

edited

Loading

davidmoten commented Mar 3, 2017

ambling commented Mar 4, 2017

davidmoten commented Mar 4, 2017 via email

ambling commented Mar 4, 2017 •

edited

Loading

davidmoten Mar 6, 2017

davidmoten Mar 6, 2017

davidmoten Mar 6, 2017

davidmoten Mar 6, 2017

davidmoten Mar 6, 2017

ambling commented Mar 6, 2017

davidmoten commented Mar 6, 2017

ambling commented Mar 6, 2017

davidmoten commented Mar 6, 2017

add support for STR bulk packing #67

add support for STR bulk packing #67

Conversation

ambling commented Mar 3, 2017

ambling commented Mar 3, 2017 • edited Loading

codecov-io commented Mar 3, 2017 • edited Loading

Codecov Report

davidmoten commented Mar 3, 2017

ambling commented Mar 4, 2017

davidmoten commented Mar 4, 2017 via email

ambling commented Mar 4, 2017 • edited Loading

davidmoten Mar 6, 2017

Choose a reason for hiding this comment

davidmoten Mar 6, 2017

Choose a reason for hiding this comment

davidmoten Mar 6, 2017

Choose a reason for hiding this comment

davidmoten Mar 6, 2017

Choose a reason for hiding this comment

davidmoten Mar 6, 2017

Choose a reason for hiding this comment

ambling commented Mar 6, 2017

davidmoten commented Mar 6, 2017

ambling commented Mar 6, 2017

davidmoten commented Mar 6, 2017

ambling commented Mar 3, 2017 •

edited

Loading

codecov-io commented Mar 3, 2017 •

edited

Loading

ambling commented Mar 4, 2017 •

edited

Loading