On Mon, 9 Sep 2024 19:02:25 GMT, Gerard Ziemski <gziem...@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
