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

[Duncan P. N. Exon Smith]

+llvm-commites

> On 2015-Mar-17, at 12:47, Rui Ueyama <ruiu at google.com> wrote:
> 
>> On Tue, Mar 17, 2015 at 10:19 AM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>> > On 2015-Mar-15, at 23:07, Rui Ueyama <ruiu at google.com> wrote:
>> >
>> > I think SmallVector guarantees the array it contains is contiguous in memory. That data structure does not work well with BumpPtrAllocator because garbage after extending the array is not reclaimed until the whole buffer is discarded. That makes memory consumption of a small vector from O(n) to O(n^2).
>> >
>> > That being said, that behavior might be acceptable if we rarely extend SmallVectors.
>> 
>> This sounds dangerous.
>> 
> Agreed.
> 
>> > So, maybe it's not a bad idea, but I don't know if it's good enough to add an additional parameter to SmallVector's constructor.
>> 
>> If anything, what you want is a recycling allocator.  If you just have one
>> vector, then you still get N^2 memory usage, but if you have at least O(N)
>> vectors, it should be amortized O(N) memory per vector.  (I don't know
>> whether that's true for this problem.)
>> 
>> But I'm not sure what the benefit is anyway.  Has anyone actually observed
>> a memory issue here?
> 
> Since currently we don't leak memory, there's no memory issue. It's more like API issue that we use a memory allocator that doesn't call destructors for allocated objects so we can't do anything that depends on destructors.

That's not true.  The allocator isn't in charge of calling destructors;
it's just in charge of freeing memory.  You can call destructors if you'd
like to.

> 
> What if we just use new instead of BumpPtrAllcoator? Would it really make the linker slow? We surely do something tricky here to use BumpPtrAllocator, which can be eliminated if we simply use new.

I don't know this code well enough to know whether it makes the linker
slower, but I imagine you'd want to profile this with large inputs before
changing it.

> 
> >
> > On Sun, Mar 15, 2015 at 10:49 PM, Yaron Keren <yaron.keren at gmail.com> wrote:
> > This may mean we need teach SmallVector to accept allocator like other ADT.
> >
> >
> > 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
> 
>