You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ya...@apache.org on 2016/08/04 00:21:28 UTC
mesos git commit: Improved performance in addition and subtraction of
resources.
Repository: mesos
Updated Branches:
refs/heads/master 9419ed410 -> c4975c99b
Improved performance in addition and subtraction of resources.
Avoid multiple calls to addable and subtractable in the arithmetic
operations in Resources. While adding or subtracting two Resources
objects, check for addable() is done in Resources::add(), and check
for subtractable() is done in Resources::subtract(). Since the
Resource_ class is private and += and -= operators are only called
from within Resources::add() and Resources::subtract(), we do not
need to call addable() or subtractable() again.
This fixes the performance regression introduced by the Resource_ and
now += and -= performance is on a par with the old Resources before
Resource_ was introduced, according to the test
Resources_BENCHMARK_Test.Arithmetic/0.
Review: https://reviews.apache.org/r/50738/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c4975c99
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c4975c99
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c4975c99
Branch: refs/heads/master
Commit: c4975c99b81f7811b3982dd078a87074e517e511
Parents: 9419ed4
Author: Anindya Sinha <an...@apple.com>
Authored: Wed Aug 3 16:32:52 2016 -0700
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Wed Aug 3 17:14:04 2016 -0700
----------------------------------------------------------------------
include/mesos/resources.hpp | 3 +++
include/mesos/v1/resources.hpp | 3 +++
src/common/resources.cpp | 40 ++++++++++++++++++-------------------
src/v1/resources.cpp | 40 ++++++++++++++++++-------------------
4 files changed, 46 insertions(+), 40 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/c4975c99/include/mesos/resources.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/resources.hpp b/include/mesos/resources.hpp
index 829f39d..bdbe8ea 100644
--- a/include/mesos/resources.hpp
+++ b/include/mesos/resources.hpp
@@ -109,8 +109,11 @@ private:
// Checks if this Resource_ is a superset of the given Resource_.
bool contains(const Resource_& that) const;
+ // The arithmetic operators, viz. += and -= assume that the corresponding
+ // Resource objects are addable or subtractable already.
Resource_& operator+=(const Resource_& that);
Resource_& operator-=(const Resource_& that);
+
bool operator==(const Resource_& that) const;
bool operator!=(const Resource_& that) const;
http://git-wip-us.apache.org/repos/asf/mesos/blob/c4975c99/include/mesos/v1/resources.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/v1/resources.hpp b/include/mesos/v1/resources.hpp
index f3c5f31..c05cb63 100644
--- a/include/mesos/v1/resources.hpp
+++ b/include/mesos/v1/resources.hpp
@@ -109,8 +109,11 @@ private:
// Checks if this Resource_ is a superset of the given Resource_.
bool contains(const Resource_& that) const;
+ // The arithmetic operators, viz. += and -= assume that the corresponding
+ // Resource objects are addable or subtractable already.
Resource_& operator+=(const Resource_& that);
Resource_& operator-=(const Resource_& that);
+
bool operator==(const Resource_& that) const;
bool operator!=(const Resource_& that) const;
http://git-wip-us.apache.org/repos/asf/mesos/blob/c4975c99/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index b5d20d6..2470c02 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -859,17 +859,17 @@ bool Resources::Resource_::contains(const Resource_& that) const
Resources::Resource_& Resources::Resource_::operator+=(const Resource_& that)
{
- if (internal::addable(resource, that.resource)) {
- if (!isShared()) {
- resource += that.resource;
- } else {
- // 'addable' makes sure both 'resource' fields are shared and
- // equal, so we just need to sum up the counters here.
- CHECK_SOME(sharedCount);
- CHECK_SOME(that.sharedCount);
+ // This function assumes that the 'resource' fields are addable.
- sharedCount = sharedCount.get() + that.sharedCount.get();
- }
+ if (!isShared()) {
+ resource += that.resource;
+ } else {
+ // 'addable' makes sure both 'resource' fields are shared and
+ // equal, so we just need to sum up the counters here.
+ CHECK_SOME(sharedCount);
+ CHECK_SOME(that.sharedCount);
+
+ sharedCount = sharedCount.get() + that.sharedCount.get();
}
return *this;
@@ -878,17 +878,17 @@ Resources::Resource_& Resources::Resource_::operator+=(const Resource_& that)
Resources::Resource_& Resources::Resource_::operator-=(const Resource_& that)
{
- if (internal::subtractable(resource, that.resource)) {
- if (!isShared()) {
- resource -= that.resource;
- } else {
- // 'subtractable' makes sure both 'resource' fields are shared and
- // equal, so we just need to subtract the counters here.
- CHECK_SOME(sharedCount);
- CHECK_SOME(that.sharedCount);
+ // This function assumes that the 'resource' fields are subtractable.
- sharedCount = sharedCount.get() - that.sharedCount.get();
- }
+ if (!isShared()) {
+ resource -= that.resource;
+ } else {
+ // 'subtractable' makes sure both 'resource' fields are shared and
+ // equal, so we just need to subtract the counters here.
+ CHECK_SOME(sharedCount);
+ CHECK_SOME(that.sharedCount);
+
+ sharedCount = sharedCount.get() - that.sharedCount.get();
}
return *this;
http://git-wip-us.apache.org/repos/asf/mesos/blob/c4975c99/src/v1/resources.cpp
----------------------------------------------------------------------
diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp
index 71eedb2..6c4e3b2 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -862,17 +862,17 @@ bool Resources::Resource_::contains(const Resource_& that) const
Resources::Resource_& Resources::Resource_::operator+=(const Resource_& that)
{
- if (internal::addable(resource, that.resource)) {
- if (!isShared()) {
- resource += that.resource;
- } else {
- // 'addable' makes sure both 'resource' fields are shared and
- // equal, so we just need to sum up the counters here.
- CHECK_SOME(sharedCount);
- CHECK_SOME(that.sharedCount);
+ // This function assumes that the 'resource' fields are addable.
- sharedCount = sharedCount.get() + that.sharedCount.get();
- }
+ if (!isShared()) {
+ resource += that.resource;
+ } else {
+ // 'addable' makes sure both 'resource' fields are shared and
+ // equal, so we just need to sum up the counters here.
+ CHECK_SOME(sharedCount);
+ CHECK_SOME(that.sharedCount);
+
+ sharedCount = sharedCount.get() + that.sharedCount.get();
}
return *this;
@@ -881,17 +881,17 @@ Resources::Resource_& Resources::Resource_::operator+=(const Resource_& that)
Resources::Resource_& Resources::Resource_::operator-=(const Resource_& that)
{
- if (internal::subtractable(resource, that.resource)) {
- if (!isShared()) {
- resource -= that.resource;
- } else {
- // 'subtractable' makes sure both 'resource' fields are shared and
- // equal, so we just need to subtract the counters here.
- CHECK_SOME(sharedCount);
- CHECK_SOME(that.sharedCount);
+ // This function assumes that the 'resource' fields are subtractable.
- sharedCount = sharedCount.get() - that.sharedCount.get();
- }
+ if (!isShared()) {
+ resource -= that.resource;
+ } else {
+ // 'subtractable' makes sure both 'resource' fields are shared and
+ // equal, so we just need to subtract the counters here.
+ CHECK_SOME(sharedCount);
+ CHECK_SOME(that.sharedCount);
+
+ sharedCount = sharedCount.get() - that.sharedCount.get();
}
return *this;