RFR(M): 7147848: [macosx] com.sun.management.UnixOperatingSystem uses hardcoded dummy values

[Staffan Larsen]

Apparently something went wrong with the webrev. Here is new one:

http://cr.openjdk.java.net/~sla/7147848/webrev.02/

Thanks Dmitry,
/Staffan

On 11 apr 2012, at 12:33, Dmitry Samersoff wrote:

> Staffan,
> 
> MacosxOperatingSystem.c
> 
> Probably something went wrong with webrev:
> 
> 48     if (kr != KERN_SUCCESS) {
> 49         return 0;
> 50     }
> 
> 
> 133     if (gettimeofday(&now, NULL)) {
> 134        return -1;
> 135     }
> 
> -Dmitry
> 
> 
> On 2012-04-11 11:52, Staffan Larsen wrote:
>> Thank you all for your comments!
>> 
>> New webrev here: http://cr.openjdk.java.net/~sla/7147848/webrev.01/
>> 
>> Changes:
>> - Fixed Dmitry's comments
>> - Updated TestTotalSwap.sh for Darwin (thanks Mandy)
>> 
>> I have verified the values manually by running a Java program (see
>> attachment) and comparing to top and other system utilities. I've also
>> compared output with a Linux system to compare magnitude of numbers. I
>> have run the various regression tests.
>> 
>> Thanks,
>> /Staffan
>> 
>> 
>> 
>> 
>> On 10 apr 2012, at 21:57, Mandy Chung wrote:
>> 
>>> Staffan, Roger,
>>> 
>>> There isn't any undocumented semantics other than what the
>>> specification for com.sun.management.OperatingSystemMXBean specifies
>>> (that is indicated by its method name):
>>> http://docs.oracle.com/javase/7/docs/jre/api/management/extension/index.html
>>> 
>>> As Roger suggests, you can do some sanity tests and compare the result
>>> with native commands (top or other CLI). There are a few regression
>>> tests in jdk/test/com/sun/management. In particular, you might want to
>>> update test/com/sun/management/TestTotalSwap.sh to check the swap
>>> space with a suitable macosx command, if there is one.
>>> 
>>> FYI. I'm not familiar with Mac OS X API and didn't review the code.
>>> 
>>> Mandy
>>> 
>>> On 4/10/2012 10:51 AM, rhoover wrote:
>>>> Scott, Steffan
>>>> 
>>>> (I am he one who put in the original lines of: return -1.0; // not
>>>> available being replaced by this patch)
>>>> 
>>>> I was reluctant to implement these functions because the linux code
>>>> was quite involved and it appeared to me that there might be some
>>>> additional semantics to what was implemented than what was indicated
>>>> by the function names alone.
>>>> 
>>>> I have not compared the code with the 'top' source, but it looks
>>>> plausible. As a sanity test, the function values being returned could
>>>> be printed by a java program and visually compared with the output of
>>>> 'top' as a system is loaded up. It might also be wise to run the same
>>>> java program on other platforms to make sure that the magnitude of
>>>> the numbers is in the same ballpark.
>>>> 
>>>> On Apr 10, 2012, at 10:21 AM, Scott Kovatch wrote:
>>>> 
>>>>> Regarding Apple, Roger Hoover would be a good person to look at
>>>>> this, as he's spent more time in the Darwin levels of the VM. I
>>>>> think he's still partially attached to the OpenJDK work.
>>>>> 
>>>>> -- Scott
>>>>> 
>>>>> On Apr 10, 2012, at 8:58 AM, Daniel D. Daugherty wrote:
>>>>> 
>>>>>> Staffan,
>>>>>> 
>>>>>> I reviewed it and I think it looks OK. I tried looking at the code
>>>>>> in MacosxOperatingSystem.c relative to the Linux version, but I think
>>>>>> it is easily possible to miss something subtle here.
>>>>>> 
>>>>>> You might try a direct ping to Mandy Chung since M&M was her area.
>>>>>> You might also try a direct ping to Mike Swingler to get an Apple
>>>>>> reviewer.
>>>>>> 
>>>>>> Dan
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> On 4/10/12 3:30 AM, Staffan Larsen wrote:
>>>>>>> Any takers for this review? (added core-libs-dev as well)
>>>>>>> 
>>>>>>> Thanks,
>>>>>>> /Staffan
>>>>>>> 
>>>>>>> On 3 apr 2012, at 15:39, Staffan Larsen wrote:
>>>>>>> 
>>>>>>>> Please review the following fix:
>>>>>>>> 
>>>>>>>> webrev:
>>>>>>>> http://cr.openjdk.java.net/~sla/7147848/webrev.00/<http://cr.openjdk.java.net/%7Esla/7147848/webrev.00/>
>>>>>>>> <http://cr.openjdk.java.net/~sla/7147848/webrev.00/<http://cr.openjdk.java.net/%7Esla/7147848/webrev.00/>>
>>>>>>>> bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7147848
>>>>>>>> 
>>>>>>>> This fix implements the missing functionality in
>>>>>>>> UnixOperatingSystem for Mac OS X. Any feedback on the
>>>>>>>> implementation is welcome as I am not very familiar with the APIs
>>>>>>>> in Mac OS X.
>>>>>>>> 
>>>>>>>> I have verified that the changes build on all platforms through
>>>>>>>> JPRT. The correctness has been verified manually by looking in
>>>>>>>> JConsole and running the tests in
>>>>>>>> test/java/lang/management/OperatingSystemMXBean
>>>>>>>> test/com/sun/management/OperatingSystemMXBean.
>>>>>>>> 
>>>>>>>> Thanks,
>>>>>>>> /Staffan
>> 
> 
> 
> -- 
> Dmitry Samersoff
> Java Hotspot development team, SPB04
> * There will come soft rains ...

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20120411/f7d33edb/attachment-0001.html