[llvm] r211595 - Pass a unique_ptr<MemoryBuffer> to the constructors in the Binary hierarchy.

[Alp Toker]

On 05/07/2014 04:59, Rafael Espíndola wrote:
>> Where? *All* current uses on ToT seem to mmap their own private MemoryBuffer
>> and un-mmap it immediately afterwards.
> Where? Show an strace where an **ObjectFile** is reopened.

Trivially: just name them multiple times in llvm-dwarfdump/lli. That's 
probably fine, but less so with the proposed LTO object file patch where 
there's will be real cost.

>
> With object files we don't have the complexities of a source manager.
> There are two reasonable ownership models: It owns the buffer and it
> don't own the buffer. Not owning it might be more convenient,, I am
> looking into it. A reference count makes no sense for the uses we
> have.

Reference counting is tangential to the discussion.

The caller can do what they want to provision the MemoryBuffer instances 
-- that could be reference counting, malloc or who knows, even stack 
allocation. It's invisible to ObjectFile as long as it only passes 
around pointers.

Refcounting was just my suggestion for the slicing problem -- not a 
requirement or demand in any way, and you already said it's not needed..


>
> If the complexities of a source manager require a reference count to
> avoid duplicating read and stats, then it should hold a list of
> std::shared_ptr, or a list of a RefCountedMemoryBuffer. Changing how
> ObjectFile works because a *source* manager wants to refcount buffers
> is not OK.

My point is that it's not OK to delete the caller's MemoryBuffer, nor 
does it make sense to pass it around in a std::unique_ptr given that the 
buffer is clearly shareable and holds no state.

If you like you can store a pointer, keep a reference, or refcount -- 
but two commits look wrong because they enforce unique ownership on a 
resource that clearly shouldn't be uniquely owned.

Alp.




>
> Cheers,
> Rafael

-- 
http://www.nuanti.com
the browser experts