[llvm] r289579 - ADT: Add OwningArrayRef class.
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 20 15:17:02 PST 2016
> On 2016-Dec-20, at 14:41, John McCall <rjmccall at apple.com> wrote:
>
>> On Dec 20, 2016, at 11:57 AM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> I tend to agree with John/Peter that a dynarray-like data structure is worth having around. There are use cases where the memory overhead of std::vector is relevant; this strips it down a little. It seems useful.
>>
>> However, I'm skeptical of having a container inherit from ArrayRef (or MutableArrayRef). Not all of the ArrayRef API really makes sense for a container. And anything named ArrayRef is going to trigger people into thinking that operations are "cheap", when in point of fact, every operation here involves a malloc/delete[] pair.
>
> Well, if by "every operation" you mean "just the operation of constructing it from an ArrayRef".
There are many things that are implicitly convertible to ArrayRef (and thus to OwningArrayRef), but yeah, that might have been an overstatement.
>
>> If we're going to have this, I think it should be called llvm::DynArray or something.
>
> I would not object to this being a unrelated type as long as you can easily get a MutableArrayRef from it.
I agree this is a good idea.
> John.
>
>>
>>> On 2016-Dec-19, at 23:12, David Blaikie via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>>
>>> Fair point-ish. Though my argument is a bit different since this is has a more direct analogy with something in the standard than some of the other ADTs we have that have more clear trade-offs against the things in the standard. In this case it's basically the exact thing that was considered and not standardized, as I understand it. So figure there might be some context there that could be relevant.
>>>
>>>
>>> On Mon, Dec 19, 2016, 10:00 PM John McCall <rjmccall at apple.com> wrote:
>>>> On Dec 19, 2016, at 9:47 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>> On Mon, Dec 19, 2016 at 9:24 PM Peter Collingbourne <peter at pcc.me.uk> wrote:
>>>> Sure, I understood what you meant. I meant that I wouldn't take a position on whether avoiding the cost of the capacity and the reserve area is worth it.
>>>>
>>>> (If pressed I think I'd say no, the average tu doesn't have that many vtables, and there are far more egregious wastes of memory in llvm anyway (e.g. llvm::DIE) that we should be concentrating on first, but while I was adding another thing to vtables I figured it wouldn't hurt to be consistent with the others, then rule of three kicked in so seemed reasonable to add the abstraction.)
>>>>
>>>> Yeah, that's pretty much how I feel too.
>>>>
>>>> Richard, Chandler - I seem to recall this has come up before (whether or not LLVM would benefit from a dynarray like abstraction) & I don't remember the backstory on the standards committee for dynarary (which I would've only heard second hand from one of you, I think). Any extra context/thoughts you could share here, briefly?
>>>
>>> Is your argument really that we should intentionally pessimize something because the committee decided not to standardize a similar container? LLVM's entire ADT library is basically a laundry list of micro-optimizations that we felt at some point or another had advantages over what the STL provides.
>>>
>>> John.
>>>
>>>>
>>>> - Dave
>>>>
>>>>
>>>> Peter
>>>>
>>>>
>>>> On Dec 19, 2016 20:29, "David Blaikie" <dblaikie at gmail.com> wrote:
>>>> What I mean is: compare OwningArrayRef to std::vector, not OwningArrayRef to manual memory management
>>>>
>>>> On Mon, Dec 19, 2016 at 8:07 PM Peter Collingbourne <peter at pcc.me.uk> wrote:
>>>> I don't really have a strong opinion about it. It wraps up some manual memory management code we used to have in the vtable builder, although I couldn't say how beneficial that memory management really is.
>>>>
>>>> Peter
>>>>
>>>> On Mon, Dec 19, 2016 at 7:56 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>> Is this really worth having compared to std::vector? std::dynarray was rejected from standardization for that reason, if I understand/heard correctly (& OwningArrayRef seems similar to std::dynarray)
>>>>
>>>> On Tue, Dec 13, 2016 at 12:34 PM Peter Collingbourne via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>>> Author: pcc
>>>> Date: Tue Dec 13 14:24:24 2016
>>>> New Revision: 289579
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=289579&view=rev
>>>> Log:
>>>> ADT: Add OwningArrayRef class.
>>>>
>>>> This is a MutableArrayRef that owns its array.
>>>> I plan to use this in D22296.
>>>>
>>>> Differential Revision: https://reviews.llvm.org/D27723
>>>>
>>>> Modified:
>>>> llvm/trunk/include/llvm/ADT/ArrayRef.h
>>>>
>>>> Modified: llvm/trunk/include/llvm/ADT/ArrayRef.h
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ArrayRef.h?rev=289579&r1=289578&r2=289579&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/include/llvm/ADT/ArrayRef.h (original)
>>>> +++ llvm/trunk/include/llvm/ADT/ArrayRef.h Tue Dec 13 14:24:24 2016
>>>> @@ -413,6 +413,25 @@ namespace llvm {
>>>> }
>>>> };
>>>>
>>>> + /// This is a MutableArrayRef that owns its array.
>>>> + template <typename T> class OwningArrayRef : public MutableArrayRef<T> {
>>>> + public:
>>>> + OwningArrayRef() {}
>>>> + OwningArrayRef(size_t Size) : MutableArrayRef<T>(new T[Size], Size) {}
>>>> + OwningArrayRef(ArrayRef<T> Data)
>>>> + : MutableArrayRef<T>(new T[Data.size()], Data.size()) {
>>>> + std::copy(Data.begin(), Data.end(), this->begin());
>>>> + }
>>>> + OwningArrayRef(OwningArrayRef &&Other) { *this = Other; }
>>>> + OwningArrayRef &operator=(OwningArrayRef &&Other) {
>>>> + delete this->data();
>>>> + this->MutableArrayRef<T>::operator=(Other);
>>>> + Other.MutableArrayRef<T>::operator=(MutableArrayRef<T>());
>>>> + return *this;
>>>> + }
>>>> + ~OwningArrayRef() { delete this->data(); }
>>>> + };
>>>> +
>>>> /// @name ArrayRef Convenience constructors
>>>> /// @{
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>>
>>>>
>>>> --
>>>> --
>>>> Peter
>>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
More information about the llvm-commits
mailing list