> On Nov. 28, 2016, 6:47 p.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/include/process/loop.hpp, line 137
> > <https://reviews.apache.org/r/54110/diff/1/?file=1570957#file1570957line137>
> >
> > Future<bool> condition = true;
> >
> > while (condition.get()) {
> > Future<T> next = discard_if_necessary(iterate());
> > if (next.isReady()) {
> > condition = discard_if_necessary(body(next.get()));
> > if (condition.isReady()) {
> > continue;
> > } else {
> > block_on_condition();
> > return;
> > }
> > } else {
> > block_on_next();
> > return;
> > }
> > }
> >
> > promise.set(Nothing()); // All done!
> >
> >
> >
> > void block_on_next()
> > {
> > next
> > .onAny([]() {
> > if (next.isReady()) {
> > condition = discard_if_necessary(body(next.get()));
> > if (condition.isReady()) {
> > run(loop);
> > } else {
> > block_on_condition();
> > }
> > } else if (isDiscarded) {
> > } else if (isFailed) {
> > }
> > });
> >
> > }
> >
> >
> > void block_on_condition()
> > {
> > condition
> > .onAny([]() {
> > if (condition.isReady()) {
> > if (condition.get()) {
> > run(loop);
> > } else {
> > promise.set(Nothing());
> > }
> > } else if (isDiscarded) {
> > } else if (isFailed) {
> > }
> > });
> > }
I think would suggest breaking this up into stages instead.
This approach removes duplicate code such as `body(next.get())`, handling of
`condition` and `promise.set(Nothing())`.
The transitions are: `run` --`next.isReady`--> `run_body`
--`condition.isReady`--> `run_condition`.
```cpp
template <typename Iterate, typename Body, typename T>
class Loop {
public:
void run() {
// This is the tight loop that we ideally stay in. We leave
// if we're gonna be blocked by `next`, or if we're done.
for (;;) {
next = propagate_discard(iterate());
if (!next.isReady()) {
break;
}
if (!run_body()) {
return;
}
}
// We're here because we're blocked on `next`,
// so let's defer a continuation on it.
next.onAny(defer(pid, [=](const Future<T>&) {
if (next.isReady()) {
if (run_body()) {
run();
}
} else if (next.isFailed()) {
promise.fail(next.failure());
} else if (next.isDiscarded()) {
promise.discard();
}
}));
}
private:
template <typename U>
Future<U> propagate_discard(Future<U> future) const {
if (promise.future().hasDiscard()) {
future.discard();
}
return future;
}
bool run_body() {
condition = propagate_discard(body(next.get()));
// This is the short-circuit. Ideally we go through here,
// but if the `condition` is blocked, then we proceed.
if (condition.isReady()) {
return run_condition();
};
// We're here because we're blocked on `condition`,
// so let's defer a continuation on it.
condition.onAny(defer(pid, [=](const Future<bool>&) {
if (condition.isReady()) {
if (run_condition()) {
run();
}
} else if (condition.isFailed()) {
promise.fail(next.failure());
} else if (condition.isDiscarded()) {
promise.discard();
}
}));
return false;
}
bool run_condition() {
bool cont = condition.get();
if (!cont) {
promise.set(Nothing());
}
return cont;
}
/* members... */
};
```
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54110/#review157168
-----------------------------------------------------------
On Nov. 28, 2016, 10:33 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54110/
> -----------------------------------------------------------
>
> (Updated Nov. 28, 2016, 10:33 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added `process::loop` abstraction.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am 71319891a451bd1d565210dcce2fb61fc69e1f61
> 3rdparty/libprocess/include/process/loop.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/loop_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/54110/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
