RFR: 8337563: NMT: rename MEMFLAGS to MemFlag
Gerard Ziemski
gziemski at openjdk.org
Thu Aug 29 23:46:19 UTC 2024
On Thu, 29 Aug 2024 16:17:09 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:
>> Please review this cleanup, where we rename `MEMFLAGS` to `MemType`.
>>
>> `MEMFLAGS` implies that we can use more than one at the same time, but those are exclusive values, so `MemType` is much more suitable name.
>>
>> There is a bunch of other related cleanup that we can do, but I will leave for follow up issues such as [NMT: rename NMTUtil::flag to NMTUtil::type](https://bugs.openjdk.org/browse/JDK-8337836)
>
> I much prefer to see MemType, but I'm warming up to NMTCategory.
>
> - MemType: Succinct - matches part of the code (E.g. the mt in mtGC)
> - MemTypeFlag: Too many words for my preference.
> - NMTCat: Meuw. :)
> - NMTCategory: Parts of the code call these categories, so I'm not entirely against this.
> - NMTGroup: "Group" is a new name for this that currently isn't reflected at all in the code.
> - NMT_MemType: I think we should try get rid of names using this style.
> - NMT::MemType: The `::` makes all function declarations noisier for very little benefit, IMO.
> I continue to mostly agree with @stefank.
>
> I think this name shouldn't be considered in isolation. There are already a bunch of "NMT_" prefixed names. That's the common idiom for things like this (often (maybe even usually?) without the "_"). Why are we proposing to adopt a new style. (For just this? That would be weird. Or more broadly? That certainly needs more discussion.)
It is precisely because we already are using `NMT_TrackingStackDepth` and `NMT_TrackingLevel` (and the fact that MemType by itself was too general) that I suggested we adopt `NMT::` Yes, it would be all static class.
Why not all lower letter `nmt::`? Again, because we already use `NMT_` elsewhere.
My plan was later to switch from `NMT_` to `NMT::` for all of them. We can do `nmt::` too.
So how about `MemTag`, `nmt::MemTag` or `NMT::MemTag`?
-------------
PR Comment: https://git.openjdk.org/jdk/pull/20497#issuecomment-2319470647
More information about the shenandoah-dev
mailing list