On Sat, 31 Aug 2024 00:04:57 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:

>> Only allocate the cmdQueueLock once rather than allocate each time there is 
>> a new connection (and destroy when the connection is terminated).
>> 
>> Currently each time there is a new debug agent connection established, the 
>> cmdQueueLock is allocated. It is then destroyed when the connection is 
>> dropped, only to be recreated on the next connection. I looked closely at 
>> the use of this lock, and what happens when the connection is dropped, and I 
>> can't see any reason to create and then dispose of cmdQueueLock for each 
>> connection. Of the 18 debug agent raw monitors, this is the only one that 
>> gets destroyed and recreated. The other 17 stick around for the next 
>> connection. I'd like to get rid of this reallocation in order to support 
>> static initialization of all the debug agent raw monitors on startup. This 
>> will be done as part of the ranked monitor support 
>> ([JDK-8328866](https://bugs.openjdk.org/browse/JDK-8328866)).
>> 
>> As part of the testing for this change, I put an assert in place when we try 
>> to create cmdQueueLock for the 2nd time. I ran all our debugger tests, and 
>> unfortunately none of them failed. This means we don't currently test 
>> attaching to the debug agent more than once in any of our tests, so I did 
>> two things to better test out this change. The first was to write a test 
>> that attaches numerous times in sequence, and the 2nd was to manually attach 
>> and reattach using jdb. At the same time I stepped through the debug agent 
>> code in gdb just to help better understand the setup and use of 
>> cmdQueueLock. I didn't see anything that gave me concern about making this 
>> change.
>> 
>> Tested by running all jdi, jdwp, and jdb tests on all supported platforms.
>
> Chris Plummer has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Get rid of extra space char

Marked as reviewed by amenkov (Reviewer).

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

PR Review: https://git.openjdk.org/jdk/pull/20755#pullrequestreview-2273684804

Reply via email to