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

[Evan Cheng]

Hi Reid,

Thanks. A couple of comments.

Can you change CheckInvariants() to return true / false and the error  
message by reference instead?

+  /// 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?

+    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,

Evan

On Jul 13, 2009, at 4:57 PM, Reid Kleckner wrote:

> On Tue, Jun 23, 2009 at 11:11 PM, Chris Lattner<clattner at apple.com>  
> wrote:
>> Have you run through the llvm testsuite with the initial threshold  
>> cranked
>> way down?  This would cause the test suite to check out whether the
>> reallocation stuff is working.  If that works, it looks good to me.
>
> So, getting back to this patch, I went ahead and did all the FIXMEs in
> JITMemoryManager.cpp.  This means that it will not run out of memory
> for function stubs, global data, or code until the OS gives up.
>
> I've added some unit tests which now pass, and the SPASS test passes
> with lli.  I had to modify lli to not allocate globals with code,
> though, but I think that should eventually become the default.  I'm
> running the rest of the nightly tests right now.
>
> I've attached the patch and here's the Rietveld link if you prefer:
> http://codereview.appspot.com/71042/show
>
> Here's a description of how the patch works:
>
> For stubs and global data, the memory manager uses the simple
> BumpAllocator class, which keeps a vector of memory slabs and a pair
> of pointers that it bumps along to make allocations.  For especially
> large allocations, it allocates a separate slab.  This is done so that
> in the worst case it wastes at most an amount of space equal to the
> threshold at the end of the block, or a page at the end of the
> separate slab.
>
> For code, the memory manager adds the ability to request more memory
> by allocating more code slabs and adding them to the free list.  If
> memory gets fragmented, this could become a problem, since each
> allocation loops over the entire free list.
>
> When the JITEmitter runs out of space in the buffer it got on its
> first try, it now frees the memory it has been working with so far,
> asks for twice as much memory, and tries again until it succeeds.
>
> Please review!
>
> Thanks,
> Reid
> <JITMemoryManager.diff>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits