Re: Review Request 41618: Edited defer documentation in libprocess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41618/#review141215 --- Although I think these changes are still a good idea, I'm going to close this review for the time being, until I have time to come back to it in earnest. - Greg Mann On Dec. 22, 2015, midnight, Greg Mann wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41618/ > --- > > (Updated Dec. 22, 2015, midnight) > > > Review request for mesos, Benjamin Hindman and Neil Conway. > > > Repository: mesos > > > Description > --- > > Edited defer documentation in libprocess. > > > Diffs > - > > 3rdparty/libprocess/README.md 6a47f6772bb7a74af368ed970af0f1c154a070e3 > > Diff: https://reviews.apache.org/r/41618/diff/ > > > Testing > --- > > Viewed on github: > https://github.com/mesosphere/mesos/tree/defer_doc_update/3rdparty/libprocess > > > Thanks, > > Greg Mann > >
Re: Review Request 41618: Edited defer documentation in libprocess.
> On Dec. 22, 2015, 10:24 a.m., Alexander Rojas wrote:
> > Hey Gregg, Thanks for working on this, I really think this needs
> > documentation. However, I think we are going great lengths to explain
> > relatively simple concepts. I also feel that tying so much the concept of a
> > `defer` to a future limits the capabilities of defer.
> >
> > I feel the best way to explain these concepts goes like this:
> >
> > # Dispatch
> >
> > `dispatch` schedules a method for asynchronous execution. It creates an
> > event which is added at the back of the process event queue and will be
> > executed once it reaches the front.
> >
> > # Defer
> >
> > Creates a [callable
> > object](http://en.cppreference.com/w/cpp/concept/Callable) of type
> > `Deferred` which will execute a `dispatch`
> > operation once it gets invoked. Example:
> >
> > ```c++
> > using namespace process;
> >
> > class CountProcess : public Process
> > {
> > public:
> > CountProcess() : counter_(0) {}
> >
> > void increase() { ++counter_; }
> >
> > private:
> > unsinged int counter_;
> > };
> >
> > int main(int argc, cha** argv)
> > {
> > CountProcess counter;
> > spawn(counter);
> >
> > Deferred deferred = defer(counter, &CountProcess::increase);
> >
> > for (int i = 0; i < 10; ++i) {
> > deferred();
> > }
> >
> > // This code is equivalent to the one above:
> > for (int i = 0; i < 10; ++i) {
> > dispatch(counter, &CountProcess::increase);
> > }
> >
> > terminate(counter);
> > wait(counter);
> >
> > return 0;
> > }
> > ```
> >
> > I allows a lot of flexibility over a dispatch, since a `Deferred` object
> > may never be invoked, it could be invoked multiple times or it could be
> > invoked in a completely different execution context. Example:
> >
> > ```c++
> > // In log.cpp
> > using namespace process;
> >
> > class LogProcess : public Process
> > {
> > public:
> > LogProcess(const std::string& filePath) : logfile_(filePath) {}
> > ~LogProcess() { logfile_.flush(); logfile.close(); }
> >
> > void log(const std::string& message) { logfile_ << message << '\n'; }
> >
> > private:
> > std:::ofstream logfile_;
> > };
> >
> > LogProcess *logProcess;
> >
> > void initLog(const std::string& filePath)
> > {
> > logProcess = new LogProcess(filePath);
> > spawn(logProcess);
> > }
> >
> > void terminateLog()
> > {
> > terminate(logProcess);
> > delete logProcess;
> > }
> >
> > Deferred getLogger()
> > {
> > return defer(logProcess, &LogProcess::log);
> > }
> >
> >
> > // in log.hpp
> > void initLog(const std::string& filePath);
> > void terminateLogger();
> > Deferred getLogger();
> >
> > // in main.cpp
> >
> > #include
> >
> > // SomeOtherClass::SomeOtherClass(std::function
> > logger);
> >
> > int main(int argc, cha** argv)
> > {
> > initLog("/var/log/test.log");
> >
> > Deferred logger = getLogger();
> > logger("Starting app");
> >
> > // SomeOtherClass has no idea of what logger
> > // is or even an idea of processes.
> > SomeOtherClass(logger);
> >
> > terminateLog();
> > return 0;
> > }
> > ```
> >
> > Its power really comes by when mixed with promises and futures, since it
> > allows to attach a `dispatch` only when a future transitions, example:
> >
> > ```c++
> > using namespace process;
> >
> > void foo()
> > {
> > ProcessBase process;
> > spawn(process);
> >
> > std::thread::id mainThreadId = std::this_thread::get_id();
> >
> > Deferred deferred = defer(
> > process,
> > [mainThreadId](int i) {
> > // Invoked _asynchronously_ using `process` as the
> > // execution context.
> > assert(mainThreadId != std::this_thread::get_id());
> > });
> >
> > Promise promise;
> >
> > promise.future().then(deferred);
> >
> > promise.future().then([mainThreadId](int i) {
> > // Invoked synchronously from the execution context of
> > // the thread that completes the future!
> > assert(mainThreadId == std::this_thread::get_id());
> > });
> >
> > // Executes both callbacks synchronously, which _dispatches_
> > // the deferred lambda to run asynchronously in the execution
> > // context of `process` but invokes the other lambda immediately.
> > promise.set(42);
> >
> > terminate(process);
> > }
> > ```
Alex, thanks for your work in writing this great feedback :-D
I like your take on it; we can incorporate some of these code excerpts, and I
think I agree that simplifying the language and the explanation will make it
easier for a newcomer to understand. I'm going to be working on these docs a
bit more as part of this sprint anyway (ticket here:
https://issues.apache.org/jira/browse/MESOS-4207). With this review, I was
originally just attempting to fix some grammatical issues, etc., but perhaps
I'll add some more significant revisions of the documentation text as well
based on your input.
Cheers!
- Greg
-
Re: Review Request 41618: Edited defer documentation in libprocess.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41618/#review111592
---
Hey Gregg, Thanks for working on this, I really think this needs documentation.
However, I think we are going great lengths to explain relatively simple
concepts. I also feel that tying so much the concept of a `defer` to a future
limits the capabilities of defer.
I feel the best way to explain these concepts goes like this:
# Dispatch
`dispatch` schedules a method for asynchronous execution. It creates an event
which is added at the back of the process event queue and will be executed once
it reaches the front.
# Defer
Creates a [callable object](http://en.cppreference.com/w/cpp/concept/Callable)
of type `Deferred` which will execute a `dispatch`
operation once it gets invoked. Example:
```c++
using namespace process;
class CountProcess : public Process
{
public:
CountProcess() : counter_(0) {}
void increase() { ++counter_; }
private:
unsinged int counter_;
};
int main(int argc, cha** argv)
{
CountProcess counter;
spawn(counter);
Deferred deferred = defer(counter, &CountProcess::increase);
for (int i = 0; i < 10; ++i) {
deferred();
}
// This code is equivalent to the one above:
for (int i = 0; i < 10; ++i) {
dispatch(counter, &CountProcess::increase);
}
terminate(counter);
wait(counter);
return 0;
}
```
I allows a lot of flexibility over a dispatch, since a `Deferred` object may
never be invoked, it could be invoked multiple times or it could be invoked in
a completely different execution context. Example:
```c++
// In log.cpp
using namespace process;
class LogProcess : public Process
{
public:
LogProcess(const std::string& filePath) : logfile_(filePath) {}
~LogProcess() { logfile_.flush(); logfile.close(); }
void log(const std::string& message) { logfile_ << message << '\n'; }
private:
std:::ofstream logfile_;
};
LogProcess *logProcess;
void initLog(const std::string& filePath)
{
logProcess = new LogProcess(filePath);
spawn(logProcess);
}
void terminateLog()
{
terminate(logProcess);
delete logProcess;
}
Deferred getLogger()
{
return defer(logProcess, &LogProcess::log);
}
// in log.hpp
void initLog(const std::string& filePath);
void terminateLogger();
Deferred getLogger();
// in main.cpp
#include
// SomeOtherClass::SomeOtherClass(std::function
logger);
int main(int argc, cha** argv)
{
initLog("/var/log/test.log");
Deferred logger = getLogger();
logger("Starting app");
// SomeOtherClass has no idea of what logger
// is or even an idea of processes.
SomeOtherClass(logger);
terminateLog();
return 0;
}
```
Its power really comes by when mixed with promises and futures, since it allows
to attach a `dispatch` only when a future transitions, example:
```c++
using namespace process;
void foo()
{
ProcessBase process;
spawn(process);
std::thread::id mainThreadId = std::this_thread::get_id();
Deferred deferred = defer(
process,
[mainThreadId](int i) {
// Invoked _asynchronously_ using `process` as the
// execution context.
assert(mainThreadId != std::this_thread::get_id());
});
Promise promise;
promise.future().then(deferred);
promise.future().then([mainThreadId](int i) {
// Invoked synchronously from the execution context of
// the thread that completes the future!
assert(mainThreadId == std::this_thread::get_id());
});
// Executes both callbacks synchronously, which _dispatches_
// the deferred lambda to run asynchronously in the execution
// context of `process` but invokes the other lambda immediately.
promise.set(42);
terminate(process);
}
```
3rdparty/libprocess/README.md (lines 113 - 114)
- Alexander Rojas
On Dec. 22, 2015, 1 a.m., Greg Mann wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41618/
> ---
>
> (Updated Dec. 22, 2015, 1 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Edited defer documentation in libprocess.
>
>
> Diffs
> -
>
> 3rdparty/libprocess/README.md 6a47f6772bb7a74af368ed970af0f1c154a070e3
>
> Diff: https://reviews.apache.org/r/41618/diff/
>
>
> Testing
> ---
>
> Viewed on github:
> https://github.com/mesosphere/mesos/tree/defer_doc_update/3rdparty/libprocess
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 41618: Edited defer documentation in libprocess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41618/#review111576 --- Patch looks great! Reviews applied: [41618] Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh - Mesos ReviewBot On Dec. 22, 2015, midnight, Greg Mann wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41618/ > --- > > (Updated Dec. 22, 2015, midnight) > > > Review request for mesos, Benjamin Hindman and Neil Conway. > > > Repository: mesos > > > Description > --- > > Edited defer documentation in libprocess. > > > Diffs > - > > 3rdparty/libprocess/README.md 6a47f6772bb7a74af368ed970af0f1c154a070e3 > > Diff: https://reviews.apache.org/r/41618/diff/ > > > Testing > --- > > Viewed on github: > https://github.com/mesosphere/mesos/tree/defer_doc_update/3rdparty/libprocess > > > Thanks, > > Greg Mann > >
Review Request 41618: Edited defer documentation in libprocess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41618/ --- Review request for mesos, Benjamin Hindman and Neil Conway. Repository: mesos Description --- Edited defer documentation in libprocess. Diffs - 3rdparty/libprocess/README.md 6a47f6772bb7a74af368ed970af0f1c154a070e3 Diff: https://reviews.apache.org/r/41618/diff/ Testing --- Viewed on github: https://github.com/mesosphere/mesos/tree/defer_doc_update/3rdparty/libprocess Thanks, Greg Mann
