Commit b80d4419 authored by vmpstr's avatar vmpstr Committed by Commit bot

cc: Change RTree nodes container to be a vector (with reserve).

This patch changes the nodes container in the rtree to use a vector.
In order to preserve the consistent access to elements, it reserves the
vector to the needed capacity. The wasted space is <= 6 elements.

Added a unittest to verify the reserve path is correct for up to 1000
rects. However, locally I've tested this for up to 520k rects (and
still testing for up to 1m rects).

The perf results from N7v2 are below.
Before the patch:
[ RUN      ] RTreePerfTest.Construct
*RESULT rtree_construct: 100= 75723.8671875 runs/s
*RESULT rtree_construct: 1000= 11985 runs/s
*RESULT rtree_construct: 10000= 715.2415161132812 runs/s
*RESULT rtree_construct: 100000= 54.19050216674805 runs/s
[       OK ] RTreePerfTest.Construct (8243 ms)
[ RUN      ] RTreePerfTest.Search
*RESULT rtree_search: 100= 352105 runs/s
*RESULT rtree_search: 1000= 97542.0234375 runs/s
*RESULT rtree_search: 10000= 10274.0595703125 runs/s
*RESULT rtree_search: 100000= 866.167236328125 runs/s
[       OK ] RTreePerfTest.Search (8122 ms)

After the patch:
[ RUN      ] RTreePerfTest.Construct
*RESULT rtree_construct: 100= 112420 runs/s (+48%)
*RESULT rtree_construct: 1000= 13482.32421875 runs/s (+12%)
*RESULT rtree_construct: 10000= 666.6328125 runs/s (-7%)
*RESULT rtree_construct: 100000= 55.98017501831055 runs/s (+3%)
[       OK ] RTreePerfTest.Construct (8340 ms)
[ RUN      ] RTreePerfTest.Search
*RESULT rtree_search: 100= 365225 runs/s
*RESULT rtree_search: 1000= 98002.0078125 runs/s
*RESULT rtree_search: 10000= 10344.048828125 runs/s
*RESULT rtree_search: 100000= 866.0883178710938 runs/s
[       OK ] RTreePerfTest.Search (8092 ms)

