Request for review (s) - 8015851 :UseAdaptiveGCBoundary is broken

[Jon Masamitsu]

On 6/7/2013 2:21 AM, Bengt Rutisson wrote:
>
> Hi Jon,
>
> On 6/6/13 2:25 AM, Jon Masamitsu wrote:
>> Fixed the CR # in the subject.
>>
>> On 6/5/2013 1:27 PM, Jon Masamitsu wrote:
>>> A recent changeset effectively removed the initialization of
>>> performance counters used  by the option UseAdaptiveGCBoundary
>
> Which change broke the initialization of the performance counters?

I didn't track down the particular changeset.  The breakage was reported to
me on the latest build.  I can hunt it down if you like.

>
> One comment:
>
> The code you add in ASPSOldGen::initialize_work() is very similar to 
> the code in PSOldGen::initialize(). The PSOldGen::initialize() method 
> is called from one (why not both?!?!) of the constructors for 
> PSOldGen, which means that it is called from one of the ASPSOldGen 
> constructors. But these constructors seem to be unused. Would it be 
> possible to move your code from ASPSOldGen::initialize_work() in to 
> the constructor of PSOldGen that is actually used?

The short answer is that I can move the code in 
ASPSOldGen::initialize_work() into
the 2nd PSOldGen that does no initialization.  And will.

For why ASPSOldGen::initialize_work() and PSOldGen::initialize() are 
similar but
different and about the PSOldGen constructors (which I think are both 
actually used).

Many (maybe most) of the Generation types have constructors that take a 
ReservedSpace
and initialize based on the ReserveSpace.  ASPSOldGen and ASPSYoungGen 
share a
ReservedSpace.  I thought that passing in the addresses and sizes of the 
part of the
ReservedSpace to be used by each generation (ASPSOldGen and ASPSYoungGen)
was more transparent than to pass the ReservedSpace to each generation and
let each generation take a piece.  That is, I thought that doing the 
division of
the space in the ReservedSpace intended for each generation in the code 
where
each generation was created was better (a single spot) than having it in two
separate places (the constructors for the generation).  So there is a 
difference
between how PSOldGen and ASPSOldGen are initialized so using 
PSOldGen::initialize()
with ASPSOldGen doesn't work without so additional refactoring.

I removed one of the constructors for ASPSOldGen in the cleanups so 
there is only

   ASPSOldGen(PSVirtualSpace* vs,
              size_t initial_byte_size,
              size_t minimum_byte_size,
              size_t byte_size_limit,
              const char* gen_name, int level);

which explicitly uses

   PSOldGen(size_t initial_size, size_t min_size, size_t max_size,
            const char* perf_data_name, int level);

which is the constructor that does not do the initialization.

Thanks for looking at this and for the suggection about moving the code
into the 2nd PSOldGen constructor.

Jon

>
> Thanks,
> Bengt
>
>>>
>>> This changeset is built on top of the one for 8014546 and the
>>> webrev comes in two parts.
>>>
>>> The fix plus the test
>>>
>>> http://cr.openjdk.java.net/~jmasa/8015851/webrev.00/
>>>
>>> Some cleanups of the code
>>>
>>> http://cr.openjdk.java.net/~jmasa/8015851/webrev_refactor.00/
>>>
>>> Thanks.
>>>
>>> Jon
>>
>