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

[Gerard Ziemski]

On Wed, 11 Sep 2024 14:03:17 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> Gerard Ziemski has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Coleen's feedback
>
> Are we sure we want `mt` for non-type parameter name in templates? We have these existing patterns already in our code:
> 
> 
> src/hotspot/share/utilities/growableArray.hpp:803:template <typename E, MemTag MT>
> src/hotspot/share/utilities/stack.hpp:54:template <class E, MemTag MT> class StackIterator;
> src/hotspot/share/utilities/concurrentHashTable.inline.hpp:78:template <typename CONFIG, MemTag MT>
> src/hotspot/share/utilities/chunkedList.hpp:31:template <class T, MemTag MT> class ChunkedList : public CHeapObj<MT> 
> src/hotspot/share/gc/g1/g1BatchedTask.hpp:32:template <typename E, MemTag MT>
> src/hotspot/share/gc/shared/taskqueue.hpp:119:template <unsigned int N, MemTag MT>
> src/hotspot/share/gc/shared/taskqueue.hpp:327:template <class E, MemTag MT, unsigned int N = TASKQUEUE_SIZE>
> src/hotspot/share/gc/shenandoah/shenandoahTaskqueue.hpp:40:template<class E, MemTag MT, unsigned int N = TASKQUEUE_SIZE>
> src/hotspot/share/nmt/arrayWithFreeList.hpp:34:template<typename E, MemTag MT>
> 
> 
> With mt they would look like:
> 
> 
> src/hotspot/share/utilities/growableArray.hpp:803:template <typename E, MemTag mt>
> src/hotspot/share/utilities/stack.hpp:54:template <class E, MemTag mt> class StackIterator;
> src/hotspot/share/utilities/concurrentHashTable.inline.hpp:78:template <typename CONFIG, MemTag mt>
> src/hotspot/share/utilities/chunkedList.hpp:31:template <class T, MemTag mt> class ChunkedList : public CHeapObj<mt> 
> src/hotspot/share/gc/g1/g1BatchedTask.hpp:32:template <typename E, MemTag mt>
> src/hotspot/share/gc/shared/taskqueue.hpp:119:template <unsigned int N, MemTag mt>
> src/hotspot/share/gc/shared/taskqueue.hpp:327:template <class E, MemTag mt, unsigned int N = TASKQUEUE_SIZE>
> src/hotspot/share/gc/shenandoah/shenandoahTaskqueue.hpp:40:template<class E, MemTag mt, unsigned int N = TASKQUEUE_SIZE>
> src/hotspot/share/nmt/arrayWithFreeList.hpp:34:template<typename E, MemTag mt>
> 
> 
> So `MT` or `mt` for non-type parameter name in templates, or should I punt on this particular change and leave it for a followup?

> Thank you @gerard-ziemski, for this huge change. After this change, the code looks much more nicer and consistent.
> 
> If we are insisting on replacing `flag` with `tag`, I could find these missed ones by regexp search for `mem.*flag`:
> 
> 7 results - 5 files
> 
> Source root • src/hotspot/share/nmt/memMapPrinter.cpp: `83: // A Cache that correlates range with MEMFLAG, optimized to be iterated quickly`
> 
> Source root • src/hotspot/share/nmt/memTracker.hpp: `208: // memory flags of the original region.`
> 
> Source root • src/hotspot/share/nmt/vmatree.hpp:
> 
> `97: assert(!(type == StateType::Released) || data.mem_tag == mtNone, "Released type must have flag mtNone");`
> 
> `108: return static_cast<MemTag>(type_flag[1]);`
> 
> Source root • test/hotspot/gtest/nmt/test_nmt_reserved_region.cpp: `50: ASSERT_EQ(region2.mem_tag(), mtThreadStack); // Should be correct flag`
> 
> Source root • test/hotspot/gtest/nmt/test_vmatree.cpp:
> 
> `435: const MemTag candidate_flags[candidates_len_flags] = {`
> 
> `459: const MemTag mem_tag = candidate_flags[os::random() % candidates_len_flags];`

Thank you Ashfin for laking such a detailed look and providing the feedback.

I fixed the issues, except:

`108:       return static_cast<MemTag>(type_flag[1]);`
This actually has more than just MemTag, it also has StateType, so I left it as is.

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

PR Comment: https://git.openjdk.org/jdk/pull/20872#issuecomment-2346602184