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

[Jesper Wilhelmsson]

I had to clone a new repository to convince JPRT that I had everything in 
order, but now the UseNUMA patch passes.
/Jesper


On 2012-07-03 17:44, Jesper Wilhelmsson wrote:
> 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.
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>