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

[Jesper Wilhelmsson]

On 2012-07-03 16:34, Vladimir Kozlov wrote:
> 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.
>
I do have the closed directories in there, could it be that jprt doesn't find 
them? I'm using -stree when running jprt.
I though that I'd get compiler errors rather than a core dump if there were 
missing files. Is there something in the build process that is dumping core?
/Jesper

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