RFR (T) 8243393: Improve ReservedSpace constructor resolution

[David Holmes]

On 29/04/2020 10:38 pm, coleen.phillimore at oracle.com wrote:
> 
> I already checked this in because it was trivial.

Didn't seem trivial to me.

> On 4/28/20 11:53 PM, David Holmes wrote:
>> Hi Coleen,
>>
>> On 28/04/2020 2:12 am, coleen.phillimore at oracle.com wrote:
>>> Summary: Remove possibly ambiguous constructor and use directly in 
>>> ReservedCodeHeap
>>>
>>> Tested with tier1-3.
>>>
>>> open webrev at 
>>> http://cr.openjdk.java.net/~coleenp/2020/8243393.01/webrev
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8243393
>>
>> Sorry but I don't understand the mapping to the constructors. In this 
>> original call from cardTable.cpp:
>>
>> ReservedSpace heap_rs(_byte_map_size, rs_align, false);
>>
>> this calls the 4-arg constructor as-if:
>>
>> ReservedSpace heap_rs(_byte_map_size, rs_align, false, NULL);
>>
>> and rs_align maps to the alignment parameter.
>>
>> But in the new two-arg constructor call:
>>
>> ReservedSpace heap_rs(_byte_map_size, rs_align)
>>
>> the rs_align is no longer treated as alignment but rather 
>> preferred_page_size! It isn't at all obvious to me that this is an 
>> equivalent construction.
>>
> http://cr.openjdk.java.net/~coleenp/2020/8243393.01/webrev/src/hotspot/share/gc/shared/cardTable.cpp.udiff.html 
> 
> 
> Yes, you're right, this is part of the change is wrong.  It was matching 
> the version I took out.  I'll file a bug to fix it.

Okay.

>> Both constructors need to be adequately documented - at present only 
>> the two-arg constructor is.
> 
> Please suggest a comment.

Ha! I don't understand what the args to these constructors are supposed 
to indicate - that is the problem.

Cheers,
David
-----

> thanks,
> Coleen
>>
>> Thanks,
>> David
>> http://cr.openjdk.java.net/~coleenp/2020/8243393.01/webrev/src/hotspot/share/gc/shared/cardTable.cpp.udiff.html 
>>
>>> Thanks,
>>> Coleen
>