RFR: JDK-8077842 - Remove the level parameter passed around in GenCollectedHeap

Mikael Gerdin mikael.gerdin at oracle.com
Mon Jun 1 15:23:21 UTC 2015


Hi Jesper,


On 2015-05-28 17:57, Jesper Wilhelmsson wrote:
> Hi,
>
> Another non-trivial merge later the webrev looks like this:
>
> http://cr.openjdk.java.net/~jwilhelm/8077842/webrev.04
>
> I reran all the tests and it turned out that the new assert in
> defNewGeneration.cpp was assuming that the generations were set up,
> which was not true at startup. So I have changed the assert to:
>
> +DefNewGeneration::IsAliveClosure::IsAliveClosure(Generation* young_gen)
> : _young_gen(young_gen) {
> +  assert(_young_gen->kind() == Generation::ParNew ||
> +         _young_gen->kind() == Generation::DefNew, "Expected the young
> generation here");
>
> This is more like the old assert that also looked at the properties of
> the generation itself:
>
> -DefNewGeneration::IsAliveClosure::IsAliveClosure(Generation* g) : _g(g) {
> -  assert(g->level() == 0, "Optimized for youngest gen.");
>
> Compared to the other webrevs this is the only change modulo some code
> that was removed by recent changes and didn't need to be changed any more.
>
> Still need a Reviewer to have a look at this.

Now that you've introduced the Generation::Type enum, can you use that 
instead of doing pointer compares on Generation instances?
It "doesn't feel right" to do all these pointer compares even though I 
know they are singletons and so on.

ConcurrentMarkSweepGeneration::printOccupancy acesses the 
GenCollectedHeap::_young_gen field instead of using the accessor, is 
that intended?

Further cleanups:
Generation::update_gc_stats could be moved to CardGeneration (since 
that's the common superclass for old gens) if GenCollectedHeap::_old_gen 
had CardGeneration* as its type.

/Mikael

>
> Thanks,
> /Jesper
>
>
> Jesper Wilhelmsson skrev den 20/5/15 19:15:
>> Kim Barrett skrev den 14/5/15 00:03:
>>> On May 12, 2015, at 3:02 PM, Jesper Wilhelmsson
>>> <jesper.wilhelmsson at oracle.com> wrote:
>>>>
>>>> Thanks Kim!
>>>>
>>>> I reverted the latest change in PointerLocation.java and
>>>> CollectedHeap.java,
>>>> now we're back to casts :)
>>>>
>>>> That's the only change in this webrev:
>>>>
>>>> http://cr.openjdk.java.net/~jwilhelm/8077842/webrev.02/
>>>
>>> Looks ok.
>>>
>>> I’m assuming the (Java) GenCollectedHeap.getGen() will eventually be
>>> replaced
>>> with specific young/old-gen accessors?
>>>
>>
>> I filed JDK-8080765 to handle the cleanups in the SA.
>>
>> Since the restructure of the GC files went in I had to rebase my patch.
>> Mercurial handled it flawlessly and I only had to merge two files
>> where I added
>> a new include, genMarkSweep.cpp and vmGCOperations.hpp
>>
>> A new webrev with the new patch is available here:
>>
>> http://cr.openjdk.java.net/~jwilhelm/8077842/webrev.03
>>
>> Thanks,
>> /Jesper


More information about the hotspot-gc-dev mailing list