You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2015/05/03 02:58:40 UTC
[1/2] mesos git commit: Use unrestricted union to remove dynamic
allocation from Option.
Repository: mesos
Updated Branches:
refs/heads/master 30113086a -> 1226e0ad6
Use unrestricted union to remove dynamic allocation from Option.
Refactor `Option<T>` to use unrestricted union to remove dynamic
allocation. This depends on the upgrade to GCC 4.8+.
Note that `std::remove_const<T>::type` was used to remove the `const`
from `T` when storing `T` in the union so that we can properly do
placement new from outside the initializer list for the case
`Option<const T>`.
Review: https://reviews.apache.org/r/32838
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/a5640ad8
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a5640ad8
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a5640ad8
Branch: refs/heads/master
Commit: a5640ad813e6256b548fca068f04fd9fa3a03eda
Parents: 3011308
Author: Joris Van Remoortere <jo...@gmail.com>
Authored: Sat May 2 17:00:23 2015 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sat May 2 17:02:19 2015 -0700
----------------------------------------------------------------------
.../3rdparty/stout/include/stout/option.hpp | 59 ++++++++++----------
1 file changed, 31 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/a5640ad8/3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp
index 47fe92c..ea79b50 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp
@@ -27,50 +27,49 @@ class Option
public:
static Option<T> none()
{
- return Option<T>(NONE);
+ return Option<T>();
}
static Option<T> some(const T& t)
{
- return Option<T>(SOME, new T(t));
+ return Option<T>(t);
}
- Option() : state(NONE), t(NULL) {}
+ Option() : state(NONE) {}
- Option(const T& _t) : state(SOME), t(new T(_t)) {}
+ Option(const T& _t) : state(SOME), t(_t) {}
template <typename U>
- Option(const U& u) : state(SOME), t(new T(u)) {}
+ Option(const U& u) : state(SOME), t(u) {}
- Option(const None& none) : state(NONE), t(NULL) {}
+ Option(const None& none) : state(NONE) {}
template <typename U>
- Option(const _Some<U>& some) : state(SOME), t(new T(some.t)) {}
+ Option(const _Some<U>& some) : Option(some.t) {}
- Option(const Option<T>& that)
+ Option(const Option<T>& that) : state(that.state)
{
- state = that.state;
- if (that.t != NULL) {
- t = new T(*that.t);
- } else {
- t = NULL;
+ if (that.isSome()) {
+ new (&t) T(that.t);
}
}
~Option()
{
- delete t;
+ if (isSome()) {
+ t.~T();
+ }
}
Option<T>& operator = (const Option<T>& that)
{
if (this != &that) {
- delete t;
+ if (isSome()) {
+ t.~T();
+ }
state = that.state;
- if (that.t != NULL) {
- t = new T(*that.t);
- } else {
- t = NULL;
+ if (that.isSome()) {
+ new (&t) T(that.t);
}
}
@@ -79,8 +78,8 @@ public:
bool operator == (const Option<T>& that) const
{
- return (state == NONE && that.state == NONE) ||
- (state == SOME && that.state == SOME && *t == *that.t);
+ return (isNone() && that.isNone()) ||
+ (isSome() && that.isSome() && t == that.t);
}
bool operator != (const Option<T>& that) const
@@ -90,7 +89,7 @@ public:
bool operator == (const T& that) const
{
- return state == SOME && *t == that;
+ return isSome() && t == that;
}
bool operator != (const T& that) const
@@ -101,10 +100,10 @@ public:
bool isSome() const { return state == SOME; }
bool isNone() const { return state == NONE; }
- const T& get() const { assert(state == SOME); return *t; }
+ const T& get() const { assert(isSome()); return t; }
// This must return a copy to avoid returning a reference to a temporary.
- T get(const T& _t) const { return state == NONE ? _t : *t; }
+ T get(const T& _t) const { return isNone() ? _t : t; }
private:
enum State {
@@ -112,11 +111,15 @@ private:
NONE,
};
- Option(State _state, T* _t = NULL)
- : state(_state), t(_t) {}
-
State state;
- T* t;
+
+ union {
+ // We remove the const qualifier (if there is one) from T so that
+ // we can initialize 't' from outside of the initializer list
+ // using placement new. This is necessary because sometimes we
+ // specialize 'Option' as such: 'Option<const T>'.
+ typename std::remove_const<T>::type t;
+ };
};
[2/2] mesos git commit: Re-order structs to prevent forward
declaration dependency.
Posted by be...@apache.org.
Re-order structs to prevent forward declaration dependency.
Necessary for unrestricted union implementation of Option<T>.
Review: https://reviews.apache.org/r/32837
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1226e0ad
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1226e0ad
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1226e0ad
Branch: refs/heads/master
Commit: 1226e0ad6b036a19a5b981c7c1b85941677e25c5
Parents: a5640ad
Author: Joris Van Remoortere <jo...@gmail.com>
Authored: Sat May 2 17:16:58 2015 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sat May 2 17:51:48 2015 -0700
----------------------------------------------------------------------
src/slave/state.hpp | 124 +++++++++++++++++++++++++----------------------
1 file changed, 66 insertions(+), 58 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/1226e0ad/src/slave/state.hpp
----------------------------------------------------------------------
diff --git a/src/slave/state.hpp b/src/slave/state.hpp
index 31dfdd5..fed4b7e 100644
--- a/src/slave/state.hpp
+++ b/src/slave/state.hpp
@@ -47,11 +47,12 @@ namespace state {
// Forward declarations.
struct State;
struct SlaveState;
+struct ResourcesState;
struct FrameworkState;
struct ExecutorState;
struct RunState;
struct TaskState;
-struct ResourcesState;
+
// This function performs recovery from the state stored at 'rootDir'.
// If the 'strict' flag is set, any errors encountered while
@@ -66,6 +67,7 @@ struct ResourcesState;
// machine has rebooted since the last slave run, None is returned.
Result<State> recover(const std::string& rootDir, bool strict);
+
namespace internal {
inline Try<Nothing> checkpoint(
@@ -166,47 +168,72 @@ Try<Nothing> checkpoint(const std::string& path, const T& t)
}
-// The top level state. Each of the structs below (recursively)
-// recover the checkpointed state.
-struct State
+// NOTE: The *State structs (e.g., TaskState, RunState, etc) are
+// defined in reverse dependency order because many of them have
+// Option<*State> dependencies which means we need them declared in
+// their entirety in order to compile because things like
+// Option<*State> need to know the final size of the types.
+
+struct TaskState
{
- State() : errors(0) {}
+ TaskState () : errors(0) {}
- Option<ResourcesState> resources;
- Option<SlaveState> slave;
+ static Try<TaskState> recover(
+ const std::string& rootDir,
+ const SlaveID& slaveId,
+ const FrameworkID& frameworkId,
+ const ExecutorID& executorId,
+ const ContainerID& containerId,
+ const TaskID& taskId,
+ bool strict);
- // TODO(jieyu): Consider using a vector of Option<Error> here so
- // that we can print all the errors. This also applies to all the
- // State structs below.
+ TaskID id;
+ Option<Task> info;
+ std::vector<StatusUpdate> updates;
+ hashset<UUID> acks;
unsigned int errors;
};
-struct ResourcesState
+struct RunState
{
- ResourcesState() : errors(0) {}
+ RunState () : completed(false), errors(0) {}
- static Try<ResourcesState> recover(
+ static Try<RunState> recover(
const std::string& rootDir,
+ const SlaveID& slaveId,
+ const FrameworkID& frameworkId,
+ const ExecutorID& executorId,
+ const ContainerID& containerId,
bool strict);
- Resources resources;
+ Option<ContainerID> id;
+ hashmap<TaskID, TaskState> tasks;
+ Option<pid_t> forkedPid;
+ Option<process::UPID> libprocessPid;
+
+ // Executor terminated and all its updates acknowledged.
+ bool completed;
+
unsigned int errors;
};
-struct SlaveState
+struct ExecutorState
{
- SlaveState () : errors(0) {}
+ ExecutorState () : errors(0) {}
- static Try<SlaveState> recover(
+ static Try<ExecutorState> recover(
const std::string& rootDir,
const SlaveID& slaveId,
+ const FrameworkID& frameworkId,
+ const ExecutorID& executorId,
bool strict);
- SlaveID id;
- Option<SlaveInfo> info;
- hashmap<FrameworkID, FrameworkState> frameworks;
+ ExecutorID id;
+ Option<ExecutorInfo> info;
+ Option<ContainerID> latest;
+ hashmap<ContainerID, RunState> runs;
unsigned int errors;
};
@@ -229,66 +256,47 @@ struct FrameworkState
};
-struct ExecutorState
+struct ResourcesState
{
- ExecutorState () : errors(0) {}
+ ResourcesState() : errors(0) {}
- static Try<ExecutorState> recover(
+ static Try<ResourcesState> recover(
const std::string& rootDir,
- const SlaveID& slaveId,
- const FrameworkID& frameworkId,
- const ExecutorID& executorId,
bool strict);
- ExecutorID id;
- Option<ExecutorInfo> info;
- Option<ContainerID> latest;
- hashmap<ContainerID, RunState> runs;
+ Resources resources;
unsigned int errors;
};
-struct RunState
+struct SlaveState
{
- RunState () : completed(false), errors(0) {}
+ SlaveState () : errors(0) {}
- static Try<RunState> recover(
+ static Try<SlaveState> recover(
const std::string& rootDir,
const SlaveID& slaveId,
- const FrameworkID& frameworkId,
- const ExecutorID& executorId,
- const ContainerID& containerId,
bool strict);
- Option<ContainerID> id;
- hashmap<TaskID, TaskState> tasks;
- Option<pid_t> forkedPid;
- Option<process::UPID> libprocessPid;
-
- // Executor terminated and all its updates acknowledged.
- bool completed;
-
+ SlaveID id;
+ Option<SlaveInfo> info;
+ hashmap<FrameworkID, FrameworkState> frameworks;
unsigned int errors;
};
-struct TaskState
+// The top level state. The members are child nodes in the tree. Each
+// child node (recursively) recovers the checkpointed state.
+struct State
{
- TaskState () : errors(0) {}
+ State() : errors(0) {}
- static Try<TaskState> recover(
- const std::string& rootDir,
- const SlaveID& slaveId,
- const FrameworkID& frameworkId,
- const ExecutorID& executorId,
- const ContainerID& containerId,
- const TaskID& taskId,
- bool strict);
+ Option<ResourcesState> resources;
+ Option<SlaveState> slave;
- TaskID id;
- Option<Task> info;
- std::vector<StatusUpdate> updates;
- hashset<UUID> acks;
+ // TODO(jieyu): Consider using a vector of Option<Error> here so
+ // that we can print all the errors. This also applies to all the
+ // State structs below.
unsigned int errors;
};