RFR: JDK-8293313: NMT: Rework MallocLimit [v4]

Thomas Stuefe stuefe at openjdk.org
Fri Jan 27 07:07:31 UTC 2023


On Thu, 26 Jan 2023 17:33:26 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Revert strchrnul
>
> The rest looks good to me.
> 
> This is a really nice feature, thank you for doing this!

Hi @gerard-ziemski, thanks for your review! Could you take a look at the final version?

> src/hotspot/share/services/mallocLimit.cpp line 150:
> 
>> 148:     }
>> 149:   }
>> 150: }
> 
> I am not crazy about:
> 
> 
> static const char* flagnames[] = { "fatal", "oom" };
> 
> 
> which requires that we position the strings in `flagnames` in a way that matches `MallocLimitMode` enum values:
> 
> 
> enum class MallocLimitMode {
>   trigger_fatal = 0,
>   trigger_oom   = 1
> };
> 
> 
> Could we do instead:
> 
> 
> void MallocLimitSet::print_on(outputStream* st) const {
>   if (_glob.sz > 0) {
>     st->print_cr("MallocLimit: total limit: " PROPERFMT " (%s)",
>                  PROPERFMTARGS(_glob.sz), MallocLimitSet::limit_name(_glob.mode));
>   } else {
>     for (int i = 0; i < mt_number_of_types; i++) {
>       if (_cat[i].sz > 0) {
>         st->print_cr("MallocLimit: category "%s" limit: " PROPERFMT " (%s)",
>           NMTUtil::flag_to_enum_name(NMTUtil::index_to_flag(i)),
>           PROPERFMTARGS(_cat[i].sz), MallocLimitSet::limit_name(_cat[i].mode));
>       }
>     }
>   }
> }
> 
> 
> and add new API to `MallocLimitSet`:
> 
> 
>   static const char* const limit_name(MallocLimitMode l)  { return (l==MallocLimitMode::trigger_fatal)?"fatal":"oom"; }
> 
> 
> This way we also avoid the `(int)` casting from the `enum`.
> 
> We could also potentially use `MallocLimitSet::limit_name()` in `match_mode_flag()` instead of hardcoding "oom" and "fatal"

Okay, minus the exposure of the API. Its not needed outside of this file.

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

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


More information about the hotspot-dev mailing list