[llvm-commits] [llvm] r151834 - in /llvm/trunk: lib/Support/Allocator.cpp unittests/Support/AllocatorTest.cpp

[Argyrios Kyrtzidis]

On Mar 1, 2012, at 1:03 PM, Benjamin Kramer wrote:

> 
> On 01.03.2012, at 21:36, Argyrios Kyrtzidis wrote:
> 
>> Author: akirtzidis
>> Date: Thu Mar  1 14:36:32 2012
>> New Revision: 151834
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=151834&view=rev
>> Log:
>> If BumpPtrAllocator is requested to allocate a size that exceeds the slab size,
>> increase the slab size.
>> 
>> Modified:
>>   llvm/trunk/lib/Support/Allocator.cpp
>>   llvm/trunk/unittests/Support/AllocatorTest.cpp
>> 
>> Modified: llvm/trunk/lib/Support/Allocator.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Allocator.cpp?rev=151834&r1=151833&r2=151834&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Support/Allocator.cpp (original)
>> +++ llvm/trunk/lib/Support/Allocator.cpp Thu Mar  1 14:36:32 2012
>> @@ -87,15 +87,21 @@
>> /// Allocate - Allocate space at the specified alignment.
>> ///
>> void *BumpPtrAllocator::Allocate(size_t Size, size_t Alignment) {
>> +  // 0-byte alignment means 1-byte alignment.
>> +  if (Alignment == 0) Alignment = 1;
>> +
>> +  size_t PaddedSize = Size + sizeof(MemSlab) + Alignment - 1;
>> +
>> +  // If requested size exceeds slab size, increase slab size.
>> +  while (PaddedSize > SlabSize)
>> +    SlabSize *= 2;
> 
> Have you measured that this doesn't make us waste a ton of memory?

Without my change BumpPtrAllocator will just corrupt memory when "PaddedSize > SlabSize"; do you have an alternative way in mind to handle this ?

> Also this makes the "separate slab" code below redundant.

Not sure what you mean, could you elaborate ?

> 
> BumpPtrAllocator already (slowly) grows the allocation size when a lot of stuff is allocated, see StartNewSlab. Back then I tweaked the parameters there to minimize wasted memory in clang's ASTContext. If you're seeing a lot of malloc calls it may make sense to revisit the parameters there or initialize an allocator with a bigger slab size.

I'm not interested in reducing malloc calls, the issue is that if the BumpPtrAllocator starts with a small size and then you try to allocate a bigger size than that, you either get assertion or corrupted memory, see the unit test case later in then commit.

-Argyrios

> 
> - Ben
> 
>> +
>>  if (!CurSlab) // Start a new slab if we haven't allocated one already.
>>    StartNewSlab();
>> 
>>  // Keep track of how many bytes we've allocated.
>>  BytesAllocated += Size;
>> 
>> -  // 0-byte alignment means 1-byte alignment.
>> -  if (Alignment == 0) Alignment = 1;
>> -
>>  // Allocate the aligned space, going forwards from CurPtr.
>>  char *Ptr = AlignPtr(CurPtr, Alignment);
>> 
>> @@ -106,7 +112,6 @@
>>  }
>> 
>>  // If Size is really big, allocate a separate slab for it.
>> -  size_t PaddedSize = Size + sizeof(MemSlab) + Alignment - 1;
>>  if (PaddedSize > SizeThreshold) {
>>    MemSlab *NewSlab = Allocator.Allocate(PaddedSize);
>> 
>> 
>> Modified: llvm/trunk/unittests/Support/AllocatorTest.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/AllocatorTest.cpp?rev=151834&r1=151833&r2=151834&view=diff
>> ==============================================================================
>> --- llvm/trunk/unittests/Support/AllocatorTest.cpp (original)
>> +++ llvm/trunk/unittests/Support/AllocatorTest.cpp Thu Mar  1 14:36:32 2012
>> @@ -93,6 +93,14 @@
>>  EXPECT_EQ(2U, Alloc.GetNumSlabs());
>> }
>> 
>> +// Test allocating with a size larger than the initial slab size.
>> +TEST(AllocatorTest, TestSmallSlabSize) {
>> +  BumpPtrAllocator Alloc(128);
>> +
>> +  Alloc.Allocate(200, 0);
>> +  EXPECT_EQ(1U, Alloc.GetNumSlabs());
>> +}
>> +
>> // Mock slab allocator that returns slabs aligned on 4096 bytes.  There is no
>> // easy portable way to do this, so this is kind of a hack.
>> class MockSlabAllocator : public SlabAllocator {
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>