RFR(L): 8220312: Implementation: NUMA-Aware Memory Allocation for G1, Logging (3/3)

[Thomas Schatzl]

Hi,

On 05.11.19 20:57, Kim Barrett wrote:
>> On Nov 5, 2019, at 3:24 AM, Stefan Johansson <stefan.johansson at oracle.com> wrote:
>> On 2019-11-05 07:22, sangheon.kim at oracle.com wrote:
>>> webrev:
>>> http://cr.openjdk.java.net/~sangheki/8220312/webrev.5
>>> http://cr.openjdk.java.net/~sangheki/8220312/webrev.5.inc
>>
>> This all looks good, but there is an else-if statement that you can simplify a bit:
>> src/hotspot/share/gc/g1/g1NUMA.cpp
>> ---
>> 297   if (preferred_node_index == active_node_index) {
>> 298     _matched[preferred_node_index]++;
>> 299   } else if (preferred_node_index != active_node_index &&
>> 300              active_node_index != G1NUMA::UnknownNodeIndex) {
>> 301     _mismatched[preferred_node_index]++;
>> 302   }
>>
>> The first condition in the else-if statement will always be true (or the if-branch will be taken) and can be removed.
> 
> Looks good to me too, except for that same else-if.  I don’t need a new webrev for that.
> 
> 

same.

Plus please rename NodeIndexCheckClosure to G1NodeIndexCheckClosure (I 
know that HeapRegion* does not have a G1 prefix either, but let's not 
add to the issue).

I believe this rename can be done without a re-review from me.

Thanks,
   Thomas