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