RFR (M): 8016749: -XX:+UseISM fails an assert(obj->is_oop()) when running SPECjbb2005

[Vladimir Kozlov]

Looks good. I submitted push job.

regards,
Vladimir

On 7/10/13 1:06 PM, Albert Noll wrote:
> Hi Valdimir,
>
> thanks for looking at the patch. I implemented all changes that you
> proposed.
> Here is the new webrev:
> http://cr.openjdk.java.net/~anoll/8016749/webrev.03/
> <http://cr.openjdk.java.net/%7Eanoll/8016749/webrev.03/>
>
> Could I get a second review for this?
>
> Mant thanks,
> Albert
>
>
> On 10.07.2013 07:50, Vladimir Kozlov wrote:
>> In general it is good. Few comments about style:
>>
>> Add () for (&& (>)))
>>
>> if (UseLargePages && (alignment_hint > (size_t)vm_page_size())) {
>>
>> In os::large_page_init() reverse condition to execute code in method
>> instead of check and return.
>>
>> Add space: //Upon
>>
>> Don't convert to one line next code:
>>
>> 5435     if (!UseNUMA && ForceNUMA) {
>> 5436       UseNUMA = true;
>> 5437     }
>>
>> Thanks,
>> Vladimir
>>
>> On 7/3/13 12:41 AM, Albert Noll wrote:
>>> Hi Valdimir,
>>>
>>> you are right. These methods should not be called when there is no
>>> support for ISM. I added the fatal(#method_name
>>> should not be called on Solaris). In addition, I found more code in
>>> os_solaris.cpp that is only there to provide support
>>> for Solaris 8. I removed that code as well.
>>>
>>> A jprt run was successful. I also checked that large pages can still
>>> be used.
>>>
>>> Many thanks for your help,
>>> Albert
>>>
>>> Here is the new webrev:
>>> http://cr.openjdk.java.net/~anoll/8016749/webrev.02/
>>> <http://cr.openjdk.java.net/%7Eanoll/8016749/webrev.02/>
>>>
>>>
>>>
>>> On 02.07.2013 17:14, Vladimir Kozlov wrote:
>>>> On 7/2/13 8:07 AM, Vladimir Kozlov wrote:
>>>>> Albert,
>>>>>
>>>>> reserve_memory_special() had assert which indicated that it is only
>>>>> called with UseISM.
>>>>> Look on code in runtime/virtualspace.cpp -
>>>>> os::reserve_memory_special() is only call when
>>>>> os::can_commit_large_page_memory() return false. With your changes
>>>>> it always return 'true' (which is correct).
>>>>> I think you can replace whole body of reserve_memory_special() with
>>>>> fatal("this obsolete method should not be called on
>>>>> Solaris"); or something.
>>>>
>>>> Remove 'obsolete' from the message sine the method is not obsolete
>>>> on other platforms.
>>>>
>>>> Vladimir
>>>>
>>>>> The same is for release_memory_special() but, please, verify.
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 7/2/13 6:41 AM, Albert Noll wrote:
>>>>>> Christian, Vladimir, thanks for your feedback.
>>>>>>
>>>>>> I added the removed flags to the ObsoleteFlag list.
>>>>>>
>>>>>> Vladimir, you are right. Removing all mpss-related code does not
>>>>>> allow to use large pages anymore. I followed your
>>>>>> suggestions and replaced 'UseMPSS' with 'UseLargePages'. I also
>>>>>> checked if large pages work with 'pmap -xs' when the
>>>>>> changes in webrev.01 (see below).
>>>>>>
>>>>>> Here is the new webrev:
>>>>>> http://cr.openjdk.java.net/~anoll/8016749/webrev.01/
>>>>>> <http://cr.openjdk.java.net/%7Eanoll/8016749/webrev.01/>
>>>>>>
>>>>>> A jprt run with the above changes was successful.
>>>>>>
>>>>>> Thanks again,
>>>>>> Albert
>>>>>>
>>>>>> On 02.07.2013 04:28, Vladimir Kozlov wrote:
>>>>>>> Albert,
>>>>>>>
>>>>>>> I don't think it is incorrect changes. Only UseISM is deprecated.
>>>>>>> We still use MPSS for large pages:
>>>>>>>
>>>>>>> // UseLargePages UseMPSS UseISM
>>>>>>> //     true      true     false => JVM will use MPSS for large
>>>>>>> page memory.
>>>>>>> //                                 This is the default behavior.
>>>>>>>
>>>>>>> //     true      false    false => Unless future Solaris provides
>>>>>>> other
>>>>>>> //                                 mechanism to use large page
>>>>>>> memory, this
>>>>>>> //                                 combination is equivalent to
>>>>>>> -UseLargePages,
>>>>>>> //                                 VM will not use large page memory
>>>>>>>
>>>>>>>
>>>>>>> UseMPSS is true by default and most likely it will not work
>>>>>>> without it. So you should leave current functionality but
>>>>>>> replace UseMPSS with UseLargePages flag checks in os_solaris.cpp.
>>>>>>>
>>>>>>> Also verify that large pages are used by java (pmap -xs) after
>>>>>>> your changes.
>>>>>>>
>>>>>>> thanks,
>>>>>>> Vladimir
>>>>>>>
>>>>>>> On 7/1/13 8:35 AM, Albert Noll wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> thanks for reviewing this patch:
>>>>>>>>
>>>>>>>> jbs: https://jbs.oracle.com/bugs/browse/JDK-8016749
>>>>>>>> webrev: http://cr.openjdk.java.net/~anoll/8016749/webrev.00/
>>>>>>>> <http://cr.openjdk.java.net/%7Eanoll/8016749/webrev.00/>
>>>>>>>>
>>>>>>>> Problem: ISM and MPSS are part of Solaris 9, and we do not
>>>>>>>> support this
>>>>>>>> version of Solaris anymore. Instead, we use LargePages for
>>>>>>>> Solaris 10+.
>>>>>>>>
>>>>>>>> Solution: Remove all code that relates to ISM and MPSS (i.e.,
>>>>>>>> code that
>>>>>>>> relates to the 'UseISM' and 'UseMPSS' flags).
>>>>>>>>
>>>>>>>> Testing: the following jprt run passes successfully.
>>>>>>>>
>>>>>>>> jprt submit -stree . -ot 'solaris.*' -emailalbert.noll at oracle.com
>>>>>>>>
>>>>>>>> Best,
>>>>>>>> Albert
>>>>>>
>>>
>