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

[Evan Cheng]

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