On Sat, 22 Oct 2022 03:05:07 GMT, Chris Plummer <[email protected]> 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.
src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 322:
> 320: ThreadNode *next;
> 321:
> 322: JDI_ASSERT(list == node->list);
"list" argument is redundant
I suggest to drop it and replace this assert with
`ThreadList *list == node->list;`
-------------
PR: https://git.openjdk.org/jdk/pull/10828
