Request for review (XS): 8006431: os::Bsd::initialize_system_info() sets _physical_memory too large

[Bengt Rutisson]

Hi David,

Thanks for looking at this!

On 1/18/13 8:12 AM, David Holmes wrote:
> Hi Bengt,
>
> I must say the docs for these sysctl values is not very good. Nothing 
> states that HW_USERMEM is 32-bit, and logically it makes more sense to 
> use the amount of available memory for sizing things rather than the 
> amount of physical memory. :(

I think you are right that it would make sense to get the "usable" 
memory for sizing the heap and so on. But the method I'm changing is 
called physical_memory(). It just happens to be used for sizing. So, in 
this case I think it is more correct to use the actual physical memory. 
If I understand the implementations of os::physical_memory() for the 
other platforms it looks like they also return the actual physical 
memory and not "usable" memory.

> It would seem that HW_USERMEM is newer than the "deprecated" 32-bit 
> HW_PHYSMEM, so when was it introduced? If I go to the 10.4-intel 
> version of the manpage you link there is no HW_MEMSIZE listed.

I have not been able to figure out exactly when HW_MEMSIZE was 
introduced. But it is available in 10.7, which is the earliest version 
that hotspot supports.

Bengt

>
>
> David
>
> On 18/01/2013 4:36 PM, Bengt Rutisson wrote:
>>
>> Hi all,
>>
>> Could I have a couple of reviews for this small fix?
>>
>> http://cr.openjdk.java.net/~brutisso/8006431/webrev.00/
>>
>> On OSX we used HW_USERMEM value from sysctl() to get the available
>> physical memory on the machine. This is a 32 bit value but we store it
>> in a 64 bit variable. This means that we get kind of random and normally
>> very large values for what we think the physical memory is.
>>
>> We actually don't want a 32 bit value since we want to handle machines
>> with more than 2 or 4 GB of memory. Instead of HW_USERMEM we should be
>> using HW_MEMSIZE which will give us a 64 bit value.
>>
>> See:
>> https://developer.apple.com/library/mac/#documentation/Darwin/Reference/ManPages/man3/sysctl.3.html 
>>
>>
>> Thanks Staffan Larsen for both detecting the problem and providing a
>> solution.
>>
>> This is a one-word-change. So, to save you a mouse click on the webrev
>> link above, here is the diff:
>>
>> --- a/src/os/bsd/vm/os_bsd.cpp
>> +++ b/src/os/bsd/vm/os_bsd.cpp
>> @@ -260,7 +260,7 @@
>> * instead of hw.physmem because we need size of allocatable memory
>> */
>> mib[0] = CTL_HW;
>> - mib[1] = HW_USERMEM;
>> + mib[1] = HW_MEMSIZE;
>> len = sizeof(mem_val);
>> if (sysctl(mib, 2, &mem_val, &len, NULL, 0) != -1)
>> _physical_memory = mem_val;
>>
>> Thanks,
>> Bengt