On Tue, 7 Feb 2023 14:40:49 GMT, Justin King <jck...@openjdk.org> wrote:
>> - Rename `MEMFLAGS` to `MemoryType`. `MEMFLAGS` is highly misleading as >> flags typically can be combined. >> - Update `MemoryType` to have enumeration names that follow the style guide, >> no `mt` prefix. >> - Create aliases for old `mtXXX` names. >> - Remove `mt_number_of_types` from the enumeration. >> - Shift implementation of utilities related to `MEMFLAGS` from `NMTUtil` to >> `MemoryTypes`. Handle missing `mt` prefix during parsing. >> - `NMTUtil` references are not updated to avoid increasing patch size. >> - Merge `AllocFailStrategy` and `AllocFailType` into >> `AllocationFailureStrategy`. >> - Move `MemoryType` and `AllocationFailureStrategy` to their own respective >> headers. >> >> This change does not update variable verbiage. That should be something done >> over time. > > Justin King has updated the pull request incrementally with one additional > commit since the last revision: > > Fix refactor mistake > > Signed-off-by: Justin King <jck...@google.com> I don't like MemoryType. MEMFLAGS makes it indicative that it's a flag for some purpose and sticks out to my eyes as a template parameter, which is part of our coding style. mtWhatever is easy to spot in the code where used. I don't agree with the reasoning for this change. Moving the header file out of allocation.hpp seems good though although we still need to include allocation.hpp to get CHeapObj so not sure how much inclusion that saves. I think also having a PR with a bullet list is a good sign that you're doing too much in one change. ------------- PR: https://git.openjdk.org/jdk/pull/12454
