[aarch64-port-dev ] Fwd: RFR(S): 8159056: [aix] Compressed class space not allocated in lower regions

[Volker Simonis]

Forwarding to aarch64-dev with the kindly request for a review :)

Regards,
Volker

---------- Forwarded message ----------
From: David Holmes <david.holmes at oracle.com>
Date: Thu, Jun 9, 2016 at 9:27 AM
Subject: Re: RFR(S): 8159056: [aix] Compressed class space not
allocated in lower regions
To: "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com>,
"hotspot-runtime-dev at openjdk.java.net"
<hotspot-runtime-dev at openjdk.java.net>


On 9/06/2016 5:07 PM, Lindenmaier, Goetz wrote:
>
> HI David,
>
> thanks for looking at this!
>
>> I will sponsor this but need an Aarch64 reviewer to confirm  the
>> behaviour there is unchanged. If 4G is the first value that should
>> succeed I'm not sure what the impact of trying and failing at 1G, 2G and
>> 3G are.
>
> I think I don't really understand.  It should only try at 1-3G on aix if
> requested_addr is that small.  On aarch, it is aligned to 4G at the
> start of the loop.
> Do you think there are implications with the OS? Or with the loop
> for shared spaces below?


My bad.

> I fixed the other issues:
> http://cr.openjdk.java.net/~goetz/wr16/8159056-aix_CCSTest/webrev.02/


Looks good. Still need a second reviewer though.


Thanks,
David


> Best regards,
>   Goetz.
>
>
>>
>> More below.
>>
>> On 9/06/2016 12:21 AM, Lindenmaier, Goetz wrote:
>>>
>>> Hi,
>>>
>>> On AIX, test CompressedClassPointers.java fails. This is because
>>
>> allocate_metaspace_compressed_klass_ptrs()
>>>
>>> tries only a single fixed address, which fails on aix.  This change uses the
>>
>> loop
>>>
>>> implemented for aarch to search for a possible location.
>>> The change does not affect any other platform. I had to edit the aarch
>>
>> code, but
>>>
>>> it's supposed to behave as before.
>>>
>>> Please review this change. I please need a sponsor.
>>> http://cr.openjdk.java.net/~goetz/wr16/8159056-aix_CCSTest/webrev.01/
>>
>>
>> 2958     // the base, 4G alignemnt is helpful, too.
>>
>> Typo: alignment
>>
>> 2959     size_t incrememnt = AARCH64_ONLY(4*)G;
>>
>> Typos: increment
>>
>> 2963       if (a == (char *)(32*G)) { incrememnt = 4*G; } // Go faster
>> from here on. Zero-based is no more possible.
>>
>> Please split into multiple lines. "no more possible" is not correct
>> grammar in this context suggest "no longer possible".
>>
>> Thanks,
>> David
>>
>>> Best regards,
>>>   Goetz.
>>>