You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2015/12/22 01:00:37 UTC

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


Re: Review Request 41618: Edited defer documentation in libprocess.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 41618: Edited defer documentation in libprocess.

Posted by Greg Mann <gr...@mesosphere.io>.

> 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<ReturnType(ArgsTypes...)>` which will execute a `dispatch` operation once it gets invoked.  Example:
> > 
> > ```c++
> > using namespace process;
> > 
> > class CountProcess : public Process<CountProcess>
> > {
> > public:
> >   CountProcess() : counter_(0) {}
> >   
> >   void increase() { ++counter_; }
> >   
> > private:
> >   unsinged int counter_;
> > };
> > 
> > int main(int argc, cha** argv)
> > {
> >   CountProcess counter;
> >   spawn(counter);
> >   
> >   Deferred<void()> 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<LogProcess>
> > {
> > 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<void(const std::string&)> getLogger()
> > {
> >   return defer(logProcess, &LogProcess::log);
> > }
> > 
> > 
> > // in log.hpp
> > void initLog(const std::string& filePath);
> > void terminateLogger();
> > Deferred<void(const std::string&)> getLogger();
> > 
> > // in main.cpp
> > 
> > #include <log.hpp>
> > 
> > // SomeOtherClass::SomeOtherClass(std::function<void(const std::string&)> logger);
> > 
> > int main(int argc, cha** argv)
> > {
> >   initLog("/var/log/test.log");
> >   
> >   Deferred<void(const std::string&)> 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<void(int)> deferred = defer(
> >       process,
> >       [mainThreadId](int i) {
> >         // Invoked _asynchronously_ using `process` as the
> >         // execution context.
> >         assert(mainThreadId != std::this_thread::get_id());        
> >       });
> > 
> >   Promise<int> 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


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41618/#review111592
-----------------------------------------------------------


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.

Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
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<ReturnType(ArgsTypes...)>` which will execute a `dispatch` operation once it gets invoked.  Example:

```c++
using namespace process;

class CountProcess : public Process<CountProcess>
{
public:
  CountProcess() : counter_(0) {}
  
  void increase() { ++counter_; }
  
private:
  unsinged int counter_;
};

int main(int argc, cha** argv)
{
  CountProcess counter;
  spawn(counter);
  
  Deferred<void()> 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<LogProcess>
{
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<void(const std::string&)> getLogger()
{
  return defer(logProcess, &LogProcess::log);
}


// in log.hpp
void initLog(const std::string& filePath);
void terminateLogger();
Deferred<void(const std::string&)> getLogger();

// in main.cpp

#include <log.hpp>

// SomeOtherClass::SomeOtherClass(std::function<void(const std::string&)> logger);

int main(int argc, cha** argv)
{
  initLog("/var/log/test.log");
  
  Deferred<void(const std::string&)> 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<void(int)> deferred = defer(
      process,
      [mainThreadId](int i) {
        // Invoked _asynchronously_ using `process` as the
        // execution context.
        assert(mainThreadId != std::this_thread::get_id());        
      });

  Promise<int> 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)
<https://reviews.apache.org/r/41618/#comment171810>

    


- 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
> 
>