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

Afshin Zafari azafari at openjdk.org
Thu Sep 12 09:18:07 UTC 2024


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];`

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

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


More information about the serviceability-dev mailing list