[LLVMdev] New type of smart pointer for LLVM
David Blaikie
dblaikie at gmail.com
Thu Sep 25 11:30:28 PDT 2014
On Thu, Sep 25, 2014 at 11:06 AM, Ben Pope <benpope81 at gmail.com> wrote:
> On Friday, September 26, 2014 01:11 AM, David Blaikie wrote:
>
>>
>>
>> On Thu, Sep 25, 2014 at 1:44 AM, Renato Golin <renato.golin at linaro.org
>> <mailto:renato.golin at linaro.org>> wrote:
>>
>> On 25 September 2014 06:16, David Blaikie <dblaikie at gmail.com
>> <mailto:dblaikie at gmail.com>> wrote:
>> > I can go & dredge up some examples if we want to discuss the
>> particular
>> > merits & whether each of those cases would be better solved in some
>> other
>> > way, but it seemed pervasive enough in the current codebase that
>> some
>> > incremental improvement could be gained by replacing these cases
>> with a more
>> > specific tool for the job. We might still consider this tool to be
>> "not
>> > ideal".
>>
>> Having done that in another code base, I can see both merits and
>> problems.
>>
>> Our smart pointer behaved more or less like it's described above
>> (explicit acquire/release)
>>
>>
>> I'm not quite sure I follow you (or that you're following me). The goal
>> isn't about being explicit about acquire/release (indeed I wouldn't mind
>> more implicit acquisition from an always-owning pointer (unique_ptr) to
>> a sometimes-owning pointer (whatever we end up calling it), though
>> release is always going to be explicit I suspect).
>>
>
> Given the example, the smart_ptr can't deal with ownership at all, it
> merely destroys the pointee in the face of exceptions or forgetfulness
> (which, in the example, can't really be tolerated).
>
> and we've seen around 20% performance
>> improvement over shared_ptr due to savings on acquire/release
>> semantics.
>>
>>
>> The existing use cases I'm interested in aren't using shared_ptr and
>> wouldn't be ideally migrated to it (in some cases the existing ownership
>> might be on the stack (so it can't be shared)
>>
>
> null_deleter?
>
> or several layers up the
>> call stack through which raw pointers have been passed (eg: build
>> something, pass it through a few APIs, underlying thing 'wants' to take
>> ownership, but it will be constructed/destroyed before this API returns
>> - so we hack around it either by having a "OwnsThing" flag in the
>> underlying thing, or having a "takeThing" we hope we call before the
>> underlying thing dies and destroys the thing)).
>>
>
> Obviously combing the ownership flag and the pointer into some kind of
> "smart type" would be preferable, since they must be dealt with together.
>
> We're dealing with types that have raw pointer + bool indicating
>> ownership members or worse, types which take ownership but we lie to
>> about giving them ownership (so some API takes a non-owning T* (or T&),
>> passes it as owning to some other thing, then is sure to take ownership
>> back from that thing and call 'release()" on the unique_ptr to ensure
>> ownership was never really taken).
>>
>
> Confusing. null_deleter, again?
>
> We had three derivative types of pointers
>> (Shared/Owned/Linked) which would just differ on the default behaviour
>> of construction / destruction, but all of them could still explicitly
>> call getClear / getLink / etc.
>>
>> The downside was that almost every interaction with smart pointers had
>> to be carefully planned and there was a lot of room for errors,
>>
>>
>> My hope is that having a single construct for conditional ownership will
>> be less confusing than the existing ad-hoc solutions in many places.
>> It's possible that the right answer is to remove the conditional
>> ownership in these APIs entirely, but I'm not sure that's
>> possible/practical/desired (it might be - which is why I've been
>> hesitant to write this abstraction myself just yet - sort of letting it
>> stew in my head a bit to see what feels right).
>>
>
> If the code really does look like the example, hesitation is probably wise.
>
> especially from people that didn't know the context. In the end, the
>> amount of work that had to be put to make it work was similar than
>> when dealing with normal pointers,
>>
>>
>> I rather hope that's not the case - these conditionally owning raw
>> pointers are pretty subtle, easy to miss a delete and leak, easy to have
>> an early return and fail to take back and release ownership from
>> something that wasn't really owning in the first place, etc.
>>
>> but you had to learn yet another
>> pointer semantics.
>>
>> The marginal gain was that pointer interactions were explicit, making
>> it easier for someone *not* used to C++ pointer semantics to
>> understand when reading code, not necessarily when writing it. The
>> marginal lost was getting people that already knew the STL and Boost
>> smart pointer semantics to get confused.
>>
>>
>> This is another pointer semantic - I'm not suggesting replacing
>> unique_ptr or shared_ptr - I want to use them as much as possible where
>> they represent the right semantic. I'm just dealing with a situation
>> that doesn't map directly to either of those: conditional ownership.
>>
>
> I think I may have mentioned null_deleter.
>
> Having done that, I still rather use normal pointers and have valgrind
>> / sanitizers tell me when I screwed up.
>>
>> My tuppence.
>>
>
> consider a leaky example:
> {
> T* p = new T;
> if (condition1) {
> f(p); // takes ownership of p
> }
> p->SomeMethod();
>
> if (condition2) {
> return nullptr; // Leak!
> }
>
> g(p); // don't take ownership of p
> return p;
> }
>
Yeah, imho this is a somewhat uninteresting example - the issue is just
that ownership is passed elsewhere, then after that point a non-owning
pointer is required. This is statically known and can be coded statically
quite simply:
unique_ptr ownT = ...;
/* stuff */
T *t = ownT.get();
sink(std::move(ownT));
return t;
I don't think this particular use case really merits designing a new
(fairly powerful) abstraction. But it's not necessarily easy to provide a
nice example of the issue that's sufficiently motivating.
The use case I have in mind (of which I've seen several in LLVM and Clang)
looks something more like this:
struct foo {
unique_ptr<T> u;
foo(unique_ptr<T> u) : u(std::move(u)) {}
int stuff();
unique_ptr<T> take() { return std::move(u); }
};
int f1(T& t) {
// I want to do "stuff" to 't' but I don't own it...
// lying through my teeth
foo f(std::unique_ptr<T>(&t));
// I really hope 'stuff' doesn't throw, because then we'll end up with
a double delete
int i = f.stuff();
f.take().release(); // not really leaking because I lied in the first
place
return i;
}
That's one case - check the revision numbers mentioned in the first message
on this thread for another example that doesn't involve a member, just some
rather convoluted ownership semantics (some codepaths there's a default
object to point at, other codepaths there's a newly allocated object) it
looks something like this:
T *t = nullptr;
bool Owning = false;
if (x) {
t = new T;
Owning = true;
} else
t = &default;
func(*t);
if (Owning)
delete t;
This comes up not infrequently. The code in func doesn't care if it owns,
it just wants to do something. Now obviously in the code I just wrote it
could be easily refactored without much duplication:
if (x)
func(*make_unique<T>());
else
func(default);
But it's not always that simple. Maybe it can/should be made that simple
and we conclude that we don't want/need conditional ownership, but that's
what this thread is hopefully here to help decide.
>
> Let's see what we can do:
>
> {
> auto p = llvm::make_unique<T>();
> if (condition1) {
> f(p); // takes ownership of p
> }
> p->SomeMethod();
>
> if (condition2) {
> return nullptr; // no leak;
> }
>
> g(p.get()); // don't take ownership of p
> return p; // not p.get()!
> }
>
> f(llvm::unique_ptr<T>& p)
> {
> if(!condition2) // I guess
> p = llvm::make_unique<T>(p, null_deleter()); [1]
> }
>
> Clearly the code is a mess as f(llvm::unique_ptr<T>&) needs to know
> condition2, but smart_ptr as described isn't helping anything.
>
> [1] std::unique_ptr doesn't have a type erased deleter, but perhaps a
> unique_ptr with a type erased deleter is actually the right tool for the
> job.
Yes, the lack of type erasure is the issue - and then the lack of
compatibility with existing unique_ptrs (I'd obviously like to be able to
make a conditional ownership unique_ptr from an unconditional ownership
unique_ptr&&, for example, and to be able to take unconditional ownership
if it is owned, etc)
>
>
> Ben
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140925/371538fe/attachment.html>
More information about the llvm-dev
mailing list