This is an automated email from the ASF dual-hosted git repository. bmahler pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
The following commit(s) were added to refs/heads/master by this push: new 3bc6cfa Optimized resources filter operation. 3bc6cfa is described below commit 3bc6cfa5188ec527922e9324958722ff75b11166 Author: Meng Zhu <m...@mesosphere.io> AuthorDate: Fri Oct 19 16:40:07 2018 -0700 Optimized resources filter operation. Currently, `Resources::filter()` operation uses `add()` which scans the resources vector, a O(n) operation. This is not necessary. `filter()` operation should only remove `Resource` entries. This patch optimizes the performance by directly `push_back` the resource to the vector without scanning. Review: https://reviews.apache.org/r/69032/ --- src/common/resources.cpp | 9 ++++++--- src/v1/resources.cpp | 9 ++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/common/resources.cpp b/src/common/resources.cpp index f0f3df5..1585e30 100644 --- a/src/common/resources.cpp +++ b/src/common/resources.cpp @@ -1548,13 +1548,16 @@ Resources Resources::filter( const lambda::function<bool(const Resource&)>& predicate) const { Resources result; + result.resourcesNoMutationWithoutExclusiveOwnership.reserve(this->size()); foreach ( const Resource_Unsafe& resource_, resourcesNoMutationWithoutExclusiveOwnership) { if (predicate(resource_->resource)) { - // TODO(mzhu): `add` is O(n), explore whether we can simply - // do `push_back` by assuming resources are already combined. - result.add(resource_); + // We `push_back()` here instead of `add()` (which is O(n)). `add()` is + // not necessary because we assume all Resource objects are already + // combined in `Resources` and `filter()` should only take away + // resource objects. + result.resourcesNoMutationWithoutExclusiveOwnership.push_back(resource_); } } return result; diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp index 4d2b64f..0248f1d 100644 --- a/src/v1/resources.cpp +++ b/src/v1/resources.cpp @@ -1566,13 +1566,16 @@ Resources Resources::filter( const lambda::function<bool(const Resource&)>& predicate) const { Resources result; + result.resourcesNoMutationWithoutExclusiveOwnership.reserve(this->size()); foreach ( const Resource_Unsafe& resource_, resourcesNoMutationWithoutExclusiveOwnership) { if (predicate(resource_->resource)) { - // TODO(mzhu): `add` is O(n), investigate whether we can simply - // do `push_back` by assuming resources are already combined. - result.add(resource_); + // We `push_back()` here instead of `add()` (which is O(n)). `add()` is + // not necessary because we assume all Resource objects are already + // combined in `Resources` and `filter()` should only take away + // resource objects. + result.resourcesNoMutationWithoutExclusiveOwnership.push_back(resource_); } } return result;