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