[src/trunk]: src/lib/libpthread Improve TSD behavior

[joerg <joerg%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/873694d9cffc
branches:  trunk
changeset: 1009301:873694d9cffc
user:      joerg <joerg%NetBSD.org@localhost>
date:      Sun Apr 19 20:47:03 2020 +0000

description:
Improve TSD behavior

Optimistically check whether the key has been used by this thread
already and avoid locking in that case. This avoids the atomic operation
in the hot path. When the value is set to non-NULL for the first time,
put the entry on the to-be-freed list and keep it their until
destruction or thread exit. Setting the key to NULL and back is common
enough and updating the list is more expensive than the extra check on
the final round.

diffstat:

 lib/libpthread/pthread_tsd.c |  31 ++++++++++++++-----------------
 1 files changed, 14 insertions(+), 17 deletions(-)

diffs (75 lines):

diff -r 8d8de3318b6a -r 873694d9cffc lib/libpthread/pthread_tsd.c
--- a/lib/libpthread/pthread_tsd.c      Sun Apr 19 20:46:04 2020 +0000
+++ b/lib/libpthread/pthread_tsd.c      Sun Apr 19 20:47:03 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: pthread_tsd.c,v 1.21 2020/04/19 20:46:04 joerg Exp $   */
+/*     $NetBSD: pthread_tsd.c,v 1.22 2020/04/19 20:47:03 joerg Exp $   */
 
 /*-
  * Copyright (c) 2001, 2007 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: pthread_tsd.c,v 1.21 2020/04/19 20:46:04 joerg Exp $");
+__RCSID("$NetBSD: pthread_tsd.c,v 1.22 2020/04/19 20:47:03 joerg Exp $");
 
 /* Functions and structures dealing with thread-specific data */
 #include <errno.h>
@@ -173,14 +173,17 @@
  * elements. When an element is used it is inserted into the appropriate
  * key bucket of pthread__tsd_list. This means that ptqe_prev == NULL,
  * means that the element is not threaded, ptqe_prev != NULL it is
- * already part of the list. When we set to a NULL value we delete from the
- * list if it was in the list, and when we set to non-NULL value, we insert
- * in the list if it was not already there.
+ * already part of the list. If a key is set to a non-NULL value for the
+ * first time, it is added to the list.
  *
  * We keep this global array of lists of threads that have called
  * pthread_set_specific with non-null values, for each key so that
  * we don't have to check all threads for non-NULL values in
- * pthread_key_destroy
+ * pthread_key_destroy.
+ *
+ * The assumption here is that a concurrent pthread_key_delete is already
+ * undefined behavior. The mutex is taken only once per thread/key
+ * combination.
  *
  * We could keep an accounting of the number of specific used
  * entries per thread, so that we can update pt_havespecific when we delete
@@ -193,21 +196,15 @@
 
        pthread__assert(key >= 0 && key < pthread_keys_max);
 
-       pthread_mutex_lock(&tsd_mutex);
        pthread__assert(pthread__tsd_destructors[key] != NULL);
        pt = &self->pt_specific[key];
        self->pt_havespecific = 1;
-       if (value) {
-               if (pt->pts_next.ptqe_prev == NULL)
-                       PTQ_INSERT_HEAD(&pthread__tsd_list[key], pt, pts_next);
-       } else {
-               if (pt->pts_next.ptqe_prev != NULL) {
-                       PTQ_REMOVE(&pthread__tsd_list[key], pt, pts_next);
-                       pt->pts_next.ptqe_prev = NULL;
-               }
+       if (value && !pt->pts_next.ptqe_prev) {
+               pthread_mutex_lock(&tsd_mutex);
+               PTQ_INSERT_HEAD(&pthread__tsd_list[key], pt, pts_next);
+               pthread_mutex_unlock(&tsd_mutex);
        }
        pt->pts_value = __UNCONST(value);
-       pthread_mutex_unlock(&tsd_mutex);
 
        return 0;
 }
@@ -373,7 +370,7 @@
                                destructor = NULL;
 
                        pthread_mutex_unlock(&tsd_mutex);
-                       if (destructor != NULL) {
+                       if (destructor != NULL && val != NULL) {
                                done = 0;
                                (*destructor)(val);
                        }