Remove the experimental option UseCGroupMemoryLimitForHeap?(Internet mail)

[linzang(臧琳)]

Dear Severin,

Thanks for your comments! 

The  issue I see is that if “UseCGroupMemoryLimitForHeap” is set to true, the "if {}" codes get run twice, once in physical_memory(), and once in the "if {}". And when works in container, the result is same no matter what value the “ UseCGroupMemoryLimitForHeap” is set to.

I agree it is a back port of JDK-8194086, and we should keep the flag for compatibility concern.

It seems to me that there is no high risk here have those dup codes, I am just curious about whether we should omit them:) 

Thanks,
Lin



> On Dec 16, 2019, at 9:13 PM, Severin Gehwolf <sgehwolf at redhat.com> wrote:
> 
> On Mon, 2019-12-16 at 10:26 +0000, linzang(臧琳) wrote:
>> Dear All,
>>                I would like to discuss with you whether the experimental option “UseCGroupMemoryLimitForHeap” is unnecessary now?
>>                I see there is only one place in the code using this flag, and the logic there is considering cgroup memory limit when calculating heap size, but it is duplicated with os::physical_memory() since there is same implementation for container support.
>>                So is it ok if I remove this flag?
> 
> It would be a compatibility concern. There are users out there which
> still use it. So we cannot just remove the flag and fail the VM startup
> if provided.
> 
> Perhaps removal of code in Arguments::set_heap_size like done for JDK
> 11 in JDK-8194086, but still accepting the VM option could be a
> possibility. Perhaps also printinting a warning that the option does
> nothing along the way would be good to.
> 
> It depends what you have in mind, but it seems - unless there is a good
> reason - it would be wiser to leave that code as-is.
> 
> Thanks,
> Severin
> 
> 
> 
>