Description
The JDWP debug agent uses JVMTI thread local storage (TLS) to quickly map from a jthread to the debug agent's ThreadNode struct for the thread. However, you can't use TLS on unstarted threads. SetThreadLocalStorage will return JVMTI_ERROR_THREAD_NOT_ALIVE. Because of this the agent simply allows SetThreadLocalStorage to fail with this error, and will fix it up later when the thread is known to have started.
This is all fine, except it impacts the performance of findThread. If GetThreadLocalStorage of the jthread fails, it searches for the thread in the lists that the debug agent maintains. One list is called runningThreads and the other is called otherThreads, which is where all unstarted threads can be found. This also seems fine at first, except that whenever there is a new thread introduced, the first thing done for it is a call to findThreads. GetThreadLocalStorage will return NULL for it because the thread is new to the debug agent. Next the two lists are searched, and that will fail also. At that point the findThread will add the thread to one of the lists by calling insertThread (which BTW does another findThread call to make sure it's not already added). The end result of all this is that any new thread will first need to be searched for on the lists. When the list is long, this becomes a performance issue. For the loom project, it's possible there will be a very large number of virtual threads, so it becomes even more of a performance problem. Every new vthread results in searching a long list for it, even though it is known that the search will fail.
The key cause of this issue is the debug agent acts like the TLS for any thread for which it has already created a ThreadNode can be NULL. That's not true. It can only be NULL for threads in the otherThreads list.
The suggested fix is:
1. Don't call SetThreadLocalStorage for unstarted threads (and don't accept SetThreadLocalStorage ever failing except when used to clear the TLS data). Unstarted threads already go on the otherThreads list, so we can then assume that any thread on runningThreads has its TLS set.
2. When a thread is moved from otherThreads to runningThreads, set its TLS.
3. When findThread fails a TLS lookup, only look in otherThreads for the thread. Don't waste time looking in runningThreads, because it should always fail.
This is all fine, except it impacts the performance of findThread. If GetThreadLocalStorage of the jthread fails, it searches for the thread in the lists that the debug agent maintains. One list is called runningThreads and the other is called otherThreads, which is where all unstarted threads can be found. This also seems fine at first, except that whenever there is a new thread introduced, the first thing done for it is a call to findThreads. GetThreadLocalStorage will return NULL for it because the thread is new to the debug agent. Next the two lists are searched, and that will fail also. At that point the findThread will add the thread to one of the lists by calling insertThread (which BTW does another findThread call to make sure it's not already added). The end result of all this is that any new thread will first need to be searched for on the lists. When the list is long, this becomes a performance issue. For the loom project, it's possible there will be a very large number of virtual threads, so it becomes even more of a performance problem. Every new vthread results in searching a long list for it, even though it is known that the search will fail.
The key cause of this issue is the debug agent acts like the TLS for any thread for which it has already created a ThreadNode can be NULL. That's not true. It can only be NULL for threads in the otherThreads list.
The suggested fix is:
1. Don't call SetThreadLocalStorage for unstarted threads (and don't accept SetThreadLocalStorage ever failing except when used to clear the TLS data). Unstarted threads already go on the otherThreads list, so we can then assume that any thread on runningThreads has its TLS set.
2. When a thread is moved from otherThreads to runningThreads, set its TLS.
3. When findThread fails a TLS lookup, only look in otherThreads for the thread. Don't waste time looking in runningThreads, because it should always fail.
Attachments
Issue Links
- relates to
-
JDK-8269268 JDWP: Properly fix thread lookup assert in findThread()
-
- Resolved
-
-
-
- Resolved
-
