RFR: 8337563: NMT: rename MEMFLAGS to MemTag

[Gerard Ziemski]

On Sat, 7 Sep 2024 05:24:29 GMT, Kim Barrett <kbarrett 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)
>
> src/hotspot/share/runtime/os.hpp line 918:
> 
>> 916:   static ssize_t recv(int fd, char* buf, size_t nBytes, uint type);
>> 917:   static ssize_t send(int fd, char* buf, size_t nBytes, uint type);
>> 918:   static ssize_t raw_send(int fd, char* buf, size_t nBytes, uint type);
> 
> This set of changes is wrong.  These aren't MEMFLAGS flags.
> (I hope there aren't any more like this.  This sort of thing would be easy to miss in a change this
> large.  If I were making this change I'd have broken it up into several smaller pieces.)

Looks like a find/replace issue. Reverted the change.

> src/hotspot/share/utilities/concurrentHashTable.hpp line 43:
> 
>> 41: class Mutex;
>> 42: 
>> 43: template <typename CONFIG, MemTag F>
> 
> Parameter name should be updated throughout ConcurrentHashTable. Suggest mem_tag.

We can change this later in a followup.

> src/hotspot/share/utilities/growableArray.hpp line 803:
> 
>> 801: 
>> 802: // Leaner GrowableArray for CHeap backed data arrays, with compile-time decided MemTag.
>> 803: template <typename E, MemTag F>
> 
> Another parameter needing update, but shouldn't (can't?) be called `mem_tag` because of the
> function parameter name for allocate().

We can change this later in a followup.

> src/hotspot/share/utilities/linkedlist.hpp line 368:
> 
>> 366: template <class E, int (*FUNC)(const E&, const E&),
>> 367:   AnyObj::allocation_type T = AnyObj::C_HEAP,
>> 368:   MemTag F = mtNMT, AllocFailType alloc_failmode = AllocFailStrategy::RETURN_NULL>
> 
> Another parameter name needing update.

We can change this later in a followup.

> src/hotspot/share/utilities/objectBitSet.hpp line 42:
> 
>> 40:  * during the lifetime of the ObjectBitSet. The underlying memory is allocated from C-Heap.
>> 41:  */
>> 42: template<MemTag F>
> 
> More parameter names needing update.

I followed the local pattern, but we can change this later in a followup.

> src/hotspot/share/utilities/resizeableResourceHash.hpp line 33:
> 
>> 31:     typename K, typename V,
>> 32:     AnyObj::allocation_type ALLOC_TYPE,
>> 33:     MemTag MEM_TYPE>
> 
> I think s/MEM_TYPE/mem_type/, but other non-type template parameters here are also
> all-uppercase, so I guess better to leave it for now and look at it later.

Yes, I followed the local pattern.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20872#discussion_r1750606609
PR Review Comment: https://git.openjdk.org/jdk/pull/20872#discussion_r1750603957
PR Review Comment: https://git.openjdk.org/jdk/pull/20872#discussion_r1750603545
PR Review Comment: https://git.openjdk.org/jdk/pull/20872#discussion_r1750603358
PR Review Comment: https://git.openjdk.org/jdk/pull/20872#discussion_r1750602937
PR Review Comment: https://git.openjdk.org/jdk/pull/20872#discussion_r1750601971