You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Dominic Hamon <dh...@twopensource.com> on 2014/10/03 01:45:43 UTC
Review Request 26289: Replace some shared_ptr with Owned to clarify
ownership passing.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26289/
-----------------------------------------------------------
Review request for mesos and Ben Mahler.
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 88570f7c078faa7d79b2c187aa6a15e4e939878c
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
Re: Review Request 26289: Replace some shared_ptr with Owned to
clarify ownership passing.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26289/#review55288
-----------------------------------------------------------
Bad patch!
Reviews applied: [26289]
Failed command: git apply --index 26289.patch
Error:
error: patch failed: 3rdparty/libprocess/include/process/dispatch.hpp:65
error: 3rdparty/libprocess/include/process/dispatch.hpp: patch does not apply
- Mesos ReviewBot
On Oct. 2, 2014, 11:45 p.m., Dominic Hamon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26289/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2014, 11:45 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> 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 88570f7c078faa7d79b2c187aa6a15e4e939878c
> 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
>
>
Re: Review Request 26289: Replace some shared_ptr with Owned to
clarify ownership passing.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26289/#review55316
-----------------------------------------------------------
Patch looks great!
Reviews applied: [26289]
All tests passed.
- Mesos ReviewBot
On Oct. 3, 2014, 1:07 a.m., Dominic Hamon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26289/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2014, 1:07 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> 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
>
>
Re: Review Request 26289: Replace some shared_ptr with Owned to
clarify ownership passing.
Posted by Dominic Hamon <dh...@twopensource.com>.
> On Oct. 7, 2014, 5:32 p.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.
>
> Jie Yu wrote:
> 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.
it's anything that doesn't use the c++11 implementations of dispatch/defer. The error message is from this: http://stackoverflow.com/questions/9955714/does-stdbind-work-with-move-only-types-in-general-and-stdunique-ptr-in-part.
Essentially, binding a move-only arg creates a move-only functor which causes issues further up the stack. My workaround (which compiles for g++-4.4, g++-4.8, and i'm testing 4.6 and clang) is to have two top-level dispatch methods: one takes Owned and releases it into a unique_ptr, then forwards to the other that takes a unique_ptr directly.
I think there's another change in flight that replaces much of this code with variadic templated versions so we can revisit once it's all simplified.
- Dominic
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26289/#review55748
-----------------------------------------------------------
On Oct. 7, 2014, 5:15 p.m., Dominic Hamon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26289/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2014, 5:15 p.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
>
>
Re: Review Request 26289: Replace some shared_ptr with Owned to
clarify ownership passing.
Posted by Michael Park <mc...@gmail.com>.
> 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?
+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.
- Michael
-----------------------------------------------------------
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
>
>
Re: Review Request 26289: Replace some shared_ptr with Owned to
clarify ownership passing.
Posted by Dominic Hamon <dh...@twopensource.com>.
> On Oct. 7, 2014, 5:32 p.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.
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.
- Dominic
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26289/#review55748
-----------------------------------------------------------
On Oct. 7, 2014, 5:15 p.m., Dominic Hamon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26289/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2014, 5:15 p.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
>
>
Re: Review Request 26289: Replace some shared_ptr with Owned to
clarify ownership passing.
Posted by Dominic Hamon <dh...@twopensource.com>.
> On Oct. 7, 2014, 5:32 p.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.
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.
- Dominic
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26289/#review55748
-----------------------------------------------------------
On Oct. 7, 2014, 5:15 p.m., Dominic Hamon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26289/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2014, 5:15 p.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
>
>
Re: Review Request 26289: Replace some shared_ptr with Owned to
clarify ownership passing.
Posted by Jie Yu <yu...@gmail.com>.
> 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
>
>
Re: Review Request 26289: Replace some shared_ptr with Owned to
clarify ownership passing.
Posted by Jie Yu <yu...@gmail.com>.
> 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.
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.
- 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
>
>
Re: Review Request 26289: Replace some shared_ptr with Owned to
clarify ownership passing.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26289/#review55748
-----------------------------------------------------------
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?
- Jie Yu
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
>
>
Re: Review Request 26289: Replace some shared_ptr with
unique_ptr/Owned to clarify ownership passing.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26289/
-----------------------------------------------------------
(Updated Oct. 14, 2014, 4:29 p.m.)
Review request for mesos and Jie Yu.
Changes
-------
rebased
Repository: mesos-git
Description
-------
replace shared_ptr with Owned where ownership is being passed.
Diffs (updated)
-----
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 294e2158876b25b06456a3619e547082d71e31ed
3rdparty/libprocess/src/process.cpp 85fb9958342f0bcdde322d9c55333126e6f86668
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
Re: Review Request 26289: Replace some shared_ptr with
unique_ptr/Owned to clarify ownership passing.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26289/
-----------------------------------------------------------
(Updated Oct. 10, 2014, 2:56 p.m.)
Review request for mesos and Jie Yu.
Changes
-------
using unique_ptr where possible.
Summary (updated)
-----------------
Replace some shared_ptr with unique_ptr/Owned to clarify ownership passing.
Repository: mesos-git
Description
-------
replace shared_ptr with Owned where ownership is being passed.
Diffs (updated)
-----
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/m4/ax_cxx_compile_stdcxx_11.m4 07b298f151094e818287f741b3e0efd28374e82b
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
Re: Review Request 26289: Replace some shared_ptr with Owned to
clarify ownership passing.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
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.
Changes
-------
Handing off to jie for review.
--bmahler
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
Re: Review Request 26289: Replace some shared_ptr with Owned to
clarify ownership passing.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26289/
-----------------------------------------------------------
(Updated Oct. 6, 2014, 3:10 p.m.)
Review request for mesos and Ben Mahler.
Changes
-------
rebased
Repository: mesos-git
Description
-------
replace shared_ptr with Owned where ownership is being passed.
Diffs (updated)
-----
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
Re: Review Request 26289: Replace some shared_ptr with Owned to
clarify ownership passing.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26289/
-----------------------------------------------------------
(Updated Oct. 2, 2014, 6:07 p.m.)
Review request for mesos and Ben Mahler.
Changes
-------
rebased
Repository: mesos-git
Description
-------
replace shared_ptr with Owned where ownership is being passed.
Diffs (updated)
-----
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