mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jie Yu" <yujie....@gmail.com>
Subject Re: Review Request 26289: Replace some shared_ptr with Owned to clarify ownership passing.
Date Fri, 10 Oct 2014 18:51:43 GMT


> On Oct. 8, 2014, 12:32 a.m., Jie Yu wrote:
> > Can you use unique_ptr directly now? If yes, I would suggest using unique_ptr directly
in libprocess. Here are the reasons:
> > 
> > 1) We use lots of shared_ptr in libprocess. Mixing Owned with shared_ptr seems to
be confusing.
> > 2) What if the implementation of Owned/Shared depends on dispatch, there will be
cyclic dependencies (though it's not a problem now).
> > 3) I think directly using std::shared_ptr and std::unique_ptr is fine in libprocess
(we even use pthread in libprocess:))
> > 
> > What do you think?
> 
> Michael Park wrote:
>     +1, I'd like to see movement toward `std::unique_ptr` and `std::shared_ptr` as well.
Both are available in `gcc-4.4` and up.
> 
> Dominic Hamon wrote:
>     I discussed this with vinod and he prefered a migration approach:
>     
>     1. migrate to Owned pointer to clarify ownership
>     2. replace Owned with unique_ptr
>     
>     I was originally going to move straight to unique_ptr (yes we can use it now in g++-4.4,
and std::move which is important for unique_ptr in APIs) but that was his suggestion. I'm
open to the alternative but we'll need to hash it out with him.
> 
> Jie Yu wrote:
>     Talked to Vinod. The consensus is: for libprocess primitives (essential for the runtime
like dispatch, delay, Future, ProcessBase, SocketManager, etc.), we should use std::unique_ptr
and std::shared_ptr directly. For Mesos code, try to use Owned and Shared as much as possible.
> 
> Dominic Hamon wrote:
>     for c++11 this is trivial. for earlier compilers we run into some issues trying to
bind to move-only types. I didn't see the issues with process::Owned because it is actually
a shared_ptr (hence copyable) under the hood. I'm not seeing a clean way around this at this
point. Perhaps we can have a C++11 'dispatch' high-level method that uses the unique_ptr and
one that uses Owned for older compilers? I'm nervous about taking this difference up the stack
though.

Can you paste the gcc version and the error message? I am curious to know.

I am fine with use Owned for now with a TODO somewhere sayting that we should use unique_ptr
directly for libprocess primitives.


- Jie


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


On Oct. 8, 2014, 12:15 a.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26289/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2014, 12:15 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> replace shared_ptr with Owned where ownership is being passed.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4

>   3rdparty/libprocess/include/process/defer.hpp ebe6f2db47cab2a3306288d8ebabb720e7cd8976

>   3rdparty/libprocess/include/process/delay.hpp 487f652c9e9b7c8c3aa8b4edc9e59789cffd8da9

>   3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227

>   3rdparty/libprocess/include/process/event.hpp bf689d7270df2c8f1f5c9165d2bbcfdca06e15a8

>   3rdparty/libprocess/src/process.cpp d30ed636ee24d0fe6e62f69a921307bde1f32765 
> 
> Diff: https://reviews.apache.org/r/26289/diff/
> 
> 
> Testing
> -------
> 
> make g++-4.4 and clang++-3.5
> make check clang++-3.5
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message