RFR: 8337563: NMT: rename MEMFLAGS to MemTag [v5]

Coleen Phillimore coleenp at openjdk.org
Tue Sep 10 20:39:09 UTC 2024


On Mon, 9 Sep 2024 19:02:25 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> Please review this cleanup, where we rename `MEMFLAGS` to `MemTag`.
>> 
>> `MEMFLAGS` implies that we can use more than one at the same time, but those are exclusive values, so `MemTag` is a more suitable name.
>> 
>> This fix also includes a cleanup of all the related parameter names and local variable names.
>> 
>> Testing is pending...
>> 
>> Note: there is more history in old closed PRs [https://github.com/openjdk/jdk/pull/20497](https://github.com/openjdk/jdk/pull/20497) and [https://github.com/openjdk/jdk/pull/20472](https://github.com/openjdk/jdk/pull/20472)
>
> Gerard Ziemski has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix test

So many mem_tags.  It looks fine to me.  I didn't see that many "MemTag F"'s except in the ConcurrentHashTable.  You could change that in a further patch.  I would prefer a one letter template parameter name like maybe M (since T seems too generic).  But F doesn't really bother me that much.  It's not like typename E means that much either.

Update: if people want the template parameter to be MemTag MT, that's fine too.

src/hotspot/share/nmt/memTracker.hpp line 265:

> 263: 
> 264:   // MallocLimt: Given an allocation size s, check if mallocing this much
> 265:   // under category f would hit either the global limit or the limit for mem_tag.

I don't know what "category f" is.   Maybe reword as

    // MallocLimit: Given an allocation size s, check if allocating this much memory would hit the global limit or the
    // limit tagged with mem_tag.

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

Marked as reviewed by coleenp (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20872#pullrequestreview-2293706394
PR Comment: https://git.openjdk.org/jdk/pull/20872#issuecomment-2341965524
PR Review Comment: https://git.openjdk.org/jdk/pull/20872#discussion_r1752681558


More information about the serviceability-dev mailing list