RFR: JDK-8301749: Tracking malloc pooled memory size [v3]

[Thomas Stuefe]

On Thu, 9 Feb 2023 11:03:05 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>>> > return -1.
>>> > You aim to mimic the malloc_info() API at os:: level, so you should return -1 in case malloc_info failed, including failure do to resolving issues.
>>> 
>>> Wouldn't that lead to a caller not knowing whether `malloc_info` itself failed or if our wrapper failed? 
>> 
>> Why would that matter?
>> 
>>> `malloc_info` also sets `errno`. I think it's better if we don't imitate `malloc_info`, by returning `-2` if `malloc_info` is missing, to distinguish the different states.
>> 
>> We kind of follow this style in a lot of the wrappers we have in os::. As it is now, your malloc_info is neither here nor there - looks and mostly behaves like malloc_info, but not completely. I'd either be consistent and return -1, or convert this to a clearly named wrapper function which should return bool.
>
>> > > return -1.
>> > > You aim to mimic the malloc_info() API at os:: level, so you should return -1 in case malloc_info failed, including failure do to resolving issues.
>> > 
>> > 
>> > Wouldn't that lead to a caller not knowing whether `malloc_info` itself failed or if our wrapper failed?
>> 
>> Why would that matter?
>> 
> This matters when a caller sees -1, reads errno, and then takes an action depending on errno. If it was our wrapper that failed, then this will be a "stale" errno and a faulty action will be taken. This matters in `MallocInfoDcmd::execute` because we send different error messages depending on whether the wrapper failed or `malloc_info(3)` failed.

Okay, fair enough. I would have thought the user cannot really do anything about it anyway, but your way is certainly more precise.

-------------

PR: https://git.openjdk.org/jdk/pull/12455