RFR: 8075635 - Remove GenerationSpec array

Kim Barrett kim.barrett at oracle.com
Mon Mar 23 21:59:45 UTC 2015


On Mar 23, 2015, at 1:10 PM, Jesper Wilhelmsson <jesper.wilhelmsson at oracle.com> wrote:
> 
> Hi,
> 
> Please review another follow up after the generation array removal. In this change the array of generation specifications is reduced to two variables in the same way as was done for the generations.
> 
> The reference to the genSpecs that was previously duplicated in GenCollectedHeap is now only available through the collector policy class.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8075635
> Webrev: http://cr.openjdk.java.net/~jwilhelm/8075635/webrev.00

------------------------------------------------------------------------------
src/share/vm/gc_implementation/concurrentMarkSweep/cmsCollectorPolicy.cpp
src/share/vm/memory/collectorPolicy.hpp
src/share/vm/memory/generationSpec.hpp

Needs copyright update.

------------------------------------------------------------------------------ 
agent/src/share/classes/sun/jvm/hotspot/memory/GenCollectedHeap.java
 131     if (level == 0) {
 132       return (GenerationSpec)
 133               VMObjectFactory.newObject(GenerationSpec.class,
 134                       youngGenSpecField.getAddress());
 135     } else {
 136       return (GenerationSpec)
 137               VMObjectFactory.newObject(GenerationSpec.class,
 138                       oldGenSpecField.getAddress());
 139     }

This is effectively assuming that nGens() == 2, but the earlier
assertions in this function don't reflect that assumption.  I realize
there will probably be further cleanup in this area later, but I'd
like things to be in a consistent state at each step in these
incremental changes.

Similar issue here:
src/share/vm/memory/generation.cpp
  67   return level() == 0 ? gch->gen_policy()->young_gen_spec() : gch->gen_policy()->old_gen_spec();

------------------------------------------------------------------------------
src/share/vm/memory/genCollectedHeap.cpp
 151   size_t total_reserved = young_spec->max_size() + old_spec->max_size();
 152   if (total_reserved < young_spec->max_size() ||
 153       total_reserved < old_spec->max_size()) {

Not that it's all that important, but either of those comparisons
alone should be sufficient to test for overflow.  It might also be
nice to leave a comment about this being a test for overflow.

------------------------------------------------------------------------------
src/share/vm/memory/generationSpec.hpp
  42   GenerationSpec(Generation::Name name, size_t init_size, size_t max_size, size_t alignment) {
  43     _name = name;
  44     _init_size = align_size_up(init_size, alignment);
  45     _max_size = align_size_up(max_size, alignment);
  46   }

I like that you've eliminated the separate post-construction init_size
and max_size alignment step and are instead doing that in the
constructor.

Is there anything that ensures init_size <= max_size?  Should there be
an assertion to that effect here?

Also consider initializing members in ctor-initializer rather than
assignments in the constructor body.

------------------------------------------------------------------------------




More information about the hotspot-gc-dev mailing list