RFR (S/M): 8213827: NUMA heap allocation does not respect process membind/interleave settings [Was: Re: [PATCH] JDK NUMA Interleaving issue]

[amith pawar]

Hi Sangheon,

Thanks again. I have done the required changes and created webrev. Please
use following link to download the same as gmail is not allowing to attach.

https://drive.google.com/open?id=1QzmW6LdmKbBNHp4-hlcIr9DKY7anUMBy

Thanks,
Amit

On Thu, Jan 10, 2019 at 1:03 AM <sangheon.kim at oracle.com> wrote:

> Hi Amith,
>
> On 1/9/19 3:30 AM, amith pawar wrote:
>
> Hi Sangheon,
>
> Thanks for reviewing and updated with suggested changes. please check.
>
> Thank you for addressing my comments.
> But I can't see below comments addressed:
>
> - Looking at 'enum' at os.hpp, we use Camel style.
>
> I meant to change from 'Numa_allocation_policy' to 'NumaAllocationPolicy'.
>
> - As usual, copyright year updates. I know it was correct when you posted.
> :)
>
> Looking at the latest source code, only os_linux.hpp needs a new copyright
> year.
> - * Copyright (c) 1999, 2018, Oracle and/or its affiliates. All rights
> reserved.
> + * Copyright (c) 1999, 2019, Oracle and/or its affiliates. All rights
> reserved.
>
> Looking at the v5,
> +      ls.print("UseNUMA is enabled and invoked in '%s' mode."
> +                " Heap will be configured using NUMA memory nodes:",
> numa_mode);
> There is one more space before " Heap.... ", please remove it.
>
> I see the latest version that Thomas posted is v3, but your attached
> version is v5. :)
>
> In addition, it would be better to provide webrev instead of a patch. (
> http://openjdk.java.net/guide/codeReview.html )
>
> Thanks,
> Sangheon
>
>
> Thanks,
> Amit Pawar
>
> On Wed, Jan 9, 2019 at 12:45 AM <sangheon.kim at oracle.com> wrote:
>
>> Hi Thomas,
>>
>> On 12/13/18 2:33 AM, Thomas Schatzl wrote:
>>
>> Hi Amit,
>> On Thu, 2018-12-13 at 15:11 +0530, amith pawar wrote:
>>
>> Hi Thomas,
>>
>> Please find the attached patch updated as per your suggestion.
>> If everything OK then can you please commit this to repo ?
>>
>>   looks good. We will need a second reviewer though, I am going to ask
>> around.
>>
>> Latest webrev:http://cr.openjdk.java.net/~tschatzl/8213827/webrev.3/
>>
>> Webrev.3 looks good to me.
>>
>> I have some minor nits:
>> ----------------------------------------
>> src/hotspot/os/linux/os_linux.cpp
>> 5012       for (int node = 0; node < Linux::numa_max_node(); node++) {
>> - Looks like 'node <*=* Linux::numa_max_node()' is the right one to
>> print the latest node?
>>
>> ----------------------------------------
>> src/hotspot/os/linux/os_linux.hpp
>>  271   enum Numa_allocation_policy{
>> - Looking at 'enum' at os.hpp, we use Camel style.
>> - There are missing space before '{'.
>>
>> - As usual, copyright year updates. I know it was correct when you
>> posted. :)
>>
>> Thanks,
>> Sangheon
>>
>>
>> Thanks,
>>   Thomas
>>
>>
>>
>>
>>
>
> --
> With best regards,
> amit pawar
>
>
>

-- 
With best regards,
amit pawar