From 51346e18cd1d8773b2b480b62e6888a9754e3897 Mon Sep 17 00:00:00 2001 From: Oknet Xu Date: Mon, 23 Dec 2019 17:58:07 +0800 Subject: [PATCH] Follow the comments in I_Thread.h, add an independent ink_thread_key for EThread. (#6288) With this PR, the threads in the system (that have been created using a derived class) are registered with 2 different thread keys. One key from Thread class and another key from the derived class. The EThread or a derived class maintains its own independent key. Therefore, we can obtain thread-specific data through pthread_getspecific (independent key), and directly convert void * to derived class type via static_cast. We can also obtain the base class (Thread) object through pthread_getspecific (Thread::thread_data_key) because any derived class is also registered with Thread::thread_data_key, and directly convert void * to Thread type via static_cast. With this PR, the EThread class maintains 2 thread_keys, one is inherited from Thread class, another is an independent EThread key. All EThread are registered with these 2 keys. If there is an XThread class (derived from Thread), it also maintains 2 thread_keys, one is inherited from Thread class, another is an independent XThread key. All XThread are registered with these 2 keys. If we call pthread_getspecific (Thread::thread_data_key) or pthread_getspecific (EThread key) inside an EThread, we will get the same void * pointer to the EThread object. If we perform the same operation inside an XThread, the pthread_getspecific (Thread::thread_data_key) will return a void * pointer to the XThread object, the pthread_getspecific (EThread key) will return NULL. If we perform the same operation inside an thread which is not a Thread or derived from Thread class, the this_thread() and this_xthread() will return NULL. The this_thread() always returns the result of pthread_getspecific (Thread::thread_data_key). The this_ethread() always returns the result of pthread_getspecific (EThread key). The this_xthread() always returns the result of pthread_getspecific (XThread key). Different keys mean different derived object of Thread class. --- iocore/eventsystem/I_EThread.h | 2 ++ iocore/eventsystem/I_Thread.h | 2 +- iocore/eventsystem/P_UnixEThread.h | 5 +++- iocore/eventsystem/UnixEThread.cc | 39 +++++++++++++++++++++++++++++- proxy/http2/Makefile.am | 6 ++--- 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/iocore/eventsystem/I_EThread.h b/iocore/eventsystem/I_EThread.h index d8ee534b34c..49d74d896a3 100644 --- a/iocore/eventsystem/I_EThread.h +++ b/iocore/eventsystem/I_EThread.h @@ -282,6 +282,8 @@ class EThread : public Thread // Set the tail handler. void set_tail_handler(LoopTailHandler *handler); + void set_specific() override; + /* private */ Event *schedule_local(Event *e); diff --git a/iocore/eventsystem/I_Thread.h b/iocore/eventsystem/I_Thread.h index 2ab995a83ed..f2f1b1c2ccd 100644 --- a/iocore/eventsystem/I_Thread.h +++ b/iocore/eventsystem/I_Thread.h @@ -110,7 +110,7 @@ class Thread */ Ptr mutex; - void set_specific(); + virtual void set_specific(); inkcoreapi static ink_thread_key thread_data_key; diff --git a/iocore/eventsystem/P_UnixEThread.h b/iocore/eventsystem/P_UnixEThread.h index 3d34208ea8c..f95868f4522 100644 --- a/iocore/eventsystem/P_UnixEThread.h +++ b/iocore/eventsystem/P_UnixEThread.h @@ -34,6 +34,7 @@ #include "I_EventProcessor.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) @@ -186,7 +187,9 @@ EThread::schedule_spawn(Continuation *c, int ev, void *cookie) TS_INLINE EThread * this_ethread() { - return static_cast(this_thread()); + // The `dynamic_cast` has a significant performace impact (~6%). + // Reported by masaori and create PR #6281 to fix it. + return static_cast(ink_thread_getspecific(ethread_key)); } TS_INLINE EThread * diff --git a/iocore/eventsystem/UnixEThread.cc b/iocore/eventsystem/UnixEThread.cc index 06c9943af78..dc663ba3f7d 100644 --- a/iocore/eventsystem/UnixEThread.cc +++ b/iocore/eventsystem/UnixEThread.cc @@ -51,6 +51,35 @@ 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 +// 4) Clear thread specific data at destructor funtion. +// +// 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; +})(); +} + +void +EThread::set_specific() +{ + ink_thread_setspecific(ethread_key, this); + Thread::set_specific(); +} + EThread::EThread() { memset(thread_private, 0, PER_THREAD_DATA); @@ -92,7 +121,15 @@ EThread::EThread(ThreadType att, Event *e) : tt(att), start_event(e) // Provide a destructor so that SDK functions which create and destroy // threads won't have to deal with EThread memory deallocation. -EThread::~EThread() {} +EThread::~EThread() +{ + ink_release_assert(mutex->thread_holding == static_cast(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); + } +} bool EThread::is_event_type(EventType et) diff --git a/proxy/http2/Makefile.am b/proxy/http2/Makefile.am index d34de7015bd..dac5c81080e 100644 --- a/proxy/http2/Makefile.am +++ b/proxy/http2/Makefile.am @@ -66,10 +66,10 @@ TESTS = $(check_PROGRAMS) # Be careful if you change the order. Details in GitHub #6666 test_libhttp2_LDADD = \ libhttp2.a \ - $(top_builddir)/iocore/eventsystem/libinkevent.a \ - $(top_builddir)/proxy/hdrs/libhdrs.a \ - $(top_builddir)/src/tscore/libtscore.la \ $(top_builddir)/src/tscpp/util/libtscpputil.la \ + $(top_builddir)/src/tscore/libtscore.la \ + $(top_builddir)/proxy/hdrs/libhdrs.a \ + $(top_builddir)/iocore/eventsystem/libinkevent.a \ $(top_builddir)/lib/records/librecords_p.a \ $(top_builddir)/mgmt/libmgmt_p.la \ $(top_builddir)/proxy/shared/libUglyLogStubs.a \