You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2014/12/18 02:32:15 UTC

[2/4] mesos git commit: Added an overload of Resources::contains for Resource object.

Added an overload of Resources::contains for Resource object.

Review: https://reviews.apache.org/r/29134


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/894ce24c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/894ce24c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/894ce24c

Branch: refs/heads/master
Commit: 894ce24c606e4c1ac5f2e03d3156cdde796b4e87
Parents: dacc882
Author: Jie Yu <yu...@gmail.com>
Authored: Tue Dec 16 17:32:38 2014 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Dec 17 17:31:07 2014 -0800

----------------------------------------------------------------------
 include/mesos/resources.hpp | 12 +++++++++++-
 src/common/resources.cpp    | 16 ++++++++++++++--
 2 files changed, 25 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/894ce24c/include/mesos/resources.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/resources.hpp b/include/mesos/resources.hpp
index 16d0546..44cfa06 100644
--- a/include/mesos/resources.hpp
+++ b/include/mesos/resources.hpp
@@ -109,6 +109,9 @@ public:
   // Checks if this Resources is a superset of the given Resources.
   bool contains(const Resources& that) const;
 
+  // Checks if this Resources contains the given Resource.
+  bool contains(const Resource& that) const;
+
   // Returns the reserved resources, by role.
   hashmap<std::string, Resources> reserved() const;
 
@@ -250,7 +253,14 @@ public:
   };
 
 private:
-  bool contains(const Resource& that) const;
+  // Similar to 'contains(const Resource&)' but skips the validity
+  // check. This can be used to avoid the performance overhead of
+  // calling 'contains(const Resource&)' when the resource can be
+  // assumed valid (e.g. it's inside a Resources).
+  //
+  // TODO(jieyu): Measure performance overhead of validity check to
+  // ensure this is warranted.
+  bool _contains(const Resource& that) const;
 
   // Similar to the public 'find', but only for a single Resource
   // object. The target resource may span multiple roles, so this

http://git-wip-us.apache.org/repos/asf/mesos/blob/894ce24c/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index 2bc2f0a..409dc18 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -425,7 +425,10 @@ Resources::Resources(
 bool Resources::contains(const Resources& that) const
 {
   foreach (const Resource& resource, that.resources) {
-    if (!contains(resource)) {
+    // NOTE: We use _contains because Resources only contain valid
+    // Resource objects, and we don't want the performance hit of the
+    // validity check.
+    if (!_contains(resource)) {
       return false;
     }
   }
@@ -434,6 +437,15 @@ bool Resources::contains(const Resources& that) const
 }
 
 
+bool Resources::contains(const Resource& that) const
+{
+  // NOTE: We must validate 'that' because invalid resources can lead
+  // to false positives here (e.g., "cpus:-1" will return true). This
+  // is because mesos::contains assumes resources are valid.
+  return validate(that).isNone() && _contains(that);
+}
+
+
 hashmap<string, Resources> Resources::reserved() const
 {
   hashmap<string, Resources> result;
@@ -689,7 +701,7 @@ Option<Value::Ranges> Resources::ephemeral_ports() const
 }
 
 
-bool Resources::contains(const Resource& that) const
+bool Resources::_contains(const Resource& that) const
 {
   foreach (const Resource& resource, resources) {
     if (mesos::contains(resource, that)) {