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

[1/6] mesos git commit: Fixed a few places to use `foreachkey` / `foreachvalue`.

Repository: mesos
Updated Branches:
  refs/heads/master 51b151fbe -> c5c5c13de


Fixed a few places to use `foreachkey` / `foreachvalue`.

This is preferrable to doing `foreach` over `hashmap::keys()` or
`hashmap::values()`, respectively.

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


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

Branch: refs/heads/master
Commit: c08eb309cfccf769ea5ed65963d094c58e8f0e4e
Parents: 51b151f
Author: Neil Conway <ne...@gmail.com>
Authored: Thu Dec 15 05:36:53 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Thu Dec 15 05:36:53 2016 -0800

----------------------------------------------------------------------
 src/launcher/default_executor.cpp   | 2 +-
 src/slave/containerizer/fetcher.cpp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/c08eb309/src/launcher/default_executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/default_executor.cpp b/src/launcher/default_executor.cpp
index 6e377d4..2ecf1ed 100644
--- a/src/launcher/default_executor.cpp
+++ b/src/launcher/default_executor.cpp
@@ -119,7 +119,7 @@ public:
 
     // Disconnect all active connections used for waiting on child
     // containers.
-    foreach (Connection connection, waiting.values()) {
+    foreachvalue (Connection connection, waiting) {
       connection.disconnect();
     }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/c08eb309/src/slave/containerizer/fetcher.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/fetcher.cpp b/src/slave/containerizer/fetcher.cpp
index 40b247a..94cfa0e 100644
--- a/src/slave/containerizer/fetcher.cpp
+++ b/src/slave/containerizer/fetcher.cpp
@@ -277,7 +277,7 @@ void Fetcher::kill(const ContainerID& containerId)
 
 FetcherProcess::~FetcherProcess()
 {
-  foreach (const ContainerID& containerId, subprocessPids.keys()) {
+  foreachkey (const ContainerID& containerId, subprocessPids) {
     kill(containerId);
   }
 }


[6/6] mesos git commit: Removed unused header include from stout's `hashmap`.

Posted by mp...@apache.org.
Removed unused header include from stout's `hashmap`.

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


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

Branch: refs/heads/master
Commit: c5c5c13deab834e6db7e1f9d687b8cc0f6a0641f
Parents: bbf1370
Author: Neil Conway <ne...@gmail.com>
Authored: Thu Dec 15 05:38:33 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Thu Dec 15 05:38:33 2016 -0800

----------------------------------------------------------------------
 3rdparty/stout/include/stout/hashmap.hpp | 2 --
 1 file changed, 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/c5c5c13d/3rdparty/stout/include/stout/hashmap.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/hashmap.hpp b/3rdparty/stout/include/stout/hashmap.hpp
index a1bebcb..539bbfd 100644
--- a/3rdparty/stout/include/stout/hashmap.hpp
+++ b/3rdparty/stout/include/stout/hashmap.hpp
@@ -19,8 +19,6 @@
 #include <unordered_map>
 #include <utility>
 
-#include <boost/get_pointer.hpp>
-
 #include "foreach.hpp"
 #include "hashset.hpp"
 #include "none.hpp"


[4/6] mesos git commit: Modernized code to use `foreachpair` with `LinkedHashMap`.

Posted by mp...@apache.org.
Modernized code to use `foreachpair` with `LinkedHashMap`.

Various places were doing `foreach` over `LinkedHashMap::keys()` or
`LinkedHashMap::values()`; this can now be replaced with `foreachkey`
and `foreachvalue`, respectively.

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


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

Branch: refs/heads/master
Commit: 352a9d582aff07982cc5376525bebe18f8c886a9
Parents: 1a2154d
Author: Neil Conway <ne...@gmail.com>
Authored: Thu Dec 15 05:38:15 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Thu Dec 15 05:38:15 2016 -0800

----------------------------------------------------------------------
 src/examples/long_lived_executor.cpp |  4 +-
 src/examples/test_http_executor.cpp  |  4 +-
 src/exec/exec.cpp                    |  8 +---
 src/hook/manager.cpp                 | 47 +++++++++-------------
 src/launcher/default_executor.cpp    | 16 ++++----
 src/launcher/executor.cpp            |  2 +-
 src/master/master.cpp                |  5 ++-
 src/slave/http.cpp                   | 14 +++----
 src/slave/slave.cpp                  | 66 ++++++++++++-------------------
 9 files changed, 69 insertions(+), 97 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/352a9d58/src/examples/long_lived_executor.cpp
----------------------------------------------------------------------
diff --git a/src/examples/long_lived_executor.cpp b/src/examples/long_lived_executor.cpp
index 5c43ee7..319db93 100644
--- a/src/examples/long_lived_executor.cpp
+++ b/src/examples/long_lived_executor.cpp
@@ -159,12 +159,12 @@ protected:
     Call::Subscribe* subscribe = call.mutable_subscribe();
 
     // Send all unacknowledged updates.
-    foreach (const Call::Update& update, updates.values()) {
+    foreachvalue (const Call::Update& update, updates) {
       subscribe->add_unacknowledged_updates()->MergeFrom(update);
     }
 
     // Send all unacknowledged tasks.
-    foreach (const TaskInfo& task, tasks.values()) {
+    foreachvalue (const TaskInfo& task, tasks) {
       subscribe->add_unacknowledged_tasks()->MergeFrom(task);
     }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/352a9d58/src/examples/test_http_executor.cpp
----------------------------------------------------------------------
diff --git a/src/examples/test_http_executor.cpp b/src/examples/test_http_executor.cpp
index b9b6bfd..2166faf 100644
--- a/src/examples/test_http_executor.cpp
+++ b/src/examples/test_http_executor.cpp
@@ -84,12 +84,12 @@ public:
     Call::Subscribe* subscribe = call.mutable_subscribe();
 
     // Send all unacknowledged updates.
-    foreach (const Call::Update& update, updates.values()) {
+    foreachvalue (const Call::Update& update, updates) {
       subscribe->add_unacknowledged_updates()->MergeFrom(update);
     }
 
     // Send all unacknowledged tasks.
-    foreach (const TaskInfo& task, tasks.values()) {
+    foreachvalue (const TaskInfo& task, tasks) {
       subscribe->add_unacknowledged_tasks()->MergeFrom(task);
     }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/352a9d58/src/exec/exec.cpp
----------------------------------------------------------------------
diff --git a/src/exec/exec.cpp b/src/exec/exec.cpp
index 1dc2039..95c2e19 100644
--- a/src/exec/exec.cpp
+++ b/src/exec/exec.cpp
@@ -292,16 +292,12 @@ protected:
     message.mutable_framework_id()->MergeFrom(frameworkId);
 
     // Send all unacknowledged updates.
-    // TODO(vinod): Use foreachvalue instead once LinkedHashmap
-    // supports it.
-    foreach (const StatusUpdate& update, updates.values()) {
+    foreachvalue (const StatusUpdate& update, updates) {
       message.add_updates()->MergeFrom(update);
     }
 
     // Send all unacknowledged tasks.
-    // TODO(vinod): Use foreachvalue instead once LinkedHashmap
-    // supports it.
-    foreach (const TaskInfo& task, tasks.values()) {
+    foreachvalue (const TaskInfo& task, tasks) {
       message.add_tasks()->MergeFrom(task);
     }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/352a9d58/src/hook/manager.cpp
----------------------------------------------------------------------
diff --git a/src/hook/manager.cpp b/src/hook/manager.cpp
index e9ca386..82b6a46 100644
--- a/src/hook/manager.cpp
+++ b/src/hook/manager.cpp
@@ -118,8 +118,7 @@ Labels HookManager::masterLaunchTaskLabelDecorator(
     // will be the only effective hook setting the labels.
     TaskInfo taskInfo_ = taskInfo;
 
-    foreach (const string& name, availableHooks.keys()) {
-      Hook* hook = availableHooks[name];
+    foreachpair (const string& name, Hook* hook, availableHooks) {
       const Result<Labels> result =
         hook->masterLaunchTaskLabelDecorator(
             taskInfo_,
@@ -143,8 +142,7 @@ Labels HookManager::masterLaunchTaskLabelDecorator(
 
 void HookManager::masterSlaveLostHook(const SlaveInfo& slaveInfo)
 {
-  foreach (const string& name, availableHooks.keys()) {
-    Hook* hook = availableHooks[name];
+  foreachpair (const string& name, Hook* hook, availableHooks) {
     Try<Nothing> result = hook->masterSlaveLostHook(slaveInfo);
     if (result.isError()) {
       LOG(WARNING) << "Master agent-lost hook failed for module '"
@@ -163,8 +161,7 @@ Labels HookManager::slaveRunTaskLabelDecorator(
   synchronized (mutex) {
     TaskInfo taskInfo_ = taskInfo;
 
-    foreach (const string& name, availableHooks.keys()) {
-      Hook* hook = availableHooks[name];
+    foreachpair (const string& name, Hook* hook, availableHooks) {
       const Result<Labels> result = hook->slaveRunTaskLabelDecorator(
           taskInfo_, executorInfo, frameworkInfo, slaveInfo);
 
@@ -187,8 +184,7 @@ Environment HookManager::slaveExecutorEnvironmentDecorator(
     ExecutorInfo executorInfo)
 {
   synchronized (mutex) {
-    foreach (const string& name, availableHooks.keys()) {
-      Hook* hook = availableHooks[name];
+    foreachpair (const string& name, Hook* hook, availableHooks) {
       const Result<Environment> result =
         hook->slaveExecutorEnvironmentDecorator(executorInfo);
 
@@ -222,9 +218,7 @@ Future<DockerTaskExecutorPrepareInfo>
   // (the last hook takes priority).
   list<Future<Option<DockerTaskExecutorPrepareInfo>>> futures;
 
-  foreach (const string& name, availableHooks.keys()) {
-    Hook* hook = availableHooks[name];
-
+  foreachvalue (Hook* hook, availableHooks) {
     // Chain together each hook.
     futures.push_back(
         hook->slavePreLaunchDockerTaskExecutorDecorator(
@@ -256,8 +250,7 @@ void HookManager::slavePostFetchHook(
     const ContainerID& containerId,
     const string& directory)
 {
-  foreach (const string& name, availableHooks.keys()) {
-    Hook* hook = availableHooks[name];
+  foreachpair (const string& name, Hook* hook, availableHooks) {
     Try<Nothing> result = hook->slavePostFetchHook(containerId, directory);
     if (result.isError()) {
       LOG(WARNING) << "Agent post fetch hook failed for module "
@@ -271,8 +264,7 @@ void HookManager::slaveRemoveExecutorHook(
     const FrameworkInfo& frameworkInfo,
     const ExecutorInfo& executorInfo)
 {
-  foreach (const string& name, availableHooks.keys()) {
-    Hook* hook = availableHooks[name];
+  foreachpair (const string& name, Hook* hook, availableHooks) {
     const Try<Nothing> result =
       hook->slaveRemoveExecutorHook(frameworkInfo, executorInfo);
     if (result.isError()) {
@@ -288,8 +280,7 @@ TaskStatus HookManager::slaveTaskStatusDecorator(
     TaskStatus status)
 {
   synchronized (mutex) {
-    foreach (const string& name, availableHooks.keys()) {
-      Hook* hook = availableHooks[name];
+    foreachpair (const string& name, Hook* hook, availableHooks) {
       const Result<TaskStatus> result =
         hook->slaveTaskStatusDecorator(frameworkId, status);
 
@@ -317,15 +308,14 @@ TaskStatus HookManager::slaveTaskStatusDecorator(
 Resources HookManager::slaveResourcesDecorator(
     const SlaveInfo& slaveInfo)
 {
-  // We need a mutable copy of the Resources object. Each hook will see the
-  // changes made by previous hooks, so the order of execution matters. The
-  // execution order is currently unspecified since availableHooks uses a
-  // hashmap.
+  // We need a mutable copy of the Resources object. Each hook will
+  // see the changes made by previous hooks, so the order of execution
+  // matters. Hooks are executed in the order they are specified by
+  // the user (note that `availableHooks` is a LinkedHashMap).
   SlaveInfo slaveInfo_ = slaveInfo;
 
   synchronized (mutex) {
-    foreach (const string& name, availableHooks.keys()) {
-      Hook* hook = availableHooks[name];
+    foreachpair (const string& name, Hook* hook, availableHooks) {
       const Result<Resources> result =
         hook->slaveResourcesDecorator(slaveInfo_);
 
@@ -345,15 +335,14 @@ Resources HookManager::slaveResourcesDecorator(
 Attributes HookManager::slaveAttributesDecorator(
     const SlaveInfo& slaveInfo)
 {
-  // We need a mutable copy of the Attributes object. Each hook will see the
-  // changes made by previous hooks, so the order of execution matters. The
-  // execution order is currently unspecified since availableHooks uses a
-  // hashmap.
+  // We need a mutable copy of the Attributes object. Each hook will
+  // see the changes made by previous hooks, so the order of execution
+  // matters. Hooks are executed in the order they are specified by
+  // the user (note that `availableHooks` is a LinkedHashMap).
   SlaveInfo slaveInfo_ = slaveInfo;
 
   synchronized (mutex) {
-    foreach (const string& name, availableHooks.keys()) {
-      Hook* hook = availableHooks[name];
+    foreachpair (const string& name, Hook* hook, availableHooks) {
       const Result<Attributes> result =
         hook->slaveAttributesDecorator(slaveInfo_);
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/352a9d58/src/launcher/default_executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/default_executor.cpp b/src/launcher/default_executor.cpp
index 2ecf1ed..fd99639 100644
--- a/src/launcher/default_executor.cpp
+++ b/src/launcher/default_executor.cpp
@@ -231,12 +231,12 @@ protected:
     Call::Subscribe* subscribe = call.mutable_subscribe();
 
     // Send all unacknowledged updates.
-    foreach (const Call::Update& update, updates.values()) {
+    foreachvalue (const Call::Update& update, updates) {
       subscribe->add_unacknowledged_updates()->MergeFrom(update);
     }
 
     // Send the unacknowledged tasks.
-    foreach (const TaskInfo& task, tasks.values()) {
+    foreachvalue (const TaskInfo& task, tasks) {
       subscribe->add_unacknowledged_tasks()->MergeFrom(task);
     }
 
@@ -498,7 +498,7 @@ protected:
     list<Connection> connections = _connections.get();
     CHECK_EQ(containers.size(), connections.size());
 
-    foreach (const ContainerID& containerId, containers.keys()) {
+    foreachkey (const ContainerID& containerId, containers) {
       CHECK(!waiting.contains(containerId));
 
       __wait(connectionId.get(),
@@ -735,7 +735,7 @@ protected:
     }
 
     list<Future<Nothing>> killing;
-    foreach (const ContainerID& containerId, containers.keys()) {
+    foreachkey (const ContainerID& containerId, containers) {
       killing.push_back(kill(connection.get(), containerId));
     }
 
@@ -809,7 +809,7 @@ protected:
     LOG(INFO) << "Received kill for task '" << taskId << "'";
 
     bool found = false;
-    foreach (const TaskID& taskId_, containers.values()) {
+    foreachvalue (const TaskID& taskId_, containers) {
       if (taskId_ == taskId) {
         found = true;
         break;
@@ -876,8 +876,10 @@ private:
     // Fill the container ID associated with this task.
     // TODO(jieyu): Consider maintain a hashmap between TaskID to
     // ContainerID so that we don't have to loop through all tasks.
-    foreach (const ContainerID& containerId, containers.keys()) {
-      if (containers[containerId] == taskId) {
+    foreachpair (const ContainerID& containerId,
+                 const TaskID& containerTaskId,
+                 containers) {
+      if (containerTaskId == taskId) {
         ContainerStatus* containerStatus = status.mutable_container_status();
         containerStatus->mutable_container_id()->CopyFrom(containerId);
         break;

http://git-wip-us.apache.org/repos/asf/mesos/blob/352a9d58/src/launcher/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
index cd5aa46..cc9adfe 100644
--- a/src/launcher/executor.cpp
+++ b/src/launcher/executor.cpp
@@ -325,7 +325,7 @@ protected:
     Call::Subscribe* subscribe = call.mutable_subscribe();
 
     // Send all unacknowledged updates.
-    foreach (const Call::Update& update, updates.values()) {
+    foreachvalue (const Call::Update& update, updates) {
       subscribe->add_unacknowledged_updates()->MergeFrom(update);
     }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/352a9d58/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 8c1c7f9..9023a5f 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -1727,7 +1727,9 @@ void Master::doRegistryGc()
   TimeInfo currentTime = protobuf::getCurrentTime();
   hashset<SlaveID> toRemove;
 
-  foreach (const SlaveID& slave, slaves.unreachable.keys()) {
+  foreachpair (const SlaveID& slave,
+               const TimeInfo& unreachableTime,
+               slaves.unreachable) {
     // Count-based GC.
     CHECK(toRemove.size() <= unreachableCount);
 
@@ -1738,7 +1740,6 @@ void Master::doRegistryGc()
     }
 
     // Age-based GC.
-    const TimeInfo& unreachableTime = slaves.unreachable[slave];
     Duration age = Nanoseconds(
         currentTime.nanoseconds() - unreachableTime.nanoseconds());
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/352a9d58/src/slave/http.cpp
----------------------------------------------------------------------
diff --git a/src/slave/http.cpp b/src/slave/http.cpp
index ecec24a..e1cea46 100644
--- a/src/slave/http.cpp
+++ b/src/slave/http.cpp
@@ -173,7 +173,7 @@ struct ExecutorWriter
     }
 
     writer->field("tasks", [this](JSON::ArrayWriter* writer) {
-      foreach (Task* task, executor_->launchedTasks.values()) {
+      foreachvalue (Task* task, executor_->launchedTasks) {
         if (!approveViewTask(taskApprover_, *task, framework_->info)) {
           continue;
         }
@@ -183,7 +183,7 @@ struct ExecutorWriter
     });
 
     writer->field("queued_tasks", [this](JSON::ArrayWriter* writer) {
-      foreach (const TaskInfo& task, executor_->queuedTasks.values()) {
+      foreachvalue (const TaskInfo& task, executor_->queuedTasks) {
         if (!approveViewTaskInfo(taskApprover_, task, framework_->info)) {
           continue;
         }
@@ -203,9 +203,7 @@ struct ExecutorWriter
 
       // NOTE: We add 'terminatedTasks' to 'completed_tasks' for
       // simplicity.
-      // TODO(vinod): Use foreachvalue instead once LinkedHashmap
-      // supports it.
-      foreach (Task* task, executor_->terminatedTasks.values()) {
+      foreachvalue (Task* task, executor_->terminatedTasks) {
         if (!approveViewTask(taskApprover_, *task, framework_->info)) {
           continue;
         }
@@ -1569,7 +1567,7 @@ agent::Response::GetTasks Slave::Http::_getTasks(
                const Framework* framework,
                executors) {
     // Queued tasks.
-    foreach (const TaskInfo& taskInfo, executor->queuedTasks.values()) {
+    foreachvalue (const TaskInfo& taskInfo, executor->queuedTasks) {
       // Skip unauthorized tasks.
       if (!approveViewTaskInfo(tasksApprover, taskInfo, framework->info)) {
         continue;
@@ -1582,7 +1580,7 @@ agent::Response::GetTasks Slave::Http::_getTasks(
     }
 
     // Launched tasks.
-    foreach (Task* task, executor->launchedTasks.values()) {
+    foreachvalue (Task* task, executor->launchedTasks) {
       CHECK_NOTNULL(task);
       // Skip unauthorized tasks.
       if (!approveViewTask(tasksApprover, *task, framework->info)) {
@@ -1593,7 +1591,7 @@ agent::Response::GetTasks Slave::Http::_getTasks(
     }
 
     // Terminated tasks.
-    foreach (Task* task, executor->terminatedTasks.values()) {
+    foreachvalue (Task* task, executor->terminatedTasks) {
       CHECK_NOTNULL(task);
       // Skip unauthorized tasks.
       if (!approveViewTask(tasksApprover, *task, framework->info)) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/352a9d58/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 673f44b..68bdaa0 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -1407,15 +1407,15 @@ void Slave::doReliableRegistration(Duration maxBackoff)
         // unacknowledged tasks.
         // Note that for each task the latest state and status update
         // state (if any) is also included.
-        foreach (Task* task, executor->launchedTasks.values()) {
+        foreachvalue (Task* task, executor->launchedTasks) {
           message.add_tasks()->CopyFrom(*task);
         }
 
-        foreach (Task* task, executor->terminatedTasks.values()) {
+        foreachvalue (Task* task, executor->terminatedTasks) {
           message.add_tasks()->CopyFrom(*task);
         }
 
-        foreach (const TaskInfo& task, executor->queuedTasks.values()) {
+        foreachvalue (const TaskInfo& task, executor->queuedTasks) {
           message.add_tasks()->CopyFrom(protobuf::createTask(
               task, TASK_STAGING, framework->id()));
         }
@@ -1466,7 +1466,7 @@ void Slave::doReliableRegistration(Duration maxBackoff)
                 << " terminated tasks, " << executor->completedTasks.size()
                 << " completed tasks";
 
-        foreach (const Task* task, executor->terminatedTasks.values()) {
+        foreachvalue (const Task* task, executor->terminatedTasks) {
           VLOG(2) << "Reregistering terminated task " << task->task_id();
           completedFramework_->add_tasks()->CopyFrom(*task);
         }
@@ -2068,9 +2068,7 @@ void Slave::_run(
       // upcoming tasks.
       Resources resources = executor->resources;
 
-      // TODO(jieyu): Use foreachvalue instead once LinkedHashmap
-      // supports it.
-      foreach (const TaskInfo& task, executor->queuedTasks.values()) {
+      foreachvalue (const TaskInfo& task, executor->queuedTasks) {
         resources += task.resources();
       }
 
@@ -3214,9 +3212,7 @@ void Slave::subscribe(
       // upcoming tasks.
       Resources resources = executor->resources;
 
-      // TODO(jieyu): Use foreachvalue instead once LinkedHashmap
-      // supports it.
-      foreach (const TaskInfo& task, executor->queuedTasks.values()) {
+      foreachvalue (const TaskInfo& task, executor->queuedTasks) {
         resources += task.resources();
       }
 
@@ -3224,8 +3220,10 @@ void Slave::subscribe(
       // `queuedTasks`. Hence, we need to ensure that we don't send the same
       // tasks to the executor twice.
       LinkedHashMap<TaskID, TaskInfo> queuedTasks;
-      foreach (const TaskID& taskId, executor->queuedTasks.keys()) {
-        queuedTasks[taskId] = executor->queuedTasks[taskId];
+      foreachpair (const TaskID& taskId,
+                   const TaskInfo& taskInfo,
+                   executor->queuedTasks) {
+        queuedTasks[taskId] = taskInfo;
       }
 
       foreach (const TaskGroupInfo& taskGroup, executor->queuedTaskGroups) {
@@ -3263,10 +3261,7 @@ void Slave::subscribe(
       // TODO(vinod): Consider checkpointing 'TaskInfo' instead of
       // 'Task' so that we can relaunch such tasks! Currently we don't
       // do it because 'TaskInfo.data' could be huge.
-      //
-      // TODO(vinod): Use foreachvalue instead once LinkedHashmap
-      // supports it.
-      foreach (Task* task, executor->launchedTasks.values()) {
+      foreachvalue (Task* task, executor->launchedTasks) {
         if (task->state() == TASK_STAGING &&
             !unackedTasks.contains(task->task_id())) {
           mesos::TaskState newTaskState = TASK_DROPPED;
@@ -3439,9 +3434,7 @@ void Slave::registerExecutor(
       // upcoming tasks.
       Resources resources = executor->resources;
 
-      // TODO(jieyu): Use foreachvalue instead once LinkedHashmap
-      // supports it.
-      foreach (const TaskInfo& task, executor->queuedTasks.values()) {
+      foreachvalue (const TaskInfo& task, executor->queuedTasks) {
         resources += task.resources();
       }
 
@@ -3449,8 +3442,10 @@ void Slave::registerExecutor(
       // `queuedTasks`. Hence, we need to ensure that we don't send the same
       // tasks to the executor twice.
       LinkedHashMap<TaskID, TaskInfo> queuedTasks;
-      foreach (const TaskID& taskId, executor->queuedTasks.keys()) {
-        queuedTasks[taskId] = executor->queuedTasks[taskId];
+      foreachpair (const TaskID& taskId,
+                   const TaskInfo& taskInfo,
+                   executor->queuedTasks) {
+        queuedTasks[taskId] = taskInfo;
       }
 
       foreach (const TaskGroupInfo& taskGroup, executor->queuedTaskGroups) {
@@ -3583,10 +3578,7 @@ void Slave::reregisterExecutor(
       // TODO(vinod): Consider checkpointing 'TaskInfo' instead of
       // 'Task' so that we can relaunch such tasks! Currently we
       // don't do it because 'TaskInfo.data' could be huge.
-      //
-      // TODO(vinod): Use foreachvalue instead once LinkedHashmap
-      // supports it.
-      foreach (Task* task, executor->launchedTasks.values()) {
+      foreachvalue (Task* task, executor->launchedTasks) {
         if (task->state() == TASK_STAGING &&
             !unackedTasks.contains(task->task_id())) {
           mesos::TaskState newTaskState = TASK_DROPPED;
@@ -4722,9 +4714,7 @@ void Slave::executorTerminated(
       // status update streams for a framework that is terminating.
       if (framework->state != Framework::TERMINATING) {
         // Transition all live launched tasks.
-        // TODO(vinod): Use foreachvalue instead once LinkedHashmap
-        // supports it.
-        foreach (Task* task, executor->launchedTasks.values()) {
+        foreachvalue (Task* task, executor->launchedTasks) {
           if (!protobuf::isTerminalState(task->state())) {
             sendExecutorTerminatedStatusUpdate(
                 task->task_id(), termination, frameworkId, executor);
@@ -4732,9 +4722,7 @@ void Slave::executorTerminated(
         }
 
         // Transition all queued tasks.
-        // TODO(vinod): Use foreachvalue instead once LinkedHashmap
-        // supports it.
-        foreach (const TaskInfo& task, executor->queuedTasks.values()) {
+        foreachvalue (const TaskInfo& task, executor->queuedTasks) {
           sendExecutorTerminatedStatusUpdate(
               task.task_id(), termination, frameworkId, executor);
         }
@@ -5827,7 +5815,7 @@ Future<ResourceUsage> Slave::usage()
       entry->mutable_container_id()->CopyFrom(executor->containerId);
 
       // We include non-terminal tasks in ResourceUsage.
-      foreach (const Task* task, executor->launchedTasks.values()) {
+      foreachvalue (const Task* task, executor->launchedTasks) {
         ResourceUsage::Executor::Task* t = entry->add_tasks();
         t->set_name(task->name());
         t->mutable_id()->CopyFrom(task->task_id());
@@ -5885,7 +5873,7 @@ double Slave::_tasks_staging()
     foreachvalue (Executor* executor, framework->executors) {
       count += executor->queuedTasks.size();
 
-      foreach (Task* task, executor->launchedTasks.values()) {
+      foreachvalue (Task* task, executor->launchedTasks) {
         if (task->state() == TASK_STAGING) {
           count++;
         }
@@ -5901,7 +5889,7 @@ double Slave::_tasks_starting()
   double count = 0.0;
   foreachvalue (Framework* framework, frameworks) {
     foreachvalue (Executor* executor, framework->executors) {
-      foreach (Task* task, executor->launchedTasks.values()) {
+      foreachvalue (Task* task, executor->launchedTasks) {
         if (task->state() == TASK_STARTING) {
           count++;
         }
@@ -5917,7 +5905,7 @@ double Slave::_tasks_running()
   double count = 0.0;
   foreachvalue (Framework* framework, frameworks) {
     foreachvalue (Executor* executor, framework->executors) {
-      foreach (Task* task, executor->launchedTasks.values()) {
+      foreachvalue (Task* task, executor->launchedTasks) {
         if (task->state() == TASK_RUNNING) {
           count++;
         }
@@ -5933,7 +5921,7 @@ double Slave::_tasks_killing()
   double count = 0.0;
   foreachvalue (Framework* framework, frameworks) {
     foreachvalue (Executor* executor, framework->executors) {
-      foreach (Task* task, executor->launchedTasks.values()) {
+      foreachvalue (Task* task, executor->launchedTasks) {
         if (task->state() == TASK_KILLING) {
           count++;
         }
@@ -6686,12 +6674,10 @@ Executor::~Executor()
   }
 
   // Delete the tasks.
-  // TODO(vinod): Use foreachvalue instead once LinkedHashmap
-  // supports it.
-  foreach (Task* task, launchedTasks.values()) {
+  foreachvalue (Task* task, launchedTasks) {
     delete task;
   }
-  foreach (Task* task, terminatedTasks.values()) {
+  foreachvalue (Task* task, terminatedTasks) {
     delete task;
   }
 }


[2/6] mesos git commit: Changed implementation of `LinkedHashMap`.

Posted by mp...@apache.org.
Changed implementation of `LinkedHashMap`.

`LinkedHashMap<Key, Value>` used two containers: a `std::list<Key>` and
a `hashmap<Key, std::pair<Value, std::list<Key>::iterator>>`. That
approach worked fine, but it made it difficult to support iterating over
the key-value pairs in the map without jumping through hoops or copying.

Instead, this commit uses a `std::list<std::pair<Key, Value>>` and a
`hashmap<Key, std::list<std::pair<Key, Value>>::iterator>`. This makes
it easy to support iterating over the entries in the map in insertion
order: we can just iterate over the list.

This commit just changes the implementation of LinkedHashMap; support
for iteration will be added in a subsequent commit.

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


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

Branch: refs/heads/master
Commit: 67959980bdd970c0408dd643456c45cad63aa375
Parents: c08eb30
Author: Neil Conway <ne...@gmail.com>
Authored: Thu Dec 15 05:37:06 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Thu Dec 15 05:37:06 2016 -0800

----------------------------------------------------------------------
 3rdparty/stout/include/stout/linkedhashmap.hpp | 68 +++++++++++++--------
 3rdparty/stout/tests/linkedhashmap_tests.cpp   |  9 +--
 2 files changed, 43 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/67959980/3rdparty/stout/include/stout/linkedhashmap.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/linkedhashmap.hpp b/3rdparty/stout/include/stout/linkedhashmap.hpp
index f48cc59..43a19d7 100644
--- a/3rdparty/stout/include/stout/linkedhashmap.hpp
+++ b/3rdparty/stout/include/stout/linkedhashmap.hpp
@@ -16,76 +16,92 @@
 #include <list>
 #include <utility>
 
+#include <stout/foreach.hpp>
 #include <stout/hashmap.hpp>
 #include <stout/option.hpp>
 
-// Implementation of a hashmap that maintains the insertion order
-// of the keys. Note that re-insertion of a key (i.e., update)
-// doesn't update its insertion order.
+// Implementation of a hashmap that maintains the insertion order of
+// the keys. Updating a key does not change insertion order.
+//
 // TODO(vinod/bmahler): Consider extending from stout::hashmap and/or
 // having a compatible API with stout::hashmap.
 template <typename Key, typename Value>
 class LinkedHashMap
 {
 public:
-  typedef std::list<Key> list;
-  typedef hashmap<Key, std::pair<Value, typename list::iterator>> map;
+  typedef std::pair<Key, Value> entry;
+  typedef std::list<entry> list;
+  typedef hashmap<Key, typename list::iterator> map;
 
   Value& operator[] (const Key& key)
   {
-    if (!values_.contains(key)) {
-      // Insert into the list and get the "pointer" into the list.
-      typename list::iterator i = keys_.insert(keys_.end(), key);
-      values_[key] = std::make_pair(Value(), i); // Store default value.
+    if (!keys_.contains(key)) {
+      // Insert a new entry into the list and get a "pointer" to its
+      // location. The initial value is default-constructed.
+      typename list::iterator iter =
+        entries_.insert(entries_.end(), std::make_pair(key, Value()));
+
+      keys_[key] = iter;
     }
-    return values_[key].first;
+
+    return keys_[key]->second;
   }
 
   Option<Value> get(const Key& key) const
   {
-    if (values_.contains(key)) {
-      return values_.at(key).first;
+    if (keys_.contains(key)) {
+      return keys_.at(key)->second;
     }
     return None();
   }
 
   Value& at(const Key& key)
   {
-    return values_.at(key).first;
+    return keys_.at(key)->second;
   }
 
   const Value& at(const Key& key) const
   {
-    return values_.at(key).first;
+    return keys_.at(key)->second;
   }
 
   bool contains(const Key& key) const
   {
-    return values_.contains(key);
+    return keys_.contains(key);
   }
 
   size_t erase(const Key& key)
   {
-    if (values_.contains(key)) {
-      // Get the "pointer" into the list.
-      typename list::iterator i = values_[key].second;
-      keys_.erase(i);
-      return values_.erase(key);
+    if (keys_.contains(key)) {
+      typename list::iterator iter = keys_[key];
+      keys_.erase(key);
+      entries_.erase(iter);
+      return 1;
     }
     return 0;
   }
 
+  // Returns the keys in the map in insertion order.
   std::list<Key> keys() const
   {
-    return keys_;
+    std::list<Key> result;
+
+    foreach (const entry& entry, entries_) {
+      result.push_back(entry.first);
+    }
+
+    return result;
   }
 
+  // Returns the values in the map in insertion order.
   std::list<Value> values() const
   {
     std::list<Value> result;
-    foreach (const Key& key, keys_) {
-      result.push_back(values_.at(key).first);
+
+    foreach (const entry& entry, entries_) {
+      result.push_back(entry.second);
     }
+
     return result;
   }
 
@@ -101,13 +117,13 @@ public:
 
   void clear()
   {
-    values_.clear();
+    entries_.clear();
     keys_.clear();
   }
 
 private:
-  list keys_;  // Keys ordered by the insertion order.
-  map values_;  // Map of values and "pointers" to the linked list.
+  list entries_; // Key-value pairs ordered by insertion order.
+  map keys_; // Map from key to "pointer" to key's location in list.
 };
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/67959980/3rdparty/stout/tests/linkedhashmap_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/linkedhashmap_tests.cpp b/3rdparty/stout/tests/linkedhashmap_tests.cpp
index 376462d..6bc5345 100644
--- a/3rdparty/stout/tests/linkedhashmap_tests.cpp
+++ b/3rdparty/stout/tests/linkedhashmap_tests.cpp
@@ -10,8 +10,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License
 
-#include <stdint.h>
-
 #include <list>
 #include <string>
 
@@ -72,12 +70,7 @@ TEST(LinkedHashmapTest, Keys)
 {
   LinkedHashMap<string, int> map;
 
-  list<string> keys;
-  keys.push_back("foo");
-  keys.push_back("bar");
-  keys.push_back("food");
-  keys.push_back("rad");
-  keys.push_back("cat");
+  list<string> keys = {"foo", "bar", "food", "rad", "cat"};
 
   // Insert keys into the map.
   foreach (const string& key, keys) {


[5/6] mesos git commit: Updated stout's `README`.

Posted by mp...@apache.org.
Updated stout's `README`.

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


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

Branch: refs/heads/master
Commit: bbf1370ad02e26a556787cd8fb084d12e0cf4420
Parents: 352a9d5
Author: Neil Conway <ne...@gmail.com>
Authored: Thu Dec 15 05:38:26 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Thu Dec 15 05:38:26 2016 -0800

----------------------------------------------------------------------
 3rdparty/stout/README.md | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/bbf1370a/3rdparty/stout/README.md
----------------------------------------------------------------------
diff --git a/3rdparty/stout/README.md b/3rdparty/stout/README.md
index 80beb44..f4e41aa 100644
--- a/3rdparty/stout/README.md
+++ b/3rdparty/stout/README.md
@@ -435,17 +435,15 @@ There are various ways to deal with unknown flags (i.e., `--baz` in our example
 
 ## Collections
 
-Many of the collections and containers provided either by Boost or the C++ standard are cumbersome to use or yield brittle, hard to maintain code. For many of the standard data structures we provide wrappers with modified interfaces often simplified or enhanced using types like `Try` and `Option`.
+Many of the collections and containers provided either by Boost or the C++ standard are cumbersome to use or yield brittle, hard to maintain code. For many of the standard data structures we provide wrappers with modified interfaces often simplified or enhanced using types like `Try` and `Option`. These wrappers include `hashmap`, `hashset`, `multihashmap`, and `multimap`.
 
 > NOTE: The collections are not namespaced.
 
-Some of these wrappers use the Boost implementations of these data structures, including: `hashmap`, `hashset`, `linkedhashmap`, `multihashmap`, `multimap`.
-
-There is a `Set` abstraction for working with a `std::set` as well as some overloaded operators for doing set union (`|`), set intersection (`&`), and set appending (`+`).
-
-Finally, there is a `cache` implementation (also requires Boost) that provides a templated implementation of a least-recently used (LRU) cache. Note that the key type must be compatible with `boost::unordered_map`.
+`LinkedHashMap` is a hashmap that maintains the order in which the keys have been inserted. This allows both constant-time access to a particular key-value pair, as well as iteration over key-value pairs according to the insertion order.
 
+There is also a `Cache` implementation that provides a templated implementation of a least-recently used (LRU) cache. Note that the key type must be compatible with `std::unordered_map`.
 
+Finally, we provide some overloaded operators for doing set union (`|`), set intersection (`&`), and set appending (`+`) using `std::set`.
 
 <a href="miscellaneous"></a>
 


[3/6] mesos git commit: Enhanced `LinkedHashMap` to support `foreachpair` and friends.

Posted by mp...@apache.org.
Enhanced `LinkedHashMap` to support `foreachpair` and friends.

This requires providing `begin` and `end` member functions that return
iterators in insertion order. With the new implementation of
`LinkedHashMap`, this is easy to do.

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


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

Branch: refs/heads/master
Commit: 1a2154d4f15eedcf426867de7f6a38f28fe9e001
Parents: 6795998
Author: Neil Conway <ne...@gmail.com>
Authored: Thu Dec 15 05:38:05 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Thu Dec 15 05:38:05 2016 -0800

----------------------------------------------------------------------
 3rdparty/stout/include/stout/linkedhashmap.hpp |  9 +++
 3rdparty/stout/tests/linkedhashmap_tests.cpp   | 85 +++++++++++++++++++++
 2 files changed, 94 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/1a2154d4/3rdparty/stout/include/stout/linkedhashmap.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/linkedhashmap.hpp b/3rdparty/stout/include/stout/linkedhashmap.hpp
index 43a19d7..35684e4 100644
--- a/3rdparty/stout/include/stout/linkedhashmap.hpp
+++ b/3rdparty/stout/include/stout/linkedhashmap.hpp
@@ -121,6 +121,15 @@ public:
     keys_.clear();
   }
 
+  // Support for iteration; this allows using `foreachpair` and
+  // related constructs. Note that these iterate over the map in
+  // insertion order.
+  typename list::iterator begin() { return entries_.begin(); }
+  typename list::iterator end() { return entries_.end(); }
+
+  typename list::const_iterator begin() const { return entries_.cbegin(); }
+  typename list::const_iterator end() const { return entries_.cend(); }
+
 private:
   list entries_; // Key-value pairs ordered by insertion order.
   map keys_; // Map from key to "pointer" to key's location in list.

http://git-wip-us.apache.org/repos/asf/mesos/blob/1a2154d4/3rdparty/stout/tests/linkedhashmap_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/linkedhashmap_tests.cpp b/3rdparty/stout/tests/linkedhashmap_tests.cpp
index 6bc5345..267022e 100644
--- a/3rdparty/stout/tests/linkedhashmap_tests.cpp
+++ b/3rdparty/stout/tests/linkedhashmap_tests.cpp
@@ -12,6 +12,7 @@
 
 #include <list>
 #include <string>
+#include <vector>
 
 #include <gtest/gtest.h>
 
@@ -20,6 +21,7 @@
 
 using std::list;
 using std::string;
+using std::vector;
 
 TEST(LinkedHashmapTest, Put)
 {
@@ -97,3 +99,86 @@ TEST(LinkedHashmapTest, Values)
     ASSERT_EQ(val, value);
   }
 }
+
+
+TEST(LinkedHashMapTest, Foreach)
+{
+  LinkedHashMap<string, int> map;
+
+  map["foo"] = 1;
+  map["bar"] = 2;
+  map["caz"] = 3;
+
+  map["foo"] = 4; // Re-insert a key.
+
+  list<string> keyList = map.keys();
+  list<int> valueList = map.values();
+
+  vector<string> keys{keyList.begin(), keyList.end()};
+  vector<int> values{valueList.begin(), valueList.end()};
+
+  {
+    int i = 0;
+    foreachpair (const string& key, int value, map) {
+      EXPECT_EQ(keys[i], key);
+      EXPECT_EQ(values[i], value);
+      i++;
+    }
+  }
+
+  {
+    int i = 0;
+    foreachkey (const string& key, map) {
+      EXPECT_EQ(keys[i], key);
+      i++;
+    }
+  }
+
+  {
+    int i = 0;
+    foreachvalue (int value, map) {
+      EXPECT_EQ(values[i], value);
+      i++;
+    }
+  }
+}
+
+
+// Check that `foreach`-style loops can be used with a const ref to
+// LinkedHashMap.
+TEST(LinkedHashMapTest, ForeachConst)
+{
+  LinkedHashMap<string, int> map;
+
+  map["foo"] = 1;
+  map["bar"] = 2;
+  map["caz"] = 3;
+
+  const LinkedHashMap<string, int>& constMap = map;
+
+  foreachkey (const string& key, constMap) {
+    EXPECT_NE("qux", key);
+  }
+  foreachvalue (int value, constMap) {
+    EXPECT_NE(0, value);
+  }
+}
+
+
+TEST(LinkedHashMapTest, ForeachMutate)
+{
+  LinkedHashMap<int, string> map;
+
+  map[1] = "foo";
+  map[2] = "bar";
+  map[3] = "caz";
+
+  foreachpair (int key, string& value, map) {
+    if (key == 2) {
+      value = "qux";
+    }
+  }
+
+  list<string> values = {"foo", "qux", "caz"};
+  EXPECT_EQ(values, map.values());
+}