This is an automated email from the ASF dual-hosted git repository. bmahler pushed a commit to branch 1.7.x in repository https://gitbox.apache.org/repos/asf/mesos.git
commit a41731b0facd1a61301a7d0b99f10e28f3b03a48 Author: Benjamin Mahler <[email protected]> AuthorDate: Sun Sep 16 19:18:49 2018 -0700 Cached weights in the sorters nodes. This avoids making excessive map lookups each time we calculate the share for a node in the tree. Now, when the weight is needed, the value is cached. If the weight gets updated, we update the cached value. This approach proved cleaner than trying to ensure freshly constructed nodes have the right weight. Review: https://reviews.apache.org/r/68732 --- src/master/allocator/sorter/drf/sorter.cpp | 28 ++++++++++++++++++++++----- src/master/allocator/sorter/drf/sorter.hpp | 6 ++++++ src/master/allocator/sorter/random/sorter.cpp | 28 ++++++++++++++++++++++----- src/master/allocator/sorter/random/sorter.hpp | 6 ++++++ 4 files changed, 58 insertions(+), 10 deletions(-) diff --git a/src/master/allocator/sorter/drf/sorter.cpp b/src/master/allocator/sorter/drf/sorter.cpp index a45f66f..b04d684 100644 --- a/src/master/allocator/sorter/drf/sorter.cpp +++ b/src/master/allocator/sorter/drf/sorter.cpp @@ -312,6 +312,24 @@ void DRFSorter::updateWeight(const string& path, double weight) // TODO(neilc): Avoid dirtying the tree in some circumstances. dirty = true; + + // Update the weight of the corresponding internal node, + // if it exists (this client may not exist despite there + // being a weight). + Node* node = find(path); + + if (node == nullptr) { + return; + } + + // If there is a virtual leaf, we need to move up one level. + if (node->name == ".") { + node = CHECK_NOTNULL(node->parent); + } + + CHECK_EQ(path, node->path); + + node->weight = weight; } @@ -625,11 +643,11 @@ double DRFSorter::calculateShare(const Node* node) const double DRFSorter::getWeight(const Node* node) const { - // TODO(bmahler): It's expensive to have to hash the complete - // role path and re-lookup the weight each time we calculate - // the share, consider storing the weight directly in the - // node struct. - return weights.get(node->path).getOrElse(1.0); + if (node->weight.isNone()) { + node->weight = weights.get(node->path).getOrElse(1.0); + } + + return CHECK_NOTNONE(node->weight); } diff --git a/src/master/allocator/sorter/drf/sorter.hpp b/src/master/allocator/sorter/drf/sorter.hpp index 2957a2e..084df82 100644 --- a/src/master/allocator/sorter/drf/sorter.hpp +++ b/src/master/allocator/sorter/drf/sorter.hpp @@ -243,6 +243,12 @@ struct DRFSorter::Node double share; + // Cached weight of the node, access this through `getWeight()`. + // The value is cached by `getWeight()` and updated by + // `updateWeight()`. Marked mutable since the caching writes + // to this are logically const. + mutable Option<double> weight; + Kind kind; Node* parent; diff --git a/src/master/allocator/sorter/random/sorter.cpp b/src/master/allocator/sorter/random/sorter.cpp index fc47756..f578ef1 100644 --- a/src/master/allocator/sorter/random/sorter.cpp +++ b/src/master/allocator/sorter/random/sorter.cpp @@ -285,6 +285,24 @@ void RandomSorter::deactivate(const string& clientPath) void RandomSorter::updateWeight(const string& path, double weight) { weights[path] = weight; + + // Update the weight of the corresponding internal node, + // if it exists (this client may not exist despite there + // being a weight). + Node* node = find(path); + + if (node == nullptr) { + return; + } + + // If there is a virtual leaf, we need to move up one level. + if (node->name == ".") { + node = CHECK_NOTNULL(node->parent); + } + + CHECK_EQ(path, node->path); + + node->weight = weight; } @@ -542,11 +560,11 @@ size_t RandomSorter::count() const double RandomSorter::getWeight(const Node* node) const { - // TODO(bmahler): It's expensive to have to hash the complete - // role path and re-lookup the weight each time we calculate - // the share, consider storing the weight directly in the - // node struct. - return weights.get(node->path).getOrElse(1.0); + if (node->weight.isNone()) { + node->weight = weights.get(node->path).getOrElse(1.0); + } + + return CHECK_NOTNONE(node->weight); } diff --git a/src/master/allocator/sorter/random/sorter.hpp b/src/master/allocator/sorter/random/sorter.hpp index 825c158..800b22c 100644 --- a/src/master/allocator/sorter/random/sorter.hpp +++ b/src/master/allocator/sorter/random/sorter.hpp @@ -235,6 +235,12 @@ struct RandomSorter::Node // label for virtual leaf nodes. std::string path; + // Cached weight of the node, access this through `getWeight()`. + // The value is cached by `getWeight()` and updated by + // `updateWeight()`. Marked mutable since the caching writes + // to this are logically const. + mutable Option<double> weight; + Kind kind; Node* parent;
