On Wed, 8 May 2024 15:44:37 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> Chris Plummer has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Fix jcheck extra whitespace.
>> - Fix comment typo.
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19044#discussion_r1594466821
