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 <anindya_si...@apple.com> Authored: Wed Aug 3 16:32:52 2016 -0700 Committer: Jiang Yan Xu <xuj...@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;