RFR: 8337217: Port VirtualMemoryTracker to use VMATree [v32]

Afshin Zafari azafari at openjdk.org
Fri Feb 28 13:41:15 UTC 2025


On Fri, 28 Feb 2025 10:14:18 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> src/hotspot/share/nmt/vmatree.hpp line 73:
>> 
>>> 71:     assert(type < StateType::COUNT, "must be");
>>> 72:     return statetype_strings[static_cast<uint8_t>(type)];
>>> 73:   }
>> 
>> I don't like that we are hardcoding the size of this array and have COUNT be StateType. Can we do something like this?:
>> 
>> 
>>   enum class StateType : uint8_t { Reserved = 1, Committed = 3, Released = 0 };
>> 
>> private:
>>   static constexpr const char* const statetype_strings[] = {"released", "reserved", "only-committed", "committed"};
>>   static constexpr int STATETYPE_COUNT = static_cast<int>(sizeof(statetype_strings)/sizeof(char*));
>> 
>> public:
>>   NONCOPYABLE(VMATree);
>> 
>>   static const char* statetype_to_string(StateType type) {
>>     assert(static_cast<uint8_t>(type) < STATETYPE_COUNT, "must be");
>>     return statetype_strings[static_cast<uint8_t>(type)];
>>   }
>
> This is a fairly standard pattern in Hotspot, so personally I'm fine with keeping it as-is.
> 
> Here's an incomplete list of pre-existing usages:
> 
> ```c++
> // UL tag list
>   enum type {
>     __NO_TAG,
> #define LOG_TAG(name) _##name,
>     LOG_TAG_LIST
> #undef LOG_TAG
>     Count
>   };
> 
> 
> // enum for figuring positions and size of Symbol::_vm_symbols[]
> enum class vmSymbolID : int {
>   // [FIRST_SID ... LAST_SID] is the iteration range for the *valid* symbols.
>   // NO_SID is used to indicate an invalid symbol. Some implementation code
>   // *may* read _vm_symbols[NO_SID], so it must be a valid array index.
>   NO_SID = 0,                // exclusive lower limit
> 
>   #define VM_SYMBOL_ENUM(name, string) VM_SYMBOL_ENUM_NAME_(name),
>   VM_SYMBOLS_DO(VM_SYMBOL_ENUM, VM_ALIAS_IGNORE)
>   #undef VM_SYMBOL_ENUM
> 
>   SID_LIMIT,                 // exclusive upper limit
> 
>   #define VM_ALIAS_ENUM(name, def) VM_SYMBOL_ENUM_NAME_(name) = VM_SYMBOL_ENUM_NAME_(def),
>   VM_SYMBOLS_DO(VM_SYMBOL_IGNORE, VM_ALIAS_ENUM)
>   #undef VM_ALIAS_ENUM
> 
>   FIRST_SID = NO_SID + 1,    // inclusive lower limit
>   LAST_SID = SID_LIMIT - 1,  // inclusive upper limit
> };
> 
> enum class InjectedFieldID : int {
>   ALL_INJECTED_FIELDS(DECLARE_INJECTED_FIELD_ENUM)
>   MAX_enum
> };
> 
> enum class vmClassID : int {
>   #define DECLARE_VM_CLASS(name, symbol) _VM_CLASS_ENUM(name), _VM_CLASS_ENUM(symbol) = _VM_CLASS_ENUM(name),
>   VM_CLASSES_DO(DECLARE_VM_CLASS)
>   #undef DECLARE_VM_CLASS
> 
>   LIMIT,             // exclusive upper limit
>   FIRST = 0,         // inclusive upper limit
>   LAST = LIMIT - 1   // inclusive upper limit
> };
> 
> enum class CodeBlobType {
>   MethodNonProfiled   = 0,    // Execution level 1 and 4 (non-profiled) nmethods (including native nmethods)
>   MethodProfiled      = 1,    // Execution level 2 and 3 (profiled) nmethods
>   NonNMethod          = 2,    // Non-nmethods like Buffers, Adapters and Runtime Stubs
>   All                 = 3,    // All types (No code cache segmentation)
>   NumTypes            = 4     // Number of CodeBlobTypes
> };

There is coding style that the last enum member counts the number of them, like what we have `mt_number_of_tags` in `MemTag` enum. 
So, I renamed the last member to `st_number_of_states`. Would it be OK? Or preferred to use constant separately.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1975423884


More information about the hotspot-dev mailing list