On Wed, 8 May 2024 18:27:15 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> src/jdk.jdwp.agent/share/native/libjdwp/debugLoop.c line 66:
>>
>>> 64: debugLoop_initialize(void)
>>> 65: {
>>> 66: vmDeathLock = debugMonitorCreate(vmDeathLockForDebugLoop_Rank,
>>> "JDWP VM_DEATH Lock");
>>
>> Nit: Just a suggestion to consider (there was a similar suggestion from by
>> Alex)...
>> Can all the `debugMonitor's` created/initialized by one function
>> `debugMonitor_initialize()` called by util_initialize()? Then each monitor
>> consumer can cache needed `debugMonitor's` like this:
>>
>> vmDeathLock = getDebugMonitor(vmDeathLockForDebugLoop_Rank);
>
> I think you are suggesting that all monitors be created up front during
> util_intialize(). The debug agent in the past has always created them lazily
> (and sometimes some of them end up never being created because they are not
> needed). I don't really see a big advantage in creating them all up front.
>
> Alex's suggestion was a very different one, and was just a suggestion to
> initialize all the DebugRawMonitor ranks up front rather than when the
> jMonitorID is created, and the reason for the suggestion was to avoid having
> to grab the dbgRawMonitor when setting the rank, but I wasn't convinced that
> it actually allowed you to not grab the dbgRawMonitor.
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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19044#discussion_r1594571559
