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