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

[Vladimir Kozlov]

Ramki,

I looked more on this. Gen collectors align Young and
Old gens sizes down. Look in collectorPolicy.cpp on methods
initialize_flags(). They align up MaxHeapSize but if it
is already specified with good rounding (as in my case)
then it stays the same:

   set_max_heap_byte_size(align_size_up(MaxHeapSize, max_alignment()));

G1 uses this size without any adjustment (G1CollectedHeap::initialize()):

   size_t max_byte_size = collector_policy()->max_heap_byte_size();

It seems, only Parallel GC modifies it.

Thanks,
Vladimir

Y. Srinivas Ramakrishna wrote:
> 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.
>