RFR: 8337563: NMT: rename MEMFLAGS to MemFlag

Kim Barrett kbarrett at openjdk.org
Thu Aug 15 07:20:58 UTC 2024


On Thu, 15 Aug 2024 06:07:49 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

> > I agree with @tstuefe here. MemFlag and MemType sound far too general when this is NMT specific.
> 
> Yes, it is not very specific, but it also not hard to learn and then know what this type is all about.
> 
> > My preference to keep the "flags" part of the type was to avoid needing to rename many parameters. The usage of MEMFLAGS flags is quite extensive. I would not want to see a partial approach here where we end up with a non-flag type name but a flag variable name.
> 
> I think we should rename all the 'flags' variables in the same change.
> 
> > NMTTypeFlag would I hope satisfy Thomas's requirement and avoid the need to do variable renames.
> 
>     * To me, that's really not an appealing name for a type that is going to be used by all parts of the HotSpot code base. I much more prefer a shorter name that is easy on the eyes, then a longer and more specific name that is an eyesore.
> 
>     * And even as a longer name, it doesn't tell what it is going to be used for. What is a Native Memory Tracker Type Flag?
> 
>     * I don't want us to select a bad name so that we don't have to change the variable names.
> 
>     * Whatever we choose we also need to consider the mt prefix of things like mtGC, mtClass, etc.
> 
> 
> With all that said, I hope it is clear that we various reviewers have different opinions around this and that we don't integrate this before we have some kind of consensus about the way forward with this.

I strongly agree with everything @stefank said above.  I also think some of the suggestions that have been offered
are not improvements from the status quo, or not enough to be worth the code churn.

The only thing I have against MemType is that "type" is pretty overloaded.  I spent some time with a thesaurus, but
the only alternative I came up with that I liked was MemGroup, but it fails the "mt" prefix consideration.

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

PR Comment: https://git.openjdk.org/jdk/pull/20497#issuecomment-2290801845


More information about the serviceability-dev mailing list