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

[Michael Spencer]

On Tue, Mar 10, 2015 at 1:08 PM, Rui Ueyama <ruiu at google.com> wrote:
> Ah, yes, that's true. This patch doesn't work indeed.
>
> Out of curiosity, using BumpPtrAllocator to allocate atoms is just for
> speed?

Yes, that's pretty much the only reason they are used. Faster to
allocate, and more compact.

- Michael Spencer

>
> 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
>>>
>>
>>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>