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;

Reply via email to