[PATCH] LLD: Use std::vector to store SimpleReferences instead of linked list.

[Jean-Daniel Dupas]

I think the early LLD coders are the best one to answer that. Nick, Michael ?

> Le 10 mars 2015 à 21:08, Rui Ueyama <ruiu at google.com> a écrit :
> 
> Ah, yes, that's true. This patch doesn't work indeed.
> 
> Out of curiosity, using BumpPtrAllocator to allocate atoms is just for speed?
> 
> On Tue, Mar 10, 2015 at 1:04 PM, Jean-Daniel Dupas <mailing at xenonium.com <mailto:mailing at xenonium.com>> wrote:
> std::vector allocates storage to store the list of plain pointer. Its destructor must be call to free that memory.
> That changed was driven by memory leaks analysis tool that reports leaks of memory allocated by std::vector, so this is not some just some arbitrary guess.
> 
>> Le 10 mars 2015 à 20:59, Rui Ueyama <ruiu at google.com <mailto:ruiu at google.com>> a écrit :
>> 
>> It seems okay to me if _references vector's destructor is not called. It doesn't own references its pointing to -- it contains a list of plain pointers, not unique_ptr or such. So references will be managed somewhere and that's not changed by this patch.
>> 
>> On Tue, Mar 10, 2015 at 5:17 AM, Jean-Daniel Dupas <mailing at xenonium.com <mailto:mailing at xenonium.com>> wrote:
>> My bad, I was thinking about SimpleDefinedAtom allocation and destruction.
>> Many subclass of SimpleDefinedAtom are allocated using bumpPtrAllocator, (Mach-O, ELF and COFF are all doing this).
>> In your patch, the vector will be owned by such atoms and its destructor will never be executed.
>> 
>> > Le 10 mars 2015 à 05:55, Rui Ueyama <ruiu at google.com <mailto:ruiu at google.com>> a écrit :
>> >
>> > Where is the code? Looks like SimpleReference is used only by PE/COFF port.
>> >
>> > On Mon, Mar 9, 2015 at 3:46 PM, Jean-Daniel Dupas <mailing at xenonium.com <mailto:mailing at xenonium.com>> wrote:
>> > SimpleReference was implemented using a vector not so long ago, but that design choice was made because std::vector is not bumpPtrAllocator friendly.
>> > Mach-O implementation (at least) uses a bumpPtrAllocator to allocate the SimpleReferences. So SimpleReference destructor is never call, and so are members destructor.
>> > If you use a std::vector, it means you will leak the vector storage memory. As all ilist nodes are allocated using the bumpPtr allocator, we don’t have that issue.
>> >
>> >
>> > http://reviews.llvm.org/D8182 <http://reviews.llvm.org/D8182>
>> >
>> > EMAIL PREFERENCES
>> >   http://reviews.llvm.org/settings/panel/emailpreferences/ <http://reviews.llvm.org/settings/panel/emailpreferences/>
>> >
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>> 
>> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150311/5169b67f/attachment.html>