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