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