> On March 14, 2014, 7:08 p.m., Ben Mahler wrote:
> > Looks good, two main questions:
> > 
> > 1. Will you be following up to remove Sequence? Would love to see it 
> > removed if there's no use case.
> > 2. I'd really like to remove internal.hpp, I wonder if:
> >   (a) We should wait for C++11 and use std::mutex instead.
> >   (b) We should expose a synchronous Mutex in process that wraps std::mutex 
> > for C++11 and internal::acquire/internal::release for C++03.
> >   (c) We should just live with the cyclic include for now and move 
> > acquire/release into mutex.hpp. Forward declare as necessary (similar to 
> > what we did in stout/killtree.hpp)
> > 
> > Any of these sound good to you?

I don't want to expose a _synchronous_ mutex in libprocess (maybe in stout?). 
What's the advantage of doing a cyclic include instead of just pulling these 
into their own header for now? Are you worried people will be more likely to 
use them in a header? They were just as usable when they were in future.hpp. It 
seems more unfortunate to have future.hpp depend on mutex.hpp but only for 
internal::acquire and internal::release than pulling the header out.

As for removing Sequence, I think it's a generally useful abstraction (albeit 
more complicated and incomplete in as much as it's not very useful with out a 
lot more overloads) but I think it's okay to leave it in something like 
libprocess (or stout). If it was in Mesos I'd prefer to kill it as well. If 
you'd prefer to see it removed that's fine with me too, we can always 
(hopefully) recover it from the git history.


- Benjamin


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


On March 10, 2014, 8:03 a.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18950/
> -----------------------------------------------------------
> 
> (Updated March 10, 2014, 8:03 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This is a simpler alternative to using something like 'Sequence', although it 
> requires that one explicitly perform an action after the serialized work is 
> done using something like Future::onAny which now works correctly thanks to 
> the updated discard semantics.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/Makefile.am 3c6219eb6e76306463b3710ab7e50ec8b75d3d76 
>   3rdparty/libprocess/include/process/future.hpp 
> 27b0970bf1d1ae1b977ddfc2de5ee858f1031bf5 
>   3rdparty/libprocess/include/process/internal.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/mutex.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/mutex_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18950/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Reply via email to