RFR(M): 8190308: Supporting heap allocation on alternative memory devices and CSR review

sangheon.kim sangheon.kim at oracle.com
Fri Nov 3 21:38:12 UTC 2017


Hi Kishor,

On 11/03/2017 11:39 AM, Kharbas, Kishor wrote:
> Thanks a lot!
>
> Link to updated webrevs -
> http://cr.openjdk.java.net/~kkharbas/8190308/webrev.13/
> http://cr.openjdk.java.net/~kkharbas/8190308/webrev.13_to_12/
Thank you for fixing all.
Looks good to me except below.

Could you update the copyright format in TestAllocateHeapAt.java?
2  * Copyright (c) 2017 Oracle and/or its affiliates. All rights reserved.
- Missing comma: * Copyright (c) 2017_*,*_ Oracle and/or its affiliates. 
All rights reserved.

Thanks,
Sangheon


>
> @Sangheon: Please let me know if you see any corrections needed.
>
> -Kishor
>
>> -----Original Message-----
>> From: Thomas Schatzl [mailto:thomas.schatzl at oracle.com]
>> Sent: Friday, November 3, 2017 7:31 AM
>> To: Kharbas, Kishor <kishor.kharbas at intel.com>; sangheon.kim
>> <sangheon.kim at oracle.com>; 'hotspot-gc-dev at openjdk.java.net'
>> <hotspot-gc-dev at openjdk.java.net>; hotspot-runtime-
>> dev at openjdk.java.net
>> Subject: Re: RFR(M): 8190308: Supporting heap allocation on alternative
>> memory devices and CSR review
>>
>> Hi,
>>
>> On Fri, 2017-11-03 at 08:55 +0000, Kharbas, Kishor wrote:
>>> Hi Sangheon,
>>>
>>> Thanks for the review and comments. Here is an updated webrev-
>>> http://cr.openjdk.java.net/~kkharbas/8190308/webrev.12
>>> http://cr.openjdk.java.net/~kkharbas/8190308/webrev.12_to_11
>>>
>>> In addition to your suggested corrections, I added code to set Linux
>>> core dump filter ensuring Heap is dumped correctly when this feature
>>> is used. This is follow-up to Jini George’s comment
>>> (http://openjdk.5641.n7.nabble.com/RFR-M-8171181-Supporting-heap-
>>> allocation-on-alternative-memory-devices-td300109.html#a300450).
>> Some minor nits:
>>
>>   - os_posix.cpp:300: please move the else next to the brace
>>   - arguments.cpp:4624: please add a space between "if" and the bracket
>>
>> I do not need to see a new webrev for these changes. Looks good.
>>
>> Thanks,
>>    Thomas



More information about the hotspot-runtime-dev mailing list