RFR (S): JDK-8020829: JT_HS: 2 runtime NMT tests fail on platforms if NMT detail is not supported

[David Holmes]

On 21/08/2013 1:50 PM, Christian Tornqvist wrote:
>> I hate seeing these kinds of methods. Add a new platform, add a new method
> - yuck! :(
>
> We're not adding platforms at a rate where this is really a problem

The point is that every time we do add a platform - and there are a 
couple more in the pipeline now - we have to find all these places where 
the known platforms have been hardwired. It is in hotspot code, JDK code 
and test code. That isn't scalable or maintainable and at best should be 
done in one place.

>> boolean isArch(String arch) {
>>    return osArch.toLowerCase().startsWith(arch.toLowerCase());
>> }
>>
>> and similarly for the OS - but that is going beyond this fix :)
>
> This is really a bad idea, the idea is to make it easy and less error prone
> to do these checks, not the other way around. I strongly disagree with this
> and think this change should _not_ be made.

I agree the OS usage would be a problem simply because a lot of people 
would be unaware of the obscure historical strings used for the OS "name".

Anyway no change is happening - including not adding isArm().

Cheers,
David

>
> /Christian
>
> -----Original Message-----
> From: hotspot-runtime-dev-bounces at openjdk.java.net
> [mailto:hotspot-runtime-dev-bounces at openjdk.java.net] On Behalf Of David
> Holmes
> Sent: Tuesday, August 20, 2013 9:26 PM
> To: Chris Plummer
> Cc: hotspot-runtime-dev at openjdk.java.net
> Subject: Re: RFR (S): JDK-8020829: JT_HS: 2 runtime NMT tests fail on
> platforms if NMT detail is not supported
>
> Hi Chris,
>
> On 21/08/2013 5:33 AM, Chris Plummer wrote:
>> https://jbs.oracle.com/bugs/browse/JDK-8020829
>> http://cr.openjdk.java.net/~cjplummer/8020829/webrev-03/
>>
>> On some platforms, NMT detail is not supported, and this was causing
>> some jtreg tests to fail. These tests now query a new WhiteBox API to
>> see if NMT detail is supported, and now behave properly if it is not
>> supported.
>
> Okay.
>
>> I also fixed #ifdef INCLUDE_NMT in whitebox.cpp that should instead be
>> #if INCLUDE_NMT. The source within the #if is now properly excluded
>> from minimal VM builds.
>
> Okay.
>
>> The addition of Platform.isArm() is to support changes in closed source.
>
> I hate seeing these kinds of methods. Add a new platform, add a new method -
> yuck! :(
>
> We could just use getOsArch().toLowerCase().startsWith("arm") directly.
>
> I'd even prefer to see:
>
> boolean isArch(String arch) {
>     return osArch.toLowerCase().startsWith(arch.toLowerCase());
> }
>
> and similarly for the OS - but that is going beyond this fix :)
>
> Thanks,
> David
>
>> Testing was done using the existing NMT tests, verifying that they now
>> pass on platforms where NMT detail is not supported, and still pass on
>> platforms where NMT detail is supported. A jprt job is currently in
>> the queue to run all NMT jtreg tests on all platforms supported by jprt.
>>
>> Thanks!
>>
>> Chris Plummer
>