Request for review: 6976350 G1: deal with fragmentation while copying objects during GC

[Tao Mao]

a new webrev created (I need more reviews!)
http://cr.openjdk.java.net/~tamao/6976350/webrev.01/

I took Jon's suggestion for bracketing for loops.

For the rest of suggestions, I thought of them when coding but I also 
had another concern to myself not to have them. So, I'd like to discuss 
before I do further changes.

Please see inline and provide some insights on my concerns.

Thank you.
Tao

On 2/1/2013 2:58 PM, Jon Masamitsu wrote:
>
> http://cr.openjdk.java.net/~tamao/6976350/webrev.00/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp.frames.html 
>
>
> line 4386
>
> Please add the brackets for the outer for loop.  I'm not used to 
> seeing it
> without the brackets even though they are not strictly needed.
>
> http://cr.openjdk.java.net/~tamao/6976350/webrev.00/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp.frames.html 
>
>
> line 1821 same comment about brackets for for-statement.
>
> line 1869
>
> You always retire alloc_buf1.  Did you consider retiring the buffer 
> that has the
> less words_remaining?
Why not to leverage info of words_remaining:
1) simplicity.
2) The current mechanism to use the alloc_buffers in order (in routine 
allocate() ) makes alloc_buf1 stochastically filled up first.
3) The current changeset will make the situation better. Do we want an 
additional overhead (getting words_remaining) to trade for a little more 
benefits? Maybe...

>
> line 1762
>
> Can you make _alloc_buffers two dimensional?  Then you wouldn't need the
> buf_idx() method.
I rarely saw any 2D array implementation at least within GC code base. I 
didn't know whether this was a coincidence or some "convention". So I 
was some reluctant to do so.

>
> GCAllocPriority is an enum but you use them as integers in the for loops.
> Can you make the for loop index variable an enum GCAllocPriority?
Making enum GCAllocPriority as for loop index is not a big deal. But the 
problem is that, for consistency, we should make GCAllocPurpose as for 
loop index as well, which has values of GCAllocForTenured and 
GCAllocForSurvived, and don't make as much sense as GCAllocPriority1 and 
GCAllocPriority2 do.

>
> Jon
>
>
>
>
>
> On 1/28/2013 12:21 PM, Tao Mao wrote:
>> 6976350 G1: deal with fragmentation while copying objects during GC
>> https://jbs.oracle.com/bugs/browse/JDK-6976350
>>
>> webrev:
>> http://cr.openjdk.java.net/~tamao/6976350/webrev.00/
>>
>> changeset:
>> Basically, we want to reuse more of par-allocation buffers instead of 
>> retiring it immediately when it encounters an object larger than its 
>> remaining part.
>>
>> (1) instead of previously using one allocation buffer per GC purpose, 
>> we use N(=2) buffers per GC purpose and modify the corresponding 
>> code. The changeset would easily scale up to whatever N (though Tony 
>> Printezis suggests 2, or 3 may be good enough)
>>
>> *(2) Two places of cleanup: allocate_during_gc_slow() is removed due 
>> to its never being called.
>>                                               access modifier 
>> (public) before trim_queue() is redundant.
>>
>>