I have a proposal. Please let me know if that makes sense to you guys. One downside of using Owned is that it introduces a dependency to libprocess (see my previous reply). Since semantically, Owned and unique_ptr should be the same. I am in favor of replacing Owned with unique_ptr (using standard lib as much as possible).
One of the reason we want to keep Owned previously is because it allows transformation between Owned and Shared. And we do want to keep Shared because it enforces const access to underlying object (which is not provided by shared_ptr). What I am proposing right now is: 1) Get rid of Owned and replace it with unique_ptr 2) allow transformation between unique_ptr and Shared In that way, we can consistently use unique_ptr (common case) without introducing libprocess dependencies. And, we still keep Shared for enforcing const access (used by replicated log). What do you guys think? - Jie On Mon, Dec 15, 2014 at 1:31 PM, Dominic Hamon <[email protected]> wrote: > > I built some test cases using unique_ptr and ran into some issues pretty > much immediately. > > Everywhere I wanted to replace was not a raw pointer T* but a Try<T*>. The > factory methods that were used to create these already return a Try<T*>, > which means we'd need to change those to return Try<std::unique_ptr<T>> > which is intrusive. Further, Try itself doesn't readily support a > std::unique_ptr as a template parameter due to using heap allocations and > copy constructors internally. > > I think Jie's idea to use unrestricted union may help here though we'll > have to wait for g++-4.6. > > > On Fri, Dec 12, 2014 at 3:01 PM, Dominic Hamon <[email protected]> wrote: > > > > Great idea. > > > > There is one place that immediately springs to mind that I think is > > trivial but will get us used to the idea and exercise some of the demons: > > Unit tests. Within a test we regularly create some object on the heap and > > then delete it at the end of the test, or for the lifetime of a test > > fixture. All of these should be well scoped in lifetime and are prime > > targets for unique_ptr. > > > > On Wed, Dec 10, 2014 at 3:32 PM, Jie Yu <[email protected]> wrote: > >> > >> Dominic, > >> > >> Thank you for brining this up! Instead of making a decision based on > high > >> level design and coding philosophy, I would really love to see some > >> concrete examples in our code base where we can start to use unique_ptr > >> and > >> why it's better. Thoughts? > >> > >> - Jie > >> > >> On Wed, Dec 10, 2014 at 3:20 PM, Dominic Hamon <[email protected] > > > >> wrote: > >> > >> > Hello! > >> > > >> > We have access to std::unique_ptr, as many of you know, but we're not > >> using > >> > it. I'd like to start using it. Before we do, I'd like to discuss how > we > >> > use it, when we use it, and things to be aware of. > >> > > >> > The style guide has the following to say (long quote, sorry): > >> > > >> > " > >> > Prefer to have single, fixed owners for dynamically allocated objects. > >> > Prefer to transfer ownership with smart pointers. > >> > > >> > "Ownership" is a bookkeeping technique for managing dynamically > >> allocated > >> > memory (and other resources). The owner of a dynamically allocated > >> object > >> > is an object or function that is responsible for ensuring that it is > >> > deleted when no longer needed. Ownership can sometimes be shared, in > >> which > >> > case the last owner is typically responsible for deleting it. Even > when > >> > ownership is not shared, it can be transferred from one piece of code > to > >> > another. > >> > > >> > "Smart" pointers are classes that act like pointers, e.g. by > overloading > >> > the * and -> operators. Some smart pointer types can be used to > automate > >> > ownership bookkeeping, to ensure these responsibilities are met. > >> > std::unique_ptr is a smart pointer type introduced in C++11, which > >> > expresses exclusive ownership of a dynamically allocated object; the > >> object > >> > is deleted when the std::unique_ptr goes out of scope. It cannot be > >> copied, > >> > but can be moved to represent ownership transfer. std::shared_ptr is a > >> > smart pointer type that expresses shared ownership of a dynamically > >> > allocated object. std::shared_ptrs can be copied; ownership of the > >> object > >> > is shared among all copies, and the object is deleted when the last > >> > std::shared_ptr is destroyed. > >> > > >> > It's virtually impossible to manage dynamically allocated memory > without > >> > some sort of ownership logic. > >> > Transferring ownership of an object can be cheaper than copying it (if > >> > copying it is even possible). > >> > Transferring ownership can be simpler than 'borrowing' a pointer or > >> > reference, because it reduces the need to coordinate the lifetime of > the > >> > object between the two users. > >> > Smart pointers can improve readability by making ownership logic > >> explicit, > >> > self-documenting, and unambiguous. > >> > Smart pointers can eliminate manual ownership bookkeeping, simplifying > >> the > >> > code and ruling out large classes of errors. > >> > For const objects, shared ownership can be a simple and efficient > >> > alternative to deep copying. > >> > Ownership must be represented and transferred via pointers (whether > >> smart > >> > or plain). Pointer semantics are more complicated than value > semantics, > >> > especially in APIs: you have to worry not just about ownership, but > also > >> > aliasing, lifetime, and mutability, among other issues. > >> > The performance costs of value semantics are often overestimated, so > the > >> > performance benefits of ownership transfer might not justify the > >> > readability and complexity costs. > >> > APIs that transfer ownership force their clients into a single memory > >> > management model. > >> > Code using smart pointers is less explicit about where the resource > >> > releases take place. > >> > std::unique_ptr expresses ownership transfer using C++11's move > >> semantics, > >> > which are relatively new and may confuse some programmers. > >> > Shared ownership can be a tempting alternative to careful ownership > >> design, > >> > obfuscating the design of a system. > >> > Shared ownership requires explicit bookkeeping at run-time, which can > be > >> > costly. > >> > In some cases (e.g. cyclic references), objects with shared ownership > >> may > >> > never be deleted. > >> > Smart pointers are not perfect substitutes for plain pointers. > >> > If dynamic allocation is necessary, prefer to keep ownership with the > >> code > >> > that allocated it. If other code needs access to the object, consider > >> > passing it a copy, or passing a pointer or reference without > >> transferring > >> > ownership. Prefer to use std::unique_ptr to make ownership transfer > >> > explicit. For example: > >> > > >> > std::unique_ptr<Foo> FooFactory(); > >> > void FooConsumer(std::unique_ptr<Foo> ptr); > >> > Do not design your code to use shared ownership without a very good > >> reason. > >> > One such reason is to avoid expensive copy operations, but you should > >> only > >> > do this if the performance benefits are significant, and the > underlying > >> > object is immutable (i.e. std::shared_ptr<const Foo>). If you do use > >> shared > >> > ownership, prefer to use std::shared_ptr. > >> > > >> > Do not use scoped_ptr in new code unless you need to be compatible > with > >> > older versions of C++. Never use std::auto_ptr. Instead, use > >> > std::unique_ptr. > >> > " > >> > > >> > Now I'm a big fan of explicit ownership and moving ownership rather > than > >> > sharing non-smart pointers, but I recognise that in our code-base, > >> > ownership is difficult to reason about in many cases. However, we do > >> have > >> > quite a few cases where we manage lifetime scope with explicit delete > >> calls > >> > and I'd like to start by eliminating those. Ie, using std::unique_ptr > to > >> > manage lifetime not necessarily ownership. > >> > > >> > This is difficult though as we may pass these pointers to other > methods > >> or > >> > even other libprocess processes. In cases that we can reason about the > >> > lifetime of the various pointers, that should be fine, but we have to > be > >> > careful. > >> > > >> > One option is to start by replacing these pointers with > process::Owned. > >> The > >> > downside to this approach is that it introduces more non-standard > types > >> > and, because Owned is implemented using std::shared_ptr, doesn't move > us > >> > closer to defining clear ownership. > >> > > >> > So the floor is open. Do we: > >> > > >> > 1) embrace std::unique_ptr, eliminate raw pointers except in rare, > >> > well-defined cases from a lifetime point of view? > >> > 2) eliminate raw pointers in favour of std::shared_ptr and > >> std::unique_ptr > >> > then work to eliminate the std::shared_ptrs > >> > 3) use process::Owned everywhere and then find places where we can > >> > transition them to std::unique_ptr > >> > 4) do nothing; there's no great benefit to std::unique_ptr and this > >> smart > >> > pointer thing is just a fad. > >> > > >> > > >> > - dominic > >> > > >> > > >> > -- > >> > Dominic Hamon | @mrdo | Twitter > >> > *There are no bad ideas; only good ideas that go horribly wrong.* > >> > > >> > > > > > > -- > > Dominic Hamon | @mrdo | Twitter > > *There are no bad ideas; only good ideas that go horribly wrong.* > > > > > -- > Dominic Hamon | @mrdo | Twitter > *There are no bad ideas; only good ideas that go horribly wrong.* >
