From 6dedcae852baf4533586bb06f5fcf760ccde6500 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Sun, 7 Apr 2019 13:55:47 +0200 Subject: [core] refactor ThreadLocal backend --- platform/default/src/mbgl/util/thread_local.cpp | 64 +++++++++---------------- platform/qt/src/thread_local.cpp | 44 +++++++---------- src/mbgl/util/thread_local.hpp | 38 ++++++++++----- 3 files changed, 64 insertions(+), 82 deletions(-) diff --git a/platform/default/src/mbgl/util/thread_local.cpp b/platform/default/src/mbgl/util/thread_local.cpp index 068c2880c4..5fb66c3440 100644 --- a/platform/default/src/mbgl/util/thread_local.cpp +++ b/platform/default/src/mbgl/util/thread_local.cpp @@ -1,66 +1,46 @@ #include - -#include #include -#include -#include #include +#include #include namespace mbgl { namespace util { - -template -class ThreadLocal::Impl { -public: - pthread_key_t key; -}; - -template -ThreadLocal::ThreadLocal() : impl(std::make_unique()) { - int ret = pthread_key_create(&impl->key, [](void *) {}); - - if (ret) { - throw std::runtime_error("Failed to init local storage key."); +namespace impl { + +ThreadLocalBase::ThreadLocalBase() { + static_assert(sizeof(storage) >= sizeof(pthread_key_t), "storage is too small"); + static_assert(alignof(decltype(storage)) % alignof(pthread_key_t) == 0, "storage is incorrectly aligned"); + if (pthread_key_create(&reinterpret_cast(storage), nullptr) != 0) { + Log::Error(Event::General, "Failed to initialize thread-specific storage key"); + abort(); } } -template -ThreadLocal::~ThreadLocal() { - // ThreadLocal will not take ownership - // of the pointer it is managing. The pointer - // needs to be explicitly cleared before we - // destroy this object. +ThreadLocalBase::~ThreadLocalBase() { + // ThreadLocal will not take ownership of the pointer it is managing. The pointer + // needs to be explicitly cleared before we destroy this object. assert(!get()); - if (pthread_key_delete(impl->key)) { - Log::Error(Event::General, "Failed to delete local storage key."); - assert(false); + if (pthread_key_delete(reinterpret_cast(storage)) != 0) { + Log::Error(Event::General, "Failed to delete thread-specific storage key"); + abort(); } } -template -T* ThreadLocal::get() { - auto* ret = reinterpret_cast(pthread_getspecific(impl->key)); - if (!ret) { - return nullptr; - } - - return ret; +void* ThreadLocalBase::get() { + return pthread_getspecific(reinterpret_cast(storage)); } -template -void ThreadLocal::set(T* ptr) { - if (pthread_setspecific(impl->key, ptr)) { - throw std::runtime_error("Failed to set local storage."); +void ThreadLocalBase::set(void* ptr) { + if (pthread_setspecific(reinterpret_cast(storage), ptr) != 0) { + Log::Error(Event::General, "Failed to set thread-specific storage"); + abort(); } } -template class ThreadLocal; -template class ThreadLocal; -template class ThreadLocal; // For unit tests - +} // namespace impl } // namespace util } // namespace mbgl diff --git a/platform/qt/src/thread_local.cpp b/platform/qt/src/thread_local.cpp index 4191195a86..a4277f0410 100644 --- a/platform/qt/src/thread_local.cpp +++ b/platform/qt/src/thread_local.cpp @@ -1,8 +1,5 @@ #include -#include -#include - #include #include @@ -10,40 +7,33 @@ namespace mbgl { namespace util { +namespace impl { -template -class ThreadLocal::Impl { -public: - QThreadStorage> local; -}; +// QThreadStorage tries to be smart and take ownership of the data, which we don't want. So we're +// wrapping it in another type which doesn't own the pointer it contains. +using StorageType = QThreadStorage>; -template -ThreadLocal::ThreadLocal() : impl(std::make_unique()) { - set(nullptr); +ThreadLocalBase::ThreadLocalBase() { + static_assert(sizeof(storage) >= sizeof(StorageType), "storage is too small"); + static_assert(alignof(decltype(storage)) % alignof(StorageType) == 0, "storage is incorrectly aligned"); + new (&reinterpret_cast(storage)) QThreadStorage(); } -template -ThreadLocal::~ThreadLocal() { - // ThreadLocal will not take ownership - // of the pointer it is managing. The pointer - // needs to be explicitly cleared before we - // destroy this object. +ThreadLocalBase::~ThreadLocalBase() { + // ThreadLocal will not take ownership of the pointer it is managing. The pointer + // needs to be explicitly cleared before we destroy this object. assert(!get()); + reinterpret_cast(storage).~QThreadStorage(); } -template -T* ThreadLocal::get() { - return impl->local.localData()[0]; +void* ThreadLocalBase::get() { + return reinterpret_cast(storage).localData()[0]; } -template -void ThreadLocal::set(T* ptr) { - impl->local.localData()[0] = ptr; +void ThreadLocalBase::set(void* ptr) { + reinterpret_cast(storage).setLocalData({{ ptr }}); } -template class ThreadLocal; -template class ThreadLocal; -template class ThreadLocal; // For unit tests - +} // namespace impl } // namespace util } // namespace mbgl diff --git a/src/mbgl/util/thread_local.hpp b/src/mbgl/util/thread_local.hpp index b0e26356b4..cf5579a2ba 100644 --- a/src/mbgl/util/thread_local.hpp +++ b/src/mbgl/util/thread_local.hpp @@ -1,29 +1,41 @@ #pragma once -#include - -#include +#include namespace mbgl { namespace util { +namespace impl { + +class ThreadLocalBase { +protected: + ThreadLocalBase(); + ~ThreadLocalBase(); + + void* get(); + void set(void*); + +private: + std::aligned_storage_t storage; +}; + +} // namespace impl template -class ThreadLocal : public noncopyable { +class ThreadLocal : public impl::ThreadLocalBase { public: + ThreadLocal() = default; + ThreadLocal(T* val) { - ThreadLocal(); set(val); } - ThreadLocal(); - ~ThreadLocal(); - - T* get(); - void set(T* ptr); + T* get() { + return reinterpret_cast(impl::ThreadLocalBase::get()); + } -private: - class Impl; - std::unique_ptr impl; + void set(T* ptr) { + impl::ThreadLocalBase::set(ptr); + } }; } // namespace util -- cgit v1.2.1