RFR (S): 7112912:Message "Error occurred during initialization of VM" on boxes with lots of RAM

[Jon Masamitsu]

Thomas,

Thanks for the extra effort.   A comment not about correctness
but more for readability.

> +  if (is_allocatable(upper_limit)) {
> +    *limit = upper_limit;
> +  } else if (upper_limit < min_allocation_size) {
> +    *limit = upper_limit;
> +  } else if (!is_allocatable(min_allocation_size)) {
> +    // we found that not even min_allocation_size is allocatable. Return it
> +    // anyway. There is no point to search for a better value any more.
> +    *limit = min_allocation_size;

In the first "else if" if you changed it to (upper_limit <= 
min_allocation_size)
can you eliminate the second "else if" like this

+  if (is_allocatable(upper_limit)) {
+    *limit = upper_limit;
+  } else if (upper_limit <= min_allocation_size) {
+    *limit = upper_limit;

And it might read more easily as

+  if (is_allocatable(upper_limit) ||
+upper_limit <= min_allocation_size) {
+    *limit = upper_limit;

Otherwise, looks good.

Jon

On 3/21/2013 11:41 PM, Thomas Schatzl wrote:
> On Thu, 2013-03-21 at 15:42 -0700, Tao Mao wrote:
>> On 3/18/13 12:59 AM, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>>     here is a new webrev for this issue. Changes are:
>>>
>>> - merged code for os::has_allocatable_memory_limit of linux/bsd/solaris
>>> into a single method.
>> So, where is the merged routine now?
> Simply put into os_posix.cpp.
> (http://cr.openjdk.java.net/~tschatzl/7112912/webrev.1/src/os/posix/vm/os_posix.cpp.udiff.html)
>
> This is similar to other OS methods shared by all Unix OSes, so that it
> is found during compilation in all these OSes. Ie. this follows current
> standards in that respect.
>
> As mentioned, the OS class is currently being refactored where
> inheritance instead of #include is used to share functionality with all
> OSes. At least I think this is the currently agreed upon approach by the
> runtime team as far as I could understand.
>
> Thanks,
> Thomas
>