[PATCH] D40291: [Support] Add WritableMemoryBuffer class

Pavel Labath via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 22 02:37:50 PST 2017


Thank you for looking at this.

We don't need the RequiresNullTerminator flag and I don't anticipate
needing it in the future, so that can be easily removed. However, we
do need the IsVolatile flag, to avoid mmapping files that can easily
disappear.

This is also the main reason why I think something like MemoryBuffer
is better suited for this job than mapped_file_region -- it will hide
the mmap/malloc+read logic behind a unified interface. The ability to
avoid mmapping small files also sounds like a nice thing to have,
although I doubt it will have any measurable impact for us in
practice.

regards,
pavel


On 21 November 2017 at 18:09, Zachary Turner <zturner at google.com> wrote:
> I'm honestly not sure.  But, if there's not an immediate use case for them,
> I wouldn't mind removing them until someone needs them.
>
> On Tue, Nov 21, 2017 at 10:06 AM Rafael Avila de Espindola
> <rafael.espindola at gmail.com> wrote:
>>
>> Zachary Turner <zturner at google.com> writes:
>>
>> > A MutableMemoryBuffer need not be backed by mmap.  For example, you
>> > could
>> > just allocate some scratch bytes from the heap.  Allowing something like
>> > this to work with the same interface as an mmap is one of the primary
>> > advantages IMO, because it means you don't have to have different code
>> > paths for mmap and malloc.
>>
>> I agree. What about RequiresNullTerminator and IsVolatile? Do you need
>> to expose them?
>>
>> Cheers,
>> Rafael


More information about the llvm-commits mailing list