On Sat, 22 Oct 2022 03:05:07 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:

> The implementation of removeThread() is currently:
> 
> 
> static void
> removeThread(JNIEnv *env, ThreadList *list, jthread thread)
> {
>     ThreadNode *node;
> 
>     node = findThread(list, thread);
>     if (node != NULL) {
>         removeNode(list, node);
>         clearThread(env, node);
>     }
> }
> 
> However, currently all callers already have the ThreadNode*, so they end up 
> calling like the following:
> 
> `            removeThread(env, list, node->thread);`
> 
> So we go from a ThreadNode* to a jthread, only to do a findThread() to get 
> the ThreadNode* again. Also, the list is stored in the ThreadNode. 
> removeThread() can instead be implemented as:
> 
> 
> static void
> removeThread(JNIEnv *env, ThreadNode *node)
> {
>   JDI_ASSERT(node != NULL);
>   removeNode(node->list, node);
>   clearThread(env, node);
> }
> 
> 
> This is faster, although not by as much as you might think. TLS is used to 
> map a jthread to a ThreadNode*, so the findThread() call is actually already 
> pretty fast. The exception is when dealing with the otherThreads list. These 
> threads have not yet been started, so TLS cannot be used, but it is rare for 
> threads to appear on this list. Still, this is a good cleanup to do, and 
> should be a bit faster.
> 
> This cleanup was initially implemented as part of the work for 
> [JDK-8295376](https://bugs.openjdk.org/browse/JDK-8295376), but is being 
> split out.

Marked as reviewed by amenkov (Reviewer).

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

PR: https://git.openjdk.org/jdk/pull/10828

Reply via email to