You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2022/05/05 20:43:25 UTC
[trafficserver] branch 9.2.x updated: remove pthread_*specific and replace with thread_local (#8805)
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/9.2.x by this push:
new 6abfeab1f remove pthread_*specific and replace with thread_local (#8805)
6abfeab1f is described below
commit 6abfeab1f32790839c67210fc2a1dc46532aefb6
Author: Chris McFarlen <ch...@mcfarlen.us>
AuthorDate: Wed May 4 10:44:04 2022 -0500
remove pthread_*specific and replace with thread_local (#8805)
* remove pthread_*specific and replace with thread_local
* Dont assume thread calling destructor is the thread being destructed.
Co-authored-by: Chris McFarlen <cm...@apple.com>
(cherry picked from commit 60540e23fad547fdf9f6bdc23c8461b876e4d115)
---
include/tscore/ink_thread.h | 24 +-----------------------
iocore/eventsystem/I_EThread.h | 1 +
iocore/eventsystem/I_Thread.h | 2 +-
iocore/eventsystem/P_Thread.h | 4 ++--
iocore/eventsystem/P_UnixEThread.h | 5 +----
iocore/eventsystem/Thread.cc | 18 +++---------------
iocore/eventsystem/UnixEThread.cc | 29 +++++++++--------------------
src/tscore/Regex.cc | 22 +++++++++-------------
8 files changed, 27 insertions(+), 78 deletions(-)
diff --git a/include/tscore/ink_thread.h b/include/tscore/ink_thread.h
index 7a08f6205..84747d2ef 100644
--- a/include/tscore/ink_thread.h
+++ b/include/tscore/ink_thread.h
@@ -106,29 +106,7 @@ typedef timestruc_t ink_timestruc;
//////////////////////////////////////////////////////////////////////////////
#if defined(POSIX_THREAD)
-static inline void
-ink_thread_key_create(ink_thread_key *key, void (*destructor)(void *value))
-{
- ink_assert(!pthread_key_create(key, destructor));
-}
-
-static inline void
-ink_thread_setspecific(ink_thread_key key, void *value)
-{
- ink_assert(!pthread_setspecific(key, value));
-}
-
-static inline void *
-ink_thread_getspecific(ink_thread_key key)
-{
- return pthread_getspecific(key);
-}
-
-static inline void
-ink_thread_key_delete(ink_thread_key key)
-{
- ink_assert(!pthread_key_delete(key));
-}
+// NOTE(cmcfarlen): removed posix thread local key functions, use thread_local
static inline void
ink_thread_create(ink_thread *tid, void *(*f)(void *), void *a, int detached, size_t stacksize, void *stack)
diff --git a/iocore/eventsystem/I_EThread.h b/iocore/eventsystem/I_EThread.h
index 8da21e65c..16ef7571f 100644
--- a/iocore/eventsystem/I_EThread.h
+++ b/iocore/eventsystem/I_EThread.h
@@ -85,6 +85,7 @@ enum ThreadType {
class EThread : public Thread
{
public:
+ static thread_local EThread *this_ethread_ptr;
/** Handler for tail of event loop.
The event loop should not spin. To avoid that a tail handler is called to block for a limited time.
diff --git a/iocore/eventsystem/I_Thread.h b/iocore/eventsystem/I_Thread.h
index dba989769..844cfaeb0 100644
--- a/iocore/eventsystem/I_Thread.h
+++ b/iocore/eventsystem/I_Thread.h
@@ -112,7 +112,7 @@ public:
virtual void set_specific() = 0;
- static ink_thread_key thread_data_key;
+ static thread_local Thread *this_thread_ptr;
// For THREAD_ALLOC
ProxyAllocator eventAllocator;
diff --git a/iocore/eventsystem/P_Thread.h b/iocore/eventsystem/P_Thread.h
index f3030045b..c2bb070a2 100644
--- a/iocore/eventsystem/P_Thread.h
+++ b/iocore/eventsystem/P_Thread.h
@@ -39,11 +39,11 @@
TS_INLINE void
Thread::set_specific()
{
- ink_thread_setspecific(Thread::thread_data_key, this);
+ this_thread_ptr = this;
}
TS_INLINE Thread *
this_thread()
{
- return static_cast<Thread *>(ink_thread_getspecific(Thread::thread_data_key));
+ return Thread::this_thread_ptr;
}
diff --git a/iocore/eventsystem/P_UnixEThread.h b/iocore/eventsystem/P_UnixEThread.h
index 50d1885bc..21ca05240 100644
--- a/iocore/eventsystem/P_UnixEThread.h
+++ b/iocore/eventsystem/P_UnixEThread.h
@@ -35,7 +35,6 @@
#include <execinfo.h>
const ink_hrtime DELAY_FOR_RETRY = HRTIME_MSECONDS(10);
-extern ink_thread_key ethread_key;
TS_INLINE Event *
EThread::schedule_imm(Continuation *cont, int callback_event, void *cookie)
@@ -228,9 +227,7 @@ EThread::schedule_spawn(Continuation *c, int ev, void *cookie)
TS_INLINE EThread *
this_ethread()
{
- // The `dynamic_cast` has a significant performance impact (~6%).
- // Reported by masaori and create PR #6281 to fix it.
- return static_cast<EThread *>(ink_thread_getspecific(ethread_key));
+ return EThread::this_ethread_ptr;
}
TS_INLINE EThread *
diff --git a/iocore/eventsystem/Thread.cc b/iocore/eventsystem/Thread.cc
index 187a20a3e..38c6c3409 100644
--- a/iocore/eventsystem/Thread.cc
+++ b/iocore/eventsystem/Thread.cc
@@ -35,16 +35,7 @@
///////////////////////////////////////////////
ink_hrtime Thread::cur_time = ink_get_hrtime_internal();
-ink_thread_key Thread::thread_data_key;
-
-namespace
-{
-static bool initialized ATS_UNUSED = ([]() -> bool {
- // File scope initialization goes here.
- ink_thread_key_create(&Thread::thread_data_key, nullptr);
- return true;
-})();
-}
+thread_local Thread *Thread::this_thread_ptr;
Thread::Thread()
{
@@ -56,11 +47,8 @@ Thread::Thread()
Thread::~Thread()
{
ink_release_assert(mutex->thread_holding == static_cast<EThread *>(this));
-
- if (ink_thread_getspecific(Thread::thread_data_key) == this) {
- // Clear pointer to this object stored in thread-specific data by set_specific.
- //
- ink_thread_setspecific(Thread::thread_data_key, nullptr);
+ if (this_thread_ptr == this) {
+ this_thread_ptr = nullptr;
}
mutex->nthread_holding -= THREAD_MUTEX_THREAD_HOLDING;
diff --git a/iocore/eventsystem/UnixEThread.cc b/iocore/eventsystem/UnixEThread.cc
index e1dc8a1f4..e12a775f3 100644
--- a/iocore/eventsystem/UnixEThread.cc
+++ b/iocore/eventsystem/UnixEThread.cc
@@ -52,31 +52,22 @@ int const EThread::SAMPLE_COUNT[N_EVENT_TIMESCALES] = {10, 100, 1000};
int thread_max_heartbeat_mseconds = THREAD_MAX_HEARTBEAT_MSECONDS;
// To define a class inherits from Thread:
-// 1) Define an independent ink_thread_key
-// 2) Override Thread::set_specific()
-// 3) Define this_Xthread() which get thread specific data by the independent key
+// 1) Define an independent thread_local static member
+// 2) Override Thread::set_specific() and assign that member and call Thread::set_specific()
+// 3) Define this_Xthread() which get thread specific data
// 4) Clear thread specific data at destructor function.
//
// The below comments are copied from I_Thread.h
//
// Additionally, the EThread class (derived from Thread) maintains its
-// own independent key. All (and only) the threads created in the Event
-// Subsystem are registered with this key.
-ink_thread_key ethread_key;
-
-namespace
-{
-static bool ethread_initialized ATS_UNUSED = ([]() -> bool {
- // File scope initialization goes here.
- ink_thread_key_create(ðread_key, nullptr);
- return true;
-})();
-}
+// own independent data. All (and only) the threads created in the Event
+// Subsystem have this data.
+thread_local EThread *EThread::this_ethread_ptr;
void
EThread::set_specific()
{
- ink_thread_setspecific(ethread_key, this);
+ this_ethread_ptr = this;
Thread::set_specific();
}
@@ -124,10 +115,8 @@ EThread::EThread(ThreadType att, Event *e) : tt(att), start_event(e)
EThread::~EThread()
{
ink_release_assert(mutex->thread_holding == static_cast<EThread *>(this));
-
- if (ink_thread_getspecific(ethread_key) == this) {
- // Clear pointer to this object stored in thread-specific data by set_specific.
- ink_thread_setspecific(ethread_key, nullptr);
+ if (this_ethread_ptr == this) {
+ this_ethread_ptr = nullptr;
}
}
diff --git a/src/tscore/Regex.cc b/src/tscore/Regex.cc
index d9947dc34..e8195f0de 100644
--- a/src/tscore/Regex.cc
+++ b/src/tscore/Regex.cc
@@ -29,24 +29,20 @@
#include "tscore/Regex.h"
#ifdef PCRE_CONFIG_JIT
-struct RegexThreadKey {
- RegexThreadKey() { ink_thread_key_create(&this->key, reinterpret_cast<void (*)(void *)>(&pcre_jit_stack_free)); }
- ink_thread_key key;
-};
-
-static RegexThreadKey k;
-
static pcre_jit_stack *
get_jit_stack(void *data ATS_UNUSED)
{
- pcre_jit_stack *jit_stack;
+ thread_local struct JitStack {
+ JitStack()
+ {
+ jit_stack = pcre_jit_stack_alloc(ats_pagesize(), 1024 * 1024); // 1 page min and 1MB max
+ }
+ ~JitStack() { pcre_jit_stack_free(jit_stack); }
- if ((jit_stack = static_cast<pcre_jit_stack *>(ink_thread_getspecific(k.key))) == nullptr) {
- jit_stack = pcre_jit_stack_alloc(ats_pagesize(), 1024 * 1024); // 1 page min and 1MB max
- ink_thread_setspecific(k.key, (void *)jit_stack);
- }
+ pcre_jit_stack *jit_stack = nullptr;
+ } stack;
- return jit_stack;
+ return stack.jit_stack;
}
#endif