[mesos] 01/10: Simplified the weight lookup logic in the sorters.

[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 6b29bd67c21f2df13d19fd52f52e1a8d5069ab9d
Author: Benjamin Mahler <bm...@apache.org>
AuthorDate: Sun Sep 16 13:05:43 2018 -0700

    Simplified the weight lookup logic in the sorters.
    
    Review: https://reviews.apache.org/r/68729
---
 src/master/allocator/sorter/drf/sorter.cpp    | 16 +++++++---------
 src/master/allocator/sorter/drf/sorter.hpp    |  2 +-
 src/master/allocator/sorter/random/sorter.cpp | 16 +++++++---------
 src/master/allocator/sorter/random/sorter.hpp |  2 +-
 4 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/src/master/allocator/sorter/drf/sorter.cpp b/src/master/allocator/sorter/drf/sorter.cpp
index 07e5482..aa925c4 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -615,19 +615,17 @@ double DRFSorter::calculateShare(const Node* node) const
     }
   }
 
-  return share / findWeight(node);
+  return share / getWeight(node);
 }
 
 
-double DRFSorter::findWeight(const Node* node) const
+double DRFSorter::getWeight(const Node* node) const
 {
-  Option<double> weight = weights.get(node->path);
-
-  if (weight.isNone()) {
-    return 1.0;
-  }
-
-  return weight.get();
+  // 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);
 }
 
 
diff --git a/src/master/allocator/sorter/drf/sorter.hpp b/src/master/allocator/sorter/drf/sorter.hpp
index 5a4fa5e..71352c8 100644
--- a/src/master/allocator/sorter/drf/sorter.hpp
+++ b/src/master/allocator/sorter/drf/sorter.hpp
@@ -116,7 +116,7 @@ private:
   // Returns the weight associated with the node. If no weight has
   // been configured for the node's path, the default weight (1.0) is
   // returned.
-  double findWeight(const Node* node) const;
+  double getWeight(const Node* node) const;
 
   // Returns the client associated with the given path. Returns
   // nullptr if the path is not found or if the path identifies an
diff --git a/src/master/allocator/sorter/random/sorter.cpp b/src/master/allocator/sorter/random/sorter.cpp
index d17f8af..7b98389 100644
--- a/src/master/allocator/sorter/random/sorter.cpp
+++ b/src/master/allocator/sorter/random/sorter.cpp
@@ -477,7 +477,7 @@ vector<string> RandomSorter::sort()
     vector<double> weights(inactiveBegin - node->children.begin());
 
     for (int i = 0; i < inactiveBegin - node->children.begin(); ++i) {
-      weights[i] = findWeight(node->children[i]);
+      weights[i] = getWeight(node->children[i]);
     }
 
     weightedShuffle(node->children.begin(), inactiveBegin, weights, generator);
@@ -537,15 +537,13 @@ size_t RandomSorter::count() const
 }
 
 
-double RandomSorter::findWeight(const Node* node) const
+double RandomSorter::getWeight(const Node* node) const
 {
-  Option<double> weight = weights.get(node->path);
-
-  if (weight.isNone()) {
-    return 1.0;
-  }
-
-  return weight.get();
+  // 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);
 }
 
 
diff --git a/src/master/allocator/sorter/random/sorter.hpp b/src/master/allocator/sorter/random/sorter.hpp
index 7f6c0de..6bfeda0 100644
--- a/src/master/allocator/sorter/random/sorter.hpp
+++ b/src/master/allocator/sorter/random/sorter.hpp
@@ -117,7 +117,7 @@ private:
   // Returns the weight associated with the node. If no weight has
   // been configured for the node's path, the default weight (1.0) is
   // returned.
-  double findWeight(const Node* node) const;
+  double getWeight(const Node* node) const;
 
   // Returns the client associated with the given path. Returns
   // nullptr if the path is not found or if the path identifies an