[llvm-commits] [unladen-swallow] Re: Patch to allow the JITMemoryManager to allocate more blocks of memory

[Evan Cheng]

On Jul 17, 2009, at 11:25 AM, Reid Kleckner wrote:

> Here is the updated patch.  I will commit the changes to
> Support/Allocator.(h|cpp) as a separate patch, but I assume you'll
> want to review all the changes here.  It passes make check, modulo 7
> failures that look related to my old version of llvm-gcc.

Looks good to me. I'll let someone else review the Allocator changes.

Thanks,

Evan

>
> Reid
>
> On Wed, Jul 15, 2009 at 10:41 PM, Evan Cheng<evan.cheng at apple.com>  
> wrote:
>>
>>
>> On Jul 14, 2009, at 11:39 AM, Reid Kleckner wrote:
>>
>>> On Mon, Jul 13, 2009 at 10:37 PM, Evan Cheng<evan.cheng at apple.com>  
>>> wrote:
>>>>
>>>> Hi Reid,
>>>>
>>>> Thanks. A couple of comments.
>>>>
>>>> Can you change CheckInvariants() to return true / false and the  
>>>> error
>>>> message by reference instead?
>>>
>>> Sure.  That's nice because it makes the ostream deconstructor will
>>> automatically flush things, whereas it didn't before.
>>>
>>>> +  /// BumpAllocator - This is a simple memory allocator that  
>>>> simply
>>>> bumps a
>>>> +  /// pointer forwards across a slab of memory until it runs  
>>>> out, at
>>>> which
>>>> +  /// point it allocates a new slab.  If the size of an  
>>>> allocation is
>>>> above
>>>> a
>>>> +  /// certain threshold, the allocator allocates a separate slab  
>>>> for it,
>>>> to
>>>> +  /// avoid wasted space.  We use this class to allocate  
>>>> function stubs
>>>> and
>>>> +  /// global data structures.
>>>> +  class BumpAllocator {
>>>>
>>>> Is there a reason not to use the BumpPtrAllocator in Support/ 
>>>> Allocator.h?
>>>
>>> That allocator uses malloc.  One of the things I was trying to do is
>>> get all the code and globals laid out contiguously in memory by  
>>> using
>>> the NearBlock parameter of sys::Memory::AllocateRWX.  That isn't
>>> particularly reliable, and it's not even supported on Windows.  It
>>> would still be nice to get all slab allocation centralized through  
>>> the
>>> memory manager instead of through malloc, though.
>>>
>>> I could parameterize the allocator in Support to use either malloc  
>>> or
>>> the JITMemoryManager, or I could just use mine or the one in Support
>>> wholesale.  Which would you prefer?
>>
>> How about changing the allocator in Support to use AllocateRWX? On  
>> Windows,
>> can it fallback to malloc?
>>
>> Evan
>>>
>>>> +    for (MemoryRangeHeader *Hdr = (MemoryRangeHeader*)Start,  
>>>> *LastHdr =
>>>> NULL;
>>>> +         Start <= (char*)Hdr && (char*)Hdr < End;
>>>> +         Hdr = &Hdr->getBlockAfter()) {
>>>> +      if (Hdr->ThisAllocated == 0) {
>>>>
>>>> How about "if (!Hdr->ThisAllocated) continue;" to reduce  
>>>> indentation.
>>>> Thanks,
>>>
>>> That can't be done, since the 'if' doesn't cover the entire body of
>>> the for loop.  I did notice that the 'else if' clause will never
>>> execute because ThisAllocated is a one bit field, so I dropped that.
>>>
>>> Thanks for the review,
>>> Reid
>>> < 
>>> JITMemoryManager.diff 
>>> >_______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
> <JITMemoryManager.diff>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits