Request for review: 6976350 G1: deal with fragmentation while copying objects during GC
jon.masamitsu at oracle.com
Tue Feb 5 05:46:12 UTC 2013
On 2/4/2013 12:56 PM, Tao Mao wrote:
> a new webrev created (I need more reviews!)
> 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.
> On 2/1/2013 2:58 PM, Jon Masamitsu wrote:
>> 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.
>> 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...
Ok. In practical terms you're probably right that the benefit would be
>> 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.
I don't think there is a convention that says we shouldn't use
If I'm wrong, someone please let me know.
>> GCAllocPriority is an enum but you use them as integers in the for
>> 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.
True. GCAllocPurpose's don't have an order the way the GCAllocPriority
but I think the code ends up more readable.
>> On 1/28/2013 12:21 PM, Tao Mao wrote:
>>> 6976350 G1: deal with fragmentation while copying objects during GC
>>> 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.
More information about the hotspot-gc-dev