BUG=674169
R=danakj@chromium.org, dskiba@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2591533002
Cr-Commit-Position: refs/heads/master@{#439686}
parent afdd0e97
...@@ -15,12 +15,16 @@ ...@@ -15,12 +15,16 @@
namespace cc { namespace cc {
RTree::RTree() : num_data_elements_(0u), nodes_(sizeof(Node)) {} RTree::RTree() : num_data_elements_(0u) {}
RTree::~RTree() {} RTree::~RTree() {}
RTree::Node* RTree::AllocateNodeAtLevel(int level) { RTree::Node* RTree::AllocateNodeAtLevel(int level) {
Node& node = nodes_.AllocateAndConstruct<Node>(); // We don't allow reallocations, since that would invalidate references to
// existing nodes, so verify that capacity > size.
DCHECK_GT(nodes_.capacity(), nodes_.size());
nodes_.emplace_back();
Node& node = nodes_.back();
node.num_children = 0; node.num_children = 0;
node.level = level; node.level = level;
return &node; return &node;
...@@ -36,17 +40,17 @@ RTree::Branch RTree::BuildRecursive(std::vector<Branch>* branches, int level) { ...@@ -36,17 +40,17 @@ RTree::Branch RTree::BuildRecursive(std::vector<Branch>* branches, int level) {
// We might sort our branches here, but we expect Blink gives us a reasonable // We might sort our branches here, but we expect Blink gives us a reasonable
// x,y order. Skipping a call to sort (in Y) here resulted in a 17% win for // x,y order. Skipping a call to sort (in Y) here resulted in a 17% win for
// recording with negligible difference in playback speed. // recording with negligible difference in playback speed.
int num_branches = static_cast<int>(branches->size() / MAX_CHILDREN); int num_branches = static_cast<int>(branches->size() / kMaxChildren);
int remainder = static_cast<int>(branches->size() % MAX_CHILDREN); int remainder = static_cast<int>(branches->size() % kMaxChildren);
if (remainder > 0) { if (remainder > 0) {
++num_branches; ++num_branches;
// If the remainder isn't enough to fill a node, we'll add fewer nodes to // If the remainder isn't enough to fill a node, we'll add fewer nodes to
// other branches. // other branches.
if (remainder >= MIN_CHILDREN) if (remainder >= kMinChildren)
remainder = 0; remainder = 0;
else else
remainder = MIN_CHILDREN - remainder; remainder = kMinChildren - remainder;
} }
int num_strips = static_cast<int>(std::ceil(std::sqrt(num_branches))); int num_strips = static_cast<int>(std::ceil(std::sqrt(num_branches)));
...@@ -58,15 +62,15 @@ RTree::Branch RTree::BuildRecursive(std::vector<Branch>* branches, int level) { ...@@ -58,15 +62,15 @@ RTree::Branch RTree::BuildRecursive(std::vector<Branch>* branches, int level) {
for (int i = 0; i < num_strips; ++i) { for (int i = 0; i < num_strips; ++i) {
// Might be worth sorting by X here too. // Might be worth sorting by X here too.
for (int j = 0; j < num_tiles && current_branch < branches->size(); ++j) { for (int j = 0; j < num_tiles && current_branch < branches->size(); ++j) {
int increment_by = MAX_CHILDREN; int increment_by = kMaxChildren;
if (remainder != 0) { if (remainder != 0) {
// if need be, omit some nodes to make up for remainder // if need be, omit some nodes to make up for remainder
if (remainder <= MAX_CHILDREN - MIN_CHILDREN) { if (remainder <= kMaxChildren - kMinChildren) {
increment_by -= remainder; increment_by -= remainder;
remainder = 0; remainder = 0;
} else { } else {
increment_by = MIN_CHILDREN; increment_by = kMinChildren;
remainder -= MAX_CHILDREN - MIN_CHILDREN; remainder -= kMaxChildren - kMinChildren;
} }
} }
Node* node = AllocateNodeAtLevel(level); Node* node = AllocateNodeAtLevel(level);
......
...@@ -11,7 +11,6 @@ ...@@ -11,7 +11,6 @@
#include <vector> #include <vector>
#include "cc/base/cc_export.h" #include "cc/base/cc_export.h"
#include "cc/base/contiguous_container.h"
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
namespace cc { namespace cc {
...@@ -61,14 +60,33 @@ class CC_EXPORT RTree { ...@@ -61,14 +60,33 @@ class CC_EXPORT RTree {
num_data_elements_ = branches.size(); num_data_elements_ = branches.size();
if (num_data_elements_ == 1u) { if (num_data_elements_ == 1u) {
nodes_.reserve(1);
Node* node = AllocateNodeAtLevel(0); Node* node = AllocateNodeAtLevel(0);
node->num_children = 1; node->num_children = 1;
node->children[0] = branches[0]; node->children[0] = branches[0];
root_.subtree = node; root_.subtree = node;
root_.bounds = branches[0].bounds; root_.bounds = branches[0].bounds;
} else if (num_data_elements_ > 1u) { } else if (num_data_elements_ > 1u) {
// Determine a reasonable upper bound on the number of nodes to prevent
// reallocations. This is basically (n**d - 1) / (n - 1), which is the
// number of nodes in a complete tree with n branches at each node. In the
// code n = |branch_count|, d = |depth|. However, we normally would have
// kMaxChildren branch factor, but that can be broken if some children
// don't have enough nodes. That can happen for at most kMinChildren nodes
// (since otherwise, we'd create a new node).
size_t branch_count = kMaxChildren;
double depth = log(branches.size()) / log(branch_count);
size_t node_count =
static_cast<size_t>((std::pow(branch_count, depth) - 1) /
(branch_count - 1)) +
kMinChildren;
nodes_.reserve(node_count);
root_ = BuildRecursive(&branches, 0); root_ = BuildRecursive(&branches, 0);
} }
// We should've wasted at most kMinChildren nodes.
DCHECK_LE(nodes_.capacity() - nodes_.size(),
static_cast<size_t>(kMinChildren));
} }
template <typename Container> template <typename Container>
...@@ -83,7 +101,8 @@ class CC_EXPORT RTree { ...@@ -83,7 +101,8 @@ class CC_EXPORT RTree {
private: private:
// These values were empirically determined to produce reasonable performance // These values were empirically determined to produce reasonable performance
// in most cases. // in most cases.
enum { MIN_CHILDREN = 6, MAX_CHILDREN = 11 }; enum { kMinChildren = 6 };
enum { kMaxChildren = 11 };
struct Node; struct Node;
struct Branch { struct Branch {
...@@ -101,7 +120,7 @@ class CC_EXPORT RTree { ...@@ -101,7 +120,7 @@ class CC_EXPORT RTree {
struct Node { struct Node {
uint16_t num_children; uint16_t num_children;
uint16_t level; uint16_t level;
Branch children[MAX_CHILDREN]; Branch children[kMaxChildren];
}; };
void SearchRecursive(Node* root, void SearchRecursive(Node* root,
...@@ -115,7 +134,7 @@ class CC_EXPORT RTree { ...@@ -115,7 +134,7 @@ class CC_EXPORT RTree {
// This is the count of data elements (rather than total nodes in the tree) // This is the count of data elements (rather than total nodes in the tree)
size_t num_data_elements_; size_t num_data_elements_;
Branch root_; Branch root_;
ContiguousContainer<Node> nodes_; std::vector<Node> nodes_;
}; };
} // namespace cc } // namespace cc
......
...@@ -10,6 +10,20 @@ ...@@ -10,6 +10,20 @@
namespace cc { namespace cc {
TEST(RTreeTest, ReserveNodesDoesntDcheck) {
// Make sure that anywhere between 0 and 1000 rects, our reserve math in rtree
// is correct. (This test would DCHECK if broken either in
// RTree::AllocateNodeAtLevel, indicating that the capacity calculation was
// too small or in RTree::Build, indicating the capacity was too large).
for (int i = 0; i < 1000; ++i) {
std::vector<gfx::Rect> rects;
for (int j = 0; j < i; ++j)
rects.push_back(gfx::Rect(j, i, 1, 1));
RTree rtree;
rtree.Build(rects);
}
}
TEST(RTreeTest, NoOverlap) { TEST(RTreeTest, NoOverlap) {
std::vector<gfx::Rect> rects; std::vector<gfx::Rect> rects;
for (int y = 0; y < 50; ++y) { for (int y = 0; y < 50; ++y) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment