RFR: 7165755 OS Information much longer on linux than other platforms

Nils Loodin nils.loodin at oracle.com
Tue May 8 01:03:52 PDT 2012


Hrm, sorry about that people, The webrev number should have been bumped from 00 to 01. :)

http://cr.openjdk.java.net/~nloodin/7165755/webrev.01/

Nisse

On May 8, 2012, at 07:37 , David Holmes wrote:

> On 4/05/2012 6:26 PM, Nils Loodin wrote:
>> Updated this with pulling out some shared code to os_posix.cpp.
>> Some methods were different enough that this wasn't possible though.
>> 
>> Do you like this better?
>> 
>> Webrev:
>> http://cr.openjdk.java.net/~nloodin/7165755/webrev.00/
> 
> I don't see any os_posix.cpp changes here?
> 
> David
> -----
> 
> 
>> On May 2, 2012, at 14:08 , Nils Loodin wrote:
>> 
>>> 
>>> On May 2, 2012, at 14:00 , David Holmes wrote:
>>> 
>>>> Hi Nils,
>>>> 
>>>> On 2/05/2012 9:56 PM, Nils Loodin wrote:
>>>>>> Ignoring Windows (with prints very little) I'd say it is the printing of /proc/meminfo that is the main difference. Not sure why printing that was necessary ... but if we are going to remove it I think we need to know why it was added.
>>>>> Yes, that's the reason.
>>>>> Note that nothing is removed. The method still prints exactly the same info, but I introduced another method to print briefer info, to be kinder to tool developers.
>>>> 
>>>> The current one prints /proc/meminfo. You turned that code into print_full_memory_info but in the main routine you call print_memory_info. Was that a mistake?
>>> 
>>> YES! Glad you caught that :) Guess (or hope) it would have been caught in dump testing otherwise :)
>>> 
>>> Regards,
>>> Nils Loodin
>>> 
>>> 
>>>> 
>>>> David
>>>> 
>>>>> I really don't want to change the output for say, hs_err files, where I believe this info is used.
>>>>> 
>>>>>> 
>>>>>>> This can make it hard for tool writers to get a summary that look good and similar for multiple platforms (sizing of gui fields, having to parse info in the tool code etc)
>>>>>>> Lookin at the code, it's in some serious need of refactoring. It would be nice with a method to get a "brief" os info for these kinds of tools that looks similar on all platforms.
>>>>>>> 
>>>>>>> This is my suggested change:
>>>>>>> http://cr.openjdk.java.net/~nloodin/7165755/webrev.00/
>>>>>> 
>>>>>> Seems to me some of this could be factored into the top-level OS class if we shoehorn Windows into the same shape as the other OSes ;-)
>>>>> This was my first attempt also, but then a lot of empty windows-methods ensued, which was kind of ugly.
>>>>> 
>>>>>> Or at least perhaps put some of the common stuff into os_posix.cpp ?
>>>>> There's a thought!
>>>>> I'll investigate that route, it could get things to look nicer.
>>>>> 
>>>>> 
>>>>>> Cheers,
>>>>>> David
>>>>> Regards,
>>>>> Nils Loodin
>>>>> 
>>> 
>> 



More information about the serviceability-dev mailing list