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

[Thomas Schatzl]

Hi all,

On Tue, 2013-02-26 at 11:01 -0800, Tao Mao wrote:
> Hi Bengt,
> 
> When I was refactoring to get another layer of abstraction, I found 
> there would be many duplicated naming and similar functionality in the 
> existing class G1ParGCAllocBuffer and the new class 
> MultiG1ParGCAllocBuffer, surely, in different abstraction level (e.g. 
> allocate(), words_remaining(), retire(), set_word_size(), etc.).
> 
> I'd like to know whether we are talking about the same idea. What do you 
> have in your mind to lay out the abstraction?
> 

Imo this kind of hiding the internals of how the allocation is a good
idea. It removes clutter from program flow. Ideally you should be able
to exchange occurences of G1ParGCAllocBuffer with
MultiG1ParGCAllocBuffer recompile, and everything works again (with
improved performance of course =)

That could be done by making MultiParGCAllocBuffer some descendent of
ParGCAllocBuffer, and the MultiG1ParGCAllocBuffer uses instances of
G1ParGCAllocBuffer. There should be no overlap in functionality.

Unfortunately ParGCAllocBuffer does not have a single virtual method
that can be cleanly overwritten. So it is necessary to introduce an
(abstract) base class of that that defines the interface for GC
allocation buffers.

Something like that:

class GCAllocBuffer : public CHeapObj<mtGC> {
public:
  virtual HeapWord* allocate(size_t word_sz) = 0;
  virtual void  undo_allocation(...) = 0;
  [..]
};

class ParGCAllocBuffer : public GCAllocBuffer {
  // the existing implementation overriding every/most of the base
classes' things
};

class G1ParGCAllocBuffer : public ParGCAllocBuffer {
  // add G1 specific stuff
};

class G1MultiParGCAllocBuffer : public GCAllocBuffer {
private:
  G1ParGCAllocBuffer _buffer_sequence[...]; // for clarity, keep with
the array proposal for now
public:
  // implement the GCAllocBuffer interface using the
G1ParGCAllocBuffers. There should be no implementation duplication
};

One other question: how do you know whether exactly two additional
G1ParGCAllocBuffers are the best solution? Maybe one is just fine?

One other, more flexible, method is to link G1ParGCAllocBuffers together
using an extra member in G1ParGCAllocBuffer (this is somewhat ugly from
a design point of view though, but G1ParGCAllocBuffer is mostly a helper
class for G1MultiParGCAllocBuffers). This will likely also simplify the
"swap_buffers()" method.

Additionally I think swap_buffers() is an implementation detail which
should be part of retirement of the G1MultiParGCAllocBuffer.

All imho of course.

I found some oddity (not directly related to this change) in the way how
overriding is used in G1ParGCAllocBuffer: the set_buf() method of
G1ParGCAllocBuffer hides (not overrides) set_buf() of ParGCAllocBuffer.
Is this intentional? The same with retire(). Both methods are not
virtual but "overridden" anyway.

Regards,
Thomas