Request for review (s) - 8031323: Optionally align objects copied to survivor spaces

[Bengt Rutisson]

Hi Jon,

On 2014-03-17 05:40, Jon Masamitsu wrote:
> 8031323: Optionally align objects copied to survivor spaces
>
> Add the option to specify an alignment for allocations into the
> survivor spaces.  Survivor space allocations will overflow if the
> survivor spaces have not been increased.   The expected use
> is to align survivor space allocations to cache line sizes.
>
> http://cr.openjdk.java.net/~jmasa/8031323/webrev.00/

For G1 it looks to me like we are aligning objects in the old regions as 
well. I thought the intent was to only align in the survivor regions. Am 
I missing something?

Note that it is not enough to check the alloc purpose for G1 since 
G1CollectedHeap::par_allocate_during_gc() only takes the purpose as a 
hint and may still return buffers from other regions than expected for a 
purpose. But I guess that in this case it is not too important to get it 
exactly right, so it may be enough to check the alloc purpose value.


The code in PSYoungPromotionLAB::allcoate(), 
ParGCAllocBuffer::allocate_aligned() and 
ContiguousSpace::allocate_aligned() is very similar. Is there a way to 
consolidate these methods?


The code in CollectedHeap::align_allocation_or_fail() looks quite 
different from other places in the code base where we do padding. What 
do you think about doing something like this instead?

inline HeapWord* CollectedHeap::align_allocation_or_fail(HeapWord* addr, 
HeapWord* end, unsigned short alignment_in_bytes) {
   if (alignment_in_bytes <= ObjectAlignmentInBytes) {
     return addr;
   }

   assert(is_ptr_aligned(addr, HeapWordSize), "");
   assert(is_size_aligned(alignment_in_bytes, HeapWordSize), "");

   HeapWord* new_addr = (HeapWord*) align_pointer_up(addr, 
alignment_in_bytes);
   size_t padding = pointer_delta(new_addr, addr);

   if (padding == 0) {
     return addr;
   }

   if (padding < CollectedHeap::min_fill_size()) {
     padding += alignment_in_bytes / HeapWordSize;
     assert(padding >= CollectedHeap::min_fill_size(), "");
     new_addr = addr + padding;
   }

   if(new_addr > addr && new_addr < end) {
     CollectedHeap::fill_with_object(addr, padding);
     return new_addr;
   } else {
     return NULL;
   }
}

Thanks,
Bengt

>
> Jon