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

[Coleen Phillimore]

Christian's on a plane.  This code looks fine.
Coleen

On 08/22/2013 01:21 AM, David Holmes wrote:
> On 22/08/2013 5:12 AM, Chris Plummer wrote:
>> New webrev posted. The only change from the previous one is dropping the
>> Platform.java changes.
>>
>> http://cr.openjdk.java.net/~cjplummer/8020829/webrev-04/
>
> Looks fine to me - thanks.
>
> Still need a second reviewer though. Christian?
>
> David
>
>> thanks,
>>
>> Chris
>>
>> On 8/20/13 6:55 PM, David Holmes wrote:
>>> On 21/08/2013 11:47 AM, Chris Plummer wrote:
>>>> On 8/20/13 6:26 PM, David Holmes wrote:
>>>>> 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 :)
>>>> I'm not particularly tied to any specific way of doing this. I can do
>>>> either of the above if you wish. Just let me know your preference.
>>>
>>> Drop isArm() please and change the closed usage to
>>> getOsArch().toLowerCase().startsWith("arm").
>>>
>>> I'll file a RFE for Platform :)
>>>
>>> Thanks,
>>> David
>>>
>>>> Chris
>>>>>
>>>>> 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
>>>>
>>