Request for reviews (S): 7014874: Incorrect COOPs modes on solaris-{sparcv9, amd64} with ParallelGC

[Y. Srinivas Ramakrishna]

Hi Vladimir --

On 1/27/2011 3:10 PM, Vladimir Kozlov wrote:
> As Igor explained me only Parallel GC requires boundary
> page size alignment due to UseAdaptiveGCBoundary feature.

Hmm, I do see a lot of "align_size_up()" done on max_size specs
in CollectedHeap::initialize_size_info() and friends,
as well as in GenerationSpec::align(). I didn't follow the
logic through to see if that will or will not affect the
whole heap sizing behaviour wrt the issue
reported in the CR. The bug report states the
anomalous behaviour with ParallelScavenge heaps,
Have you tested G1CollectedHeap and GenCollectedHeap
to see if they do the right thing?

thanks.
-- ramki

>
> Vladimir
>
> Y. S. Ramakrishna wrote:
>> May be you need a "partition_aligned()" method that
>> takes an aligned whole and produces aligned partitions
>> thereof given a desired_partitioning request. It would
>> seem as though similar logic would need to apply to
>> other kinds of heaps... or do they do the right thing
>> already? OK, i suppose i should go look, since i am
>> supposed to be reviewing, not interviewing ;-)
>>
>> -- ramki
>>
>> On 01/27/11 14:49, Vladimir Kozlov wrote:
>>> Before rounding YG+OG == total heap size. To keep the same
>>> total size I need to round one size up and the other down.
>>> I still may not preserver the same total size if it is not
>>> rounded to the same or large alignment. But it is fine.
>>>
>>> I already fixed PG rounding before in 6984368 changes:
>>>
>>> src/share/vm/memory/collectorPolicy.cpp Tue Sep 14 17:19:35 2010 -0700
>>> @@ -32,7 +32,11 @@
>>> MaxPermSize = PermSize;
>>> }
>>> PermSize = MAX2(min_alignment(), align_size_down_(PermSize, min_alignment()));
>>> - MaxPermSize = align_size_up(MaxPermSize, max_alignment());
>>> + // Don't increase Perm size limit above specified.
>>> + MaxPermSize = align_size_down(MaxPermSize, max_alignment());
>>> + if (PermSize > MaxPermSize) {
>>> + PermSize = MaxPermSize;
>>> + }
>>>
>>> Vladimir
>>>
>>> Y. S. Ramakrishna wrote:
>>>> I don't understand the logic of the patch. Or rather, i do,
>>>> but i question if it is complete: why didn't you also align
>>>> the max size of YG and PG down in like manner? I think you
>>>> should or you would be open to the same issues, no?
>>>>
>>>> In fact, it seems as though, as a matter of uniform policy, all min's
>>>> should align upwards and all max's align downwards wherever (page-)alignment
>>>> is sought.
>>>>
>>>> -- ramki
>>>>
>>>>
>>>> On 01/27/11 14:18, Vladimir Kozlov wrote:
>>>>> http://cr.openjdk.java.net/~kvn/7014874/webrev
>>>>>
>>>>> Fixed 7014874: Incorrect COOPs modes on solaris-{sparcv9,amd64} with ParallelGC
>>>>>
>>>>> scale_by_NewRatio_aligned() aligns boundary between old
>>>>> and young gens to min_alignment() (64K). But code in
>>>>> ParallelScavengeHeap::initialize() aligns up both old and
>>>>> young gen sizes to large page size.
>>>>> As result total heap size could be increased by one large
>>>>> page and cross 4gb boundary preventing usage of 32-bit COOPs.
>>>>>
>>>>> Align old gen size down to keep specified heap size but
>>>>> not less than its old gen min size.