From reviews-return-18848-apmail-mesos-reviews-archive=mesos.apache.org@mesos.apache.org Mon Jan 4 16:49:56 2016 Return-Path: X-Original-To: apmail-mesos-reviews-archive@minotaur.apache.org Delivered-To: apmail-mesos-reviews-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 1F66618B56 for ; Mon, 4 Jan 2016 16:49:56 +0000 (UTC) Received: (qmail 48898 invoked by uid 500); 4 Jan 2016 16:49:56 -0000 Delivered-To: apmail-mesos-reviews-archive@mesos.apache.org Received: (qmail 48867 invoked by uid 500); 4 Jan 2016 16:49:56 -0000 Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@mesos.apache.org Delivered-To: mailing list reviews@mesos.apache.org Received: (qmail 48851 invoked by uid 99); 4 Jan 2016 16:49:55 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 04 Jan 2016 16:49:55 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 89C872972B2; Mon, 4 Jan 2016 16:49:54 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============5361407190204357278==" MIME-Version: 1.0 Subject: Re: Review Request 41618: Edited defer documentation in libprocess. From: "Greg Mann" To: "Benjamin Hindman" , "Neil Conway" Cc: "mesos" , "Alexander Rojas" , "Greg Mann" Date: Mon, 04 Jan 2016 16:49:54 -0000 Message-ID: <20160104164954.26043.54503@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: "Greg Mann" X-ReviewGroup: mesos X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/41618/ X-Sender: "Greg Mann" References: <20151222102439.25862.75183@reviews.apache.org> In-Reply-To: <20151222102439.25862.75183@reviews.apache.org> Reply-To: "Greg Mann" X-ReviewRequest-Repository: mesos --===============5361407190204357278== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > 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 ----------------------------------------------------------- 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 > > --===============5361407190204357278==--