On Thu, 15 Aug 2024 06:07:49 GMT, Stefan Karlsson <stef...@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
