> This PR disables VIRTUAL_THREAD_START/END events when no debugger is > attached. Some customers like to launch (in production mode) the JVM with the > debug agent enabled, but with no debugger attached. This usually has a very > minimal performance impact as long as the debugger remains unattached. > Launching the JVM in this manner allows a debugger to later be attached if > there are issues. > > In the past the debug agent has always kept THREAD_START/END events enabled, > even when no debugger is attached. It does so to keep track of all existing > threads, and this generally has minimal performance impact. However, with > virtual threads there can be a very large number of virtual threads > constantly being created and destroyed. With SkyNet, this results in as much > as a 250x performance regression when the debug agent is enabled. A large > part of this is due to all the VIRTUAL_THREAD_START/END events the debug > agent has to deal with. When these events are disabled, the performance > slowdown is more like 10x. This remaining 10x comes from JVMTI and is not > addressed in this CR. > > The fix for this performance issue is to disable VIRTUAL_THREAD_START/END > events when the debugger is not attached. This also requires forgetting about > all known virtual threads when the debugger detaches since there will be no > VIRTUAL_THREAD_END event to notify the debug agent when the virtual thread > has died. > > I decided it was appropriate to not introduce this behavior change if the > debug agent is launched with `includevirtualthreads=y` (the default is 'n'). > `includevirtualthreads=y` is meant to help legacy debuggers, and indicates > that the debug agent should try to track (remember) all vthreads that are > running, and return them when `VirtualMachine.GetAllThreads` is called. > `includevirtualthreads=y` sets `gdata->includeVThreads` true, so initially I > had these changes conditional on `gdata->includeVThreads`. However, most of > our testing is done with `includevirtualthreads=y` (an unfortunate necessity > because of how most of the test are written). This made it hard to test these > changes. So I introduced `gdata->rememberVThreadsWhenDisconnected`, and made > the changes conditional on it instead. It defaults to be the same as > `gdata->includeVThreads`. However, I did some testing with it manually set it > to always be false to get better testing coverage. This could be made a debug > agent flag set table on the command line, but I opted not to since it's not really of interest to the typical debug agent user. I suppose it could be an undocumented flag just to use in testing. I can make this change if others agree. > > A couple notes on code flow that might help with understanding the changes: > - When the connection is initiated, `debugLoop_run()` is called, and it calls > `eventHandler_onConnect()` to enable the START/END callbacks. > - When the connection is dropped, `debugLoop_run()` calls > `debugInit_reset()`, which calls `eventHandler_reset()` to disable the > START/END callbacks. A bit later `debugInit_reset()` calls > `threadControl_reset()` to free up the vthread ThreadNodes.
Chris Plummer has updated the pull request incrementally with one additional commit since the last revision: Moving setting of waiting_for_active_callbacks inside of locking code ------------- Changes: - all: https://git.openjdk.org/jdk/pull/10865/files - new: https://git.openjdk.org/jdk/pull/10865/files/f1952e7d..9e4779cc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=10865&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10865&range=00-01 Stats: 4 lines in 1 file changed: 2 ins; 2 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/10865.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10865/head:pull/10865 PR: https://git.openjdk.org/jdk/pull/10865
