Review request (S): 7179517 Enable NUMA by default on NUMA hardware

[Vladimir Kozlov]

On 7/3/12 3:54 AM, Jesper Wilhelmsson wrote:
> Hi Vladimir,
>
> My IDE wanted me to add os.hpp, I have removed it now.
> I have added UseNUMAInterleaving so that the Solaris patch is identical to the linux patch.
> New webrev (same link as before):
> http://cr.openjdk.java.net/~jwilhelm/7179517/webrev/

Looks good.

>
> I'm not sure how to handle Solaris though. It won't get the same amount of testing as the other platforms. The latest
> patch has not passed trough JPRT yet, I get weird failures. The failures are not on Solaris so I don't see how they can
> be related to this change.

You should ask when you have problems. It seems, you forgot to clone src/closed and test/closed into your repository.

Thanks,
Vladimir

> /Jesper
>
>
> On 2012-07-02 21:31, Vladimir Kozlov wrote:
>> Hi Jasper,
>>
>> Why you added #include "runtime/os.hpp"? It is compiled without it (it is
>> included in an other header file). You would not be able to compile this file
>> since it define methods declared in os.hpp.
>>
>> UseNUMAInterleaving is set 'true' only for Parallel GC in arguments.cpp. New
>> code set it 'true' for all GCs which may cause problem. Also there is solaris
>> code which depends on this flag and it is not set in this changes.
>>
>> Thanks,
>> Vladimir
>>
>> Jesper Wilhelmsson wrote:
>>> I have updated the webrev with a Solaris patch as well. It's the same URL,
>>> just do a reload in your browser.
>>>
>>> I'm not a Solaris native either, so regarding testing I have asked the
>>> performance team to look extra close on Solaris to make sure it behaves as
>>> expected.
>>> /Jesper
>>>
>>>
>>> On 2012-07-02 19:26, Vladimir Kozlov wrote:
>>>> That question was not for you but for Jesper.
>>>>
>>>> Vladimir
>>>>
>>>> Eric Caspole wrote:
>>>>> I am hoping one of you guys can do it - I have never used Solaris and I
>>>>> don't know if there is any special thing to take into account there.
>>>>>
>>>>>
>>>>> On Jul 2, 2012, at 12:07 PM, Vladimir Kozlov wrote:
>>>>>
>>>>>> Why there is no changes for Solaris?
>>>>>>
>>>>>> Vladimir
>>>>>>
>>>>>> On 7/2/12 3:57 AM, Jesper Wilhelmsson wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> This change is to turn on UseNUMA per default on NUMA hardware. It has
>>>>>>> been contributed by Eric Caspole. I'm fine with
>>>>>>> the change but we need at least one official reviewer.
>>>>>>>
>>>>>>> Webrev:
>>>>>>> http://cr.openjdk.java.net/~jwilhelm/7179517/webrev/
>>>>>>>
>>>>>>> Testing:
>>>>>>> I ran it through JPRT and did some local sanity testing.
>>>>>>> Eric has tested this on SPECjbb2005 and SPECjvm2008 with a variety of heap
>>>>>>> sizes and it is never worse than the current
>>>>>>> default. In some cases, especially with Windows, running a test with the
>>>>>>> current default and getting bad NUMA placement
>>>>>>> will be about 3x slower than the +UseNUMA score.
>>>>>>>
>>>>>>> The performance team are involved to perform some larger scale testing.
>>>>>>>
>>>>>>
>>>>>
>>>>>