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

Christian Tornqvist christian.tornqvist at oracle.com
Tue Aug 20 20:50:13 PDT 2013


>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

>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.

/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



More information about the hotspot-runtime-dev mailing list