[mesos] 04/10: Cached weights in the sorters nodes.

[bm...@apache.org]

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 <bm...@apache.org>
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;