----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61058/#review181758 -----------------------------------------------------------
Fix it, then Ship it! 3rdparty/libprocess/configure.ac Lines 72-75 (patched) <https://reviews.apache.org/r/61058/#comment257430> A TODO related to the long term plan for these would be great (i.e. make default and eventually have as the only option?) Also, looks like you still need to update `docs/configuration.md` per my existing comment? 3rdparty/libprocess/src/event_queue.hpp Lines 39-44 (patched) <https://reviews.apache.org/r/61058/#comment257437> Thanks! 3rdparty/libprocess/src/event_queue.hpp Lines 74 (patched) <https://reviews.apache.org/r/61058/#comment257439> I like that there is a producer and consumer directly accessible, should we hide the Producer/Consumer constructors? 3rdparty/libprocess/src/event_queue.hpp Lines 100 (patched) <https://reviews.apache.org/r/61058/#comment257436> Might be a bit easier to read if you define an EventQueue in each ifdef case rather than split the body of EventQueue across the ifdef? 3rdparty/libprocess/src/event_queue.hpp Lines 225 (patched) <https://reviews.apache.org/r/61058/#comment257435> s/is/are/ 3rdparty/libprocess/src/event_queue.hpp Lines 256-257 (patched) <https://reviews.apache.org/r/61058/#comment257434> Ditto my comment here and in count() about the number, and should we be consistent about the bulk dequeue size? 3rdparty/libprocess/src/event_queue.hpp Lines 322 (patched) <https://reviews.apache.org/r/61058/#comment257433> Just curious why we wouldn't just pass the largest number possible here, e.g. SIZE_MAX. Is there a tradeoff here where you're trying to bound the amount of work the consumer has to do upon dequeue? Would be good to clarify in a comment if you have some thoughts on it. 3rdparty/libprocess/src/event_queue.hpp Lines 333 (patched) <https://reviews.apache.org/r/61058/#comment257432> If you want to be a little more succinct, ~585 years is enough, don't think people need to see the exact number of nanoseconds :) 3rdparty/libprocess/src/event_queue.hpp Lines 337-338 (patched) <https://reviews.apache.org/r/61058/#comment257431> I read this as "reading or writing" to the queue, did you mean reading or writing to this variable or just that there's only a single consumer? - Benjamin Mahler On July 29, 2017, 8:52 p.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61058/ > ----------------------------------------------------------- > > (Updated July 29, 2017, 8:52 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-7798 > https://issues.apache.org/jira/browse/MESOS-7798 > > > Repository: mesos > > > Description > ------- > > Added a lock-free event queue. > > > Diffs > ----- > > 3rdparty/libprocess/configure.ac cb2cf4f32be5cbdf9df1e32f9aaf2bbba0a5ae03 > 3rdparty/libprocess/src/event_queue.hpp PRE-CREATION > 3rdparty/libprocess/src/process.cpp > b268cdad776a3ca2a87cbe60eb098bde2a70667c > > > Diff: https://reviews.apache.org/r/61058/diff/3/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Hindman > >
