R-Tree tuning #4056

danpat · 2017-05-17T10:13:34Z

This PR refactors the R-Tree data layout to improve performance, reduce data sizes (a little bit) and simplify file structures.

TL;DR - 20% faster queries at the expense of 3.8x more .ramIndex data. Simpler file structures. 1% less R-Tree data overall (data moved from disk to RAM).

The main changes here are:

The LeafNode structure is removed. The .fileIndex file now contains a flat array of EdgeDataT objects, (still) sorted by Hilbert Code on their centroid. We still choose the number of EdgeDataT objects to pack into each leaf node based on a power-of-2 block size.
The TreeNode structure now only contains the bounding rectangle for each node in the R-Tree.

Tree navigation is now achieved by calculation, rather than storing pointers within the tree structure. A small new data structure m_tree_level_sizes is stored in the .ramIndex file. It contains the number of TreeNodes in each level of the tree, starting from the root (which is always size 1).

This is sufficient information, along with knowledge of BRANCHING_FACTOR and LEAF_NODE_SIZE, to calculate the position of the child of any node in the tree. The ExploreTreeNode, ExploreLeafNode and SearchInBox functions have been updated to use this approach (the calculation logic resides in the child_indexes(TreeIndex parent) member function of the StaticRTree class.

The net result of these changes is:

Improved performance - primarily due to the leaf node bounding rectangles being accessible in RAM, avoiding I/O for the lowest level bounding box check.
Slightly less disk - about 0.5% to 1% of the total R-Tree data was eliminated.
More RAM usage - the .ramIndex is now approximately 3.8x larger

Here is a spreadsheet that summarizes the size/performance differences after these changes are applied:

Tasklist

review
adjust for comments

oxidase

LGTM! Just some minor cleanup comments

oxidase · 2017-05-26T07:24:12Z

include/util/rectangle.hpp

+        << toFloating(rect.max_lat) << "],[" << toFloating(rect.max_lon) << ","
+        << toFloating(rect.max_lat) << "],[" << toFloating(rect.max_lon) << ","
+        << toFloating(rect.min_lat) << "],[" << toFloating(rect.min_lon) << ","
+        << toFloating(rect.min_lat) << "]";


a closing square bracket is missing or an opening one is not needed

oxidase · 2017-05-26T07:36:24Z

include/util/static_rtree.hpp

 template <class EdgeDataT,
          storage::Ownership Ownership = storage::Ownership::Container,
-          std::uint32_t BRANCHING_FACTOR = 128,
+          std::uint32_t BRANCHING_FACTOR = 64,
          std::uint32_t LEAF_PAGE_SIZE = 4096>


It is unused and can be removed, also L37-45 can be removed

oxidase · 2017-05-26T07:39:06Z

include/util/static_rtree.hpp

    static_assert(((LEAF_PAGE_SIZE - 1) & LEAF_PAGE_SIZE) == 0, "page size is not a power of 2");
-    static constexpr std::uint32_t LEAF_NODE_SIZE =
-        (LEAF_PAGE_SIZE - sizeof(uint32_t) - sizeof(Rectangle)) / sizeof(EdgeDataT);
+    static constexpr std::uint32_t LEAF_NODE_SIZE = (LEAF_PAGE_SIZE / sizeof(EdgeDataT));


LEAF_NODE_SIZE can be a template parameter similar to BRANCHING_FACTOR without computing from LEAF_PAGE_SIZE

The reason I left this in is so that the leaf-node groups of EdgeDataT objects are still clustered in page-sized blocks, to minimize disk reads. I guess we could make your suggested change and then just do some empirical tests to tune, but to be honest, the existing value seemed to perform best in my local tests, so I just left it alone.

TheMarex

Looks good to me, I think there are some minor things that could improve performance.

TheMarex · 2017-05-26T09:31:01Z

include/util/static_rtree.hpp

-        std::uint32_t is_leaf : 1;
+        TreeIndex() : level(0), offset(0) {}
+        TreeIndex(std::size_t level_, std::size_t offset_) : level(level_), offset(offset_) {}
+        std::size_t level;  // Which level of the tree is this node in


Going smaller here would help, the size of TreeIndex is important for the performance of the queue: The smaller this struct is the less data will be shuffled around. Going size_t -> uint32 should work for both. (maybe even 16 bits?)

TheMarex · 2017-05-26T09:31:43Z

include/util/static_rtree.hpp

        {
        }

        QueryCandidate(std::uint64_t squared_min_dist,
                       TreeIndex tree_index,
-                       std::uint32_t segment_index,
+                       std::uint64_t segment_index,


TheMarex · 2017-05-26T09:36:54Z

include/util/static_rtree.hpp

-    Vector<TreeNode> m_search_tree;
+    // We use a const view type when we don't own the data, otherwise
+    // we use a mutable type (probably becase we're building the tree)
+    using treeviewtype = typename std::conditional<Ownership == storage::Ownership::View,


CamelCase for our types.

TheMarex · 2017-05-26T09:39:24Z

include/util/static_rtree.hpp

+            std::uint32_t nodes_in_current_level =
+                std::ceil(static_cast<double>(nodes_in_previous_level) / BRANCHING_FACTOR);
+
+            for (std::uint32_t current_node_idx = 0; current_node_idx < nodes_in_current_level;


Could be refactored using util::irange

TheMarex · 2017-05-26T09:39:37Z

include/util/static_rtree.hpp

+
+                // Calculate the bounding box for BRANCHING_FACTOR nodes in the previous
+                // level, then save that box as a new TreeNode in the new level.
+                for (auto child_node_idx = first_child_index; child_node_idx < last_child_index;


util::irange

TheMarex · 2017-05-26T09:47:36Z

include/util/static_rtree.hpp

+        // 0 12 345 6789
+        // This ordering keeps the position math easy to understand during later
+        // searches
+        for (std::size_t i = 0; i < m_tree_level_sizes.size(); ++i)


util::irange

…tree structure is in the .ramIndex file. Also tunes the BRANCHING_FACTOR a bit to speed up access with this new layout.

danpat · 2017-06-02T05:40:54Z

@TheMarex I've addressed most of the review points. I'm out for the next few days, please feel free to merge this as-is, or make whatever remaining modifications are necessary to get it into master for 5.8.

danpat added the Work In Progress label May 17, 2017

danpat added this to the 5.8.0 milestone May 17, 2017

danpat self-assigned this May 17, 2017

This was referenced May 17, 2017

OMT packing for rtree. #3941

Closed

Avoid boost::numeric_cast on trusted coordinates #4059

Merged

danpat force-pushed the rtree_tuning branch 2 times, most recently from f3c9d10 to 6e167ce Compare May 24, 2017 22:23

TheMarex mentioned this pull request May 24, 2017

Renumber graph nodes after partitioning them #4089

Merged

5 tasks

danpat force-pushed the rtree_tuning branch from 6e167ce to 697f025 Compare May 25, 2017 06:30

danpat added Review and removed Work In Progress labels May 25, 2017

danpat requested review from TheMarex and oxidase May 25, 2017 06:43

danpat force-pushed the rtree_tuning branch 3 times, most recently from b665d14 to 4a7d015 Compare May 25, 2017 16:45

oxidase approved these changes May 26, 2017

View reviewed changes

TheMarex approved these changes May 26, 2017

View reviewed changes

TheMarex added Review - In feedback and removed Review labels May 26, 2017

TheMarex mentioned this pull request May 27, 2017

Truncated .fileIndex file fails siliently. #4097

Closed

danpat mentioned this pull request May 28, 2017

Add error checking for .fileIndex and .names #4098

Merged

2 tasks

danpat force-pushed the rtree_tuning branch 2 times, most recently from 732a367 to 75b18ae Compare June 1, 2017 20:41

Refactor RTree so that .fileIndex only contains EdgeDataT, and all r-…

805b12f

…tree structure is in the .ramIndex file. Also tunes the BRANCHING_FACTOR a bit to speed up access with this new layout.

danpat force-pushed the rtree_tuning branch from 75b18ae to 805b12f Compare June 2, 2017 05:38

Remove compiler errors and use 32bit for segment_index

889a962

TheMarex added Ready To Merge and removed Review - In feedback labels Jun 2, 2017

TheMarex merged commit a195d7d into master Jun 2, 2017

TheMarex deleted the rtree_tuning branch June 2, 2017 12:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

R-Tree tuning #4056

R-Tree tuning #4056

danpat commented May 17, 2017 •

edited by TheMarex

Loading

oxidase left a comment

oxidase May 26, 2017

oxidase May 26, 2017

oxidase May 26, 2017

danpat Jun 2, 2017

TheMarex left a comment

TheMarex May 26, 2017

TheMarex May 26, 2017

TheMarex May 26, 2017

TheMarex May 26, 2017

TheMarex May 26, 2017

TheMarex May 26, 2017

danpat commented Jun 2, 2017

R-Tree tuning #4056

R-Tree tuning #4056

Conversation

danpat commented May 17, 2017 • edited by TheMarex Loading

Tasklist

oxidase left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheMarex left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danpat commented Jun 2, 2017

danpat commented May 17, 2017 •

edited by TheMarex

Loading