Hi Jesper,
On Tuesday 21 January 2014 22.49.19 Jesper Wilhelmsson wrote:
> Hi,
>
> Could I have a few reviews of this change?
>
> Summary:
> To allow applications a more fine grained control over the GC over time,
> we'll make the flags MinHeapFreeRatio and MaxHeapFreeRatio manageable.
>
> The initial request that lead up to this change involved ParallelGC which is
> notoriously unwilling to shrink the heap. Since ParallelGC didn't support
> the heap free ratio flags, this change also includes implementing support
> for these flags in ParallelGC.
>
> Changes have also been made to the argument parsing, attach listener and the
> management API to verify the flag values when set through the different
> interfaces.
>
> Webrev: http://cr.openjdk.java.net/~jwilhelm/8028391/webrev.4/
In psAdaptiveSizePolicy.cpp:
83 ParallelScavengeHeap* heap = (ParallelScavengeHeap*)Universe::heap();
You can use ParallelScavengeHeap::heap() here instead.
I had an offline discussion with Jesper with notebooks and formulas to figure
out how Min/MaxHeapFreeRatio should be used and how to change the psScavenge
change.
I have a suggestion for how to do verification as well:
if you add a
bool Flag::is_equal(void* flagptr) const { return _addr == flagptr; }
you can have a sort of generic verify_flag function with only a pointer check
and some ifs instead of doing string compares:
template <typename T>
bool CommandLineFlags::verify_flag(Flag* flag, T value) {
if (flag->is_equal(&MinHeapFreeRatio)) {
if (is_percentage(value)) {
}
if (value <= MaxHeapFreeRatio) {
}
}
if (flag->is_equal(&MaxHeapFreeRatio)) { ... }
return something;
}
Another, perhaps too complex, approach would be to trade the _type char* for a
vtable pointer for Flags and have BooleanFlag, FloatFlag, etc. and make the
_type field based on virtual dispatch.
Then each flag which needed verification could override a virtual verify()
function.
I'm not going to insist on changing this but we really need to fix this if we
add one more manageable flag which needs bounds checking or some other type of
verification.
/Mikael
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8028391
>
> The plan is to push this to 9 and then backport to 8 and 7.
>
> Thanks!
> /Jesper
