On Wed, 8 May 2024 22:41:45 GMT, Chris Plummer <[email protected]> wrote:
>> I just wanted to say that Alex's suggestion is in a similar direction.
>> I do not see a big advantage to save just on creation of a few monitors. It
>> is not a scalability problem as their number is constant. Creation of the
>> monitors at initialization time is a simplification as we do not care about
>> synchronization at this phase. Also, it is easier to understand because
>> there is no interaction with other threads.
>
> The init code would have to know the name of each monitor since it is no
> longer being passed in. Something like the following might work:
>
> static DebugRawMonitor dbg_monitors[] = {
> { NULL, "JDWP VM_DEATH Lock", vmDeathLockForDebugLoop_Rank, NULL, 0 },
> { NULL, "JDWP Callback Block", callbackBlock_Rank, NULL, 0 },
> { NULL, "JDWP Callback Lock", callbackLock_Rank, NULL, 0 },
> ...
> };
>
> And then the init() function can iterate over the array and allocate the
> monitor for each entry. Note that this array needs to be kept in sync with
> the DebugRawMonitorRank enum (same entries and in the same order). I can
> assert during the initialization that the rank field for each entry is equal
> to its array index and that the array size is NUM_DEBUG_RAW_MONITORS.
Can we do as below? :
static DebugRawMonitor dbg_monitors[]; // initialized with all zeros by default
. . .
static void
createDebugRawMonitor(DebugRawMonitorRank rank, const char* name) {
dbg_monitors[rank] = debugMonitorCreate(rank, name);
}
static void
monitors_initialize() {
createDebugRawMonitor( vmDeathLockForDebugLoop_Rank, "JDWP VM_DEATH Lock");
createDebugRawMonitor( callbackBlock_Rank, "JDWP Callback Block");
createDebugRawMonitor( callbackLock_Rank, "JDWP Callback Lock");
. . .
}
The pair of functions `createDebugRawMonitor()+debugMonitorCreate()` can be
refactored into just one function.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19044#discussion_r1594823032
