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

[Rui Ueyama]

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>
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> 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>
> 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> 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>
>> 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
>> >
>> > EMAIL PREFERENCES
>> >   http://reviews.llvm.org/settings/panel/emailpreferences/
>> >
>> >
>> >
>> > _______________________________________________
>> > 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/20150310/239acb36/attachment.html>