[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