Skip to content

Commit

Permalink
Remove ROCKSDB_SUPPORT_THREAD_LOCAL define because it's a part of C++…
Browse files Browse the repository at this point in the history
…11 (#10015)

Summary:
ROCKSDB_SUPPORT_THREAD_LOCAL definition has been removed.
`__thread`(#define) has been replaced with `thread_local`(C++ keyword) across the code base.

Pull Request resolved: #10015

Reviewed By: siying

Differential Revision: D36485491

Pulled By: pdillinger

fbshipit-source-id: 6522d212514ee190b90b4e2750c80c7e34013c78
  • Loading branch information
torwig authored and facebook-github-bot committed May 18, 2022
1 parent e3a3dbf commit 0a43061
Show file tree
Hide file tree
Showing 25 changed files with 32 additions and 114 deletions.
3 changes: 0 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -337,9 +337,6 @@ endif()
# Reset the required flags
set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})

# thread_local is part of C++11 and later (TODO: clean up this define)
add_definitions(-DROCKSDB_SUPPORT_THREAD_LOCAL)

option(WITH_IOSTATS_CONTEXT "Enable IO stats context" ON)
if (NOT WITH_IOSTATS_CONTEXT)
add_definitions(-DNIOSTATS_CONTEXT)
Expand Down
3 changes: 0 additions & 3 deletions build_tools/build_detect_platform
Original file line number Diff line number Diff line change
Expand Up @@ -800,9 +800,6 @@ if [ "$?" = 0 ]; then
COMMON_FLAGS="$COMMON_FLAGS -DHAVE_UINT128_EXTENSION"
fi

# thread_local is part of C++11 and later (TODO: clean up this define)
COMMON_FLAGS="$COMMON_FLAGS -DROCKSDB_SUPPORT_THREAD_LOCAL"

if [ "$FBCODE_BUILD" != "true" -a "$PLATFORM" = OS_LINUX ]; then
$CXX $COMMON_FLAGS $PLATFORM_SHARED_CFLAGS -x c++ -c - -o test_dl.o 2>/dev/null <<EOF
void dummy_func() {}
Expand Down
2 changes: 1 addition & 1 deletion build_tools/fbcode_config.sh
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ else
fi

CFLAGS+=" $DEPS_INCLUDE"
CFLAGS+=" -DROCKSDB_PLATFORM_POSIX -DROCKSDB_LIB_IO_POSIX -DROCKSDB_FALLOCATE_PRESENT -DROCKSDB_MALLOC_USABLE_SIZE -DROCKSDB_RANGESYNC_PRESENT -DROCKSDB_SCHED_GETCPU_PRESENT -DROCKSDB_SUPPORT_THREAD_LOCAL -DHAVE_SSE42"
CFLAGS+=" -DROCKSDB_PLATFORM_POSIX -DROCKSDB_LIB_IO_POSIX -DROCKSDB_FALLOCATE_PRESENT -DROCKSDB_MALLOC_USABLE_SIZE -DROCKSDB_RANGESYNC_PRESENT -DROCKSDB_SCHED_GETCPU_PRESENT -DHAVE_SSE42"
CXXFLAGS+=" $CFLAGS"

EXEC_LDFLAGS=" $SNAPPY_LIBS $ZLIB_LIBS $BZIP_LIBS $LZ4_LIBS $ZSTD_LIBS $GFLAGS_LIBS $NUMA_LIB $TBB_LIBS"
Expand Down
2 changes: 1 addition & 1 deletion build_tools/fbcode_config_platform009.sh
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ else
fi

CFLAGS+=" $DEPS_INCLUDE"
CFLAGS+=" -DROCKSDB_PLATFORM_POSIX -DROCKSDB_LIB_IO_POSIX -DROCKSDB_FALLOCATE_PRESENT -DROCKSDB_MALLOC_USABLE_SIZE -DROCKSDB_RANGESYNC_PRESENT -DROCKSDB_SCHED_GETCPU_PRESENT -DROCKSDB_SUPPORT_THREAD_LOCAL -DHAVE_SSE42 -DROCKSDB_IOURING_PRESENT"
CFLAGS+=" -DROCKSDB_PLATFORM_POSIX -DROCKSDB_LIB_IO_POSIX -DROCKSDB_FALLOCATE_PRESENT -DROCKSDB_MALLOC_USABLE_SIZE -DROCKSDB_RANGESYNC_PRESENT -DROCKSDB_SCHED_GETCPU_PRESENT -DHAVE_SSE42 -DROCKSDB_IOURING_PRESENT"
CXXFLAGS+=" $CFLAGS"

EXEC_LDFLAGS=" $SNAPPY_LIBS $ZLIB_LIBS $BZIP_LIBS $LZ4_LIBS $ZSTD_LIBS $GFLAGS_LIBS $NUMA_LIB $TBB_LIBS $LIBURING_LIBS $BENCHMARK_LIBS"
Expand Down
2 changes: 1 addition & 1 deletion build_tools/fbcode_config_platform010.sh
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ CFLAGS+=" -isystem $KERNEL_HEADERS_INCLUDE/linux "
CFLAGS+=" -isystem $KERNEL_HEADERS_INCLUDE "

CFLAGS+=" $DEPS_INCLUDE"
CFLAGS+=" -DROCKSDB_PLATFORM_POSIX -DROCKSDB_LIB_IO_POSIX -DROCKSDB_FALLOCATE_PRESENT -DROCKSDB_MALLOC_USABLE_SIZE -DROCKSDB_RANGESYNC_PRESENT -DROCKSDB_SCHED_GETCPU_PRESENT -DROCKSDB_SUPPORT_THREAD_LOCAL -DHAVE_SSE42 -DROCKSDB_IOURING_PRESENT"
CFLAGS+=" -DROCKSDB_PLATFORM_POSIX -DROCKSDB_LIB_IO_POSIX -DROCKSDB_FALLOCATE_PRESENT -DROCKSDB_MALLOC_USABLE_SIZE -DROCKSDB_RANGESYNC_PRESENT -DROCKSDB_SCHED_GETCPU_PRESENT -DHAVE_SSE42 -DROCKSDB_IOURING_PRESENT"
CXXFLAGS+=" $CFLAGS"

EXEC_LDFLAGS=" $SNAPPY_LIBS $ZLIB_LIBS $BZIP_LIBS $LZ4_LIBS $ZSTD_LIBS $GFLAGS_LIBS $NUMA_LIB $TBB_LIBS $LIBURING_LIBS $BENCHMARK_LIBS"
Expand Down
8 changes: 0 additions & 8 deletions db_stress_tool/db_stress_shared_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,6 @@
#include "db_stress_tool/db_stress_shared_state.h"

namespace ROCKSDB_NAMESPACE {
#if defined(ROCKSDB_SUPPORT_THREAD_LOCAL)
#if defined(OS_SOLARIS)
__thread bool SharedState::ignore_read_error;
#else
thread_local bool SharedState::ignore_read_error;
#endif // OS_SOLARIS
#else
bool SharedState::ignore_read_error;
#endif // ROCKSDB_SUPPORT_THREAD_LOCAL
} // namespace ROCKSDB_NAMESPACE
#endif // GFLAGS
8 changes: 0 additions & 8 deletions db_stress_tool/db_stress_shared_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,7 @@ class SharedState {
// local variable updated via sync points to keep track of errors injected
// while reading filter blocks in order to ignore the Get/MultiGet result
// for those calls
#if defined(ROCKSDB_SUPPORT_THREAD_LOCAL)
#if defined(OS_SOLARIS)
static __thread bool ignore_read_error;
#else
static thread_local bool ignore_read_error;
#endif // OS_SOLARIS
#else
static bool ignore_read_error;
#endif // ROCKSDB_SUPPORT_THREAD_LOCAL

SharedState(Env* /*env*/, StressTest* stress_test)
: cv_(&mu_),
Expand Down
6 changes: 2 additions & 4 deletions include/rocksdb/iostats_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,8 @@ struct IOStatsContext {
// If RocksDB is compiled with -DNIOSTATS_CONTEXT, then a pointer to a global,
// non-thread-local IOStatsContext object will be returned. Attempts to update
// this object will be ignored, and reading from it will also be no-op.
// Otherwise,
// a) if thread-local is supported on the platform, then a pointer to
// a thread-local IOStatsContext object will be returned.
// b) if thread-local is NOT supported, then compilation will fail.
// Otherwise, a pointer to a thread-local IOStatsContext object will be
// returned.
//
// This function never returns nullptr.
IOStatsContext* get_iostats_context();
Expand Down
3 changes: 1 addition & 2 deletions include/rocksdb/thread_status.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@

#include "rocksdb/rocksdb_namespace.h"

#if !defined(ROCKSDB_LITE) && !defined(NROCKSDB_THREAD_STATUS) && \
defined(ROCKSDB_SUPPORT_THREAD_LOCAL)
#if !defined(ROCKSDB_LITE) && !defined(NROCKSDB_THREAD_STATUS)
#define ROCKSDB_USING_THREAD_STATUS
#endif

Expand Down
6 changes: 1 addition & 5 deletions memory/concurrent_arena.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@

namespace ROCKSDB_NAMESPACE {

#ifdef ROCKSDB_SUPPORT_THREAD_LOCAL
__thread size_t ConcurrentArena::tls_cpuid = 0;
#endif
thread_local size_t ConcurrentArena::tls_cpuid = 0;

namespace {
// If the shard block size is too large, in the worst case, every core
Expand All @@ -36,11 +34,9 @@ ConcurrentArena::ConcurrentArena(size_t block_size, AllocTracker* tracker,

ConcurrentArena::Shard* ConcurrentArena::Repick() {
auto shard_and_index = shards_.AccessElementAndIndex();
#ifdef ROCKSDB_SUPPORT_THREAD_LOCAL
// even if we are cpu 0, use a non-zero tls_cpuid so we can tell we
// have repicked
tls_cpuid = shard_and_index.second | shards_.Size();
#endif
return shard_and_index.first;
}

Expand Down
6 changes: 1 addition & 5 deletions memory/concurrent_arena.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,7 @@ class ConcurrentArena : public Allocator {
Shard() : free_begin_(nullptr), allocated_and_unused_(0) {}
};

#ifdef ROCKSDB_SUPPORT_THREAD_LOCAL
static __thread size_t tls_cpuid;
#else
enum ZeroFirstEnum : size_t { tls_cpuid = 0 };
#endif
static thread_local size_t tls_cpuid;

char padding0[56] ROCKSDB_FIELD_UNUSED;

Expand Down
5 changes: 1 addition & 4 deletions monitoring/iostats_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,8 @@ namespace ROCKSDB_NAMESPACE {
// Should not be used because the counters are not thread-safe.
// Put here just to make get_iostats_context() simple without ifdef.
static IOStatsContext iostats_context;
#elif defined(ROCKSDB_SUPPORT_THREAD_LOCAL)
__thread IOStatsContext iostats_context;
#else
#error \
"No thread-local support. Disable iostats context with -DNIOSTATS_CONTEXT."
thread_local IOStatsContext iostats_context;
#endif

IOStatsContext* get_iostats_context() {
Expand Down
8 changes: 4 additions & 4 deletions monitoring/iostats_context_imp.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
#include "monitoring/perf_step_timer.h"
#include "rocksdb/iostats_context.h"

#if defined(ROCKSDB_SUPPORT_THREAD_LOCAL) && !defined(NIOSTATS_CONTEXT)
#if !defined(NIOSTATS_CONTEXT)
namespace ROCKSDB_NAMESPACE {
extern __thread IOStatsContext iostats_context;
extern thread_local IOStatsContext iostats_context;
} // namespace ROCKSDB_NAMESPACE

// increment a specific counter by the specified value
Expand Down Expand Up @@ -40,7 +40,7 @@ extern __thread IOStatsContext iostats_context;
PerfLevel::kEnableTimeAndCPUTimeExceptForMutex); \
iostats_step_timer_##metric.Start();

#else // ROCKSDB_SUPPORT_THREAD_LOCAL && !NIOSTATS_CONTEXT
#else // !NIOSTATS_CONTEXT

#define IOSTATS_ADD(metric, value)
#define IOSTATS_ADD_IF_POSITIVE(metric, value)
Expand All @@ -53,4 +53,4 @@ extern __thread IOStatsContext iostats_context;
#define IOSTATS_TIMER_GUARD(metric)
#define IOSTATS_CPU_TIMER_GUARD(metric, clock) static_cast<void>(clock)

#endif // ROCKSDB_SUPPORT_THREAD_LOCAL && !NIOSTATS_CONTEXT
#endif // !NIOSTATS_CONTEXT
10 changes: 2 additions & 8 deletions monitoring/perf_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,16 @@ namespace ROCKSDB_NAMESPACE {
// Should not be used because the counters are not thread-safe.
// Put here just to make get_perf_context() simple without ifdef.
PerfContext perf_context;
#elif defined(ROCKSDB_SUPPORT_THREAD_LOCAL)
#if defined(OS_SOLARIS)
__thread PerfContext perf_context;
#else // OS_SOLARIS
thread_local PerfContext perf_context;
#endif // OS_SOLARIS
#else
#error "No thread-local support. Disable perf context with -DNPERF_CONTEXT."
thread_local PerfContext perf_context;
#endif

PerfContext* get_perf_context() {
return &perf_context;
}

PerfContext::~PerfContext() {
#if !defined(NPERF_CONTEXT) && defined(ROCKSDB_SUPPORT_THREAD_LOCAL) && !defined(OS_SOLARIS)
#if !defined(NPERF_CONTEXT) && !defined(OS_SOLARIS)
ClearPerLevelPerfContext();
#endif
}
Expand Down
4 changes: 2 additions & 2 deletions monitoring/perf_context_imp.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
#include "util/stop_watch.h"

namespace ROCKSDB_NAMESPACE {
#if defined(NPERF_CONTEXT) || !defined(ROCKSDB_SUPPORT_THREAD_LOCAL)
#if defined(NPERF_CONTEXT)
extern PerfContext perf_context;
#else
#if defined(OS_SOLARIS)
extern __thread PerfContext perf_context_;
extern thread_local PerfContext perf_context_;
#define perf_context (*get_perf_context())
#else
extern thread_local PerfContext perf_context;
Expand Down
6 changes: 1 addition & 5 deletions monitoring/perf_level.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,7 @@

namespace ROCKSDB_NAMESPACE {

#ifdef ROCKSDB_SUPPORT_THREAD_LOCAL
__thread PerfLevel perf_level = kEnableCount;
#else
PerfLevel perf_level = kEnableCount;
#endif
thread_local PerfLevel perf_level = kEnableCount;

void SetPerfLevel(PerfLevel level) {
assert(level > kUninitialized);
Expand Down
6 changes: 1 addition & 5 deletions monitoring/perf_level_imp.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@

namespace ROCKSDB_NAMESPACE {

#ifdef ROCKSDB_SUPPORT_THREAD_LOCAL
extern __thread PerfLevel perf_level;
#else
extern PerfLevel perf_level;
#endif
extern thread_local PerfLevel perf_level;

} // namespace ROCKSDB_NAMESPACE
3 changes: 2 additions & 1 deletion monitoring/thread_status_updater.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ namespace ROCKSDB_NAMESPACE {

#ifdef ROCKSDB_USING_THREAD_STATUS

__thread ThreadStatusData* ThreadStatusUpdater::thread_status_data_ = nullptr;
thread_local ThreadStatusData* ThreadStatusUpdater::thread_status_data_ =
nullptr;

void ThreadStatusUpdater::RegisterThread(ThreadStatus::ThreadType ttype,
uint64_t thread_id) {
Expand Down
2 changes: 1 addition & 1 deletion monitoring/thread_status_updater.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class ThreadStatusUpdater {
protected:
#ifdef ROCKSDB_USING_THREAD_STATUS
// The thread-local variable for storing thread status.
static __thread ThreadStatusData* thread_status_data_;
static thread_local ThreadStatusData* thread_status_data_;

// Returns the pointer to the thread status data only when the
// thread status data is non-null and has enable_tracking == true.
Expand Down
6 changes: 3 additions & 3 deletions monitoring/thread_status_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
namespace ROCKSDB_NAMESPACE {

#ifdef ROCKSDB_USING_THREAD_STATUS
__thread ThreadStatusUpdater* ThreadStatusUtil::thread_updater_local_cache_ =
nullptr;
__thread bool ThreadStatusUtil::thread_updater_initialized_ = false;
thread_local ThreadStatusUpdater*
ThreadStatusUtil::thread_updater_local_cache_ = nullptr;
thread_local bool ThreadStatusUtil::thread_updater_initialized_ = false;

void ThreadStatusUtil::RegisterThread(const Env* env,
ThreadStatus::ThreadType thread_type) {
Expand Down
4 changes: 2 additions & 2 deletions monitoring/thread_status_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class ThreadStatusUtil {
// When this variable is set to true, thread_updater_local_cache_
// will not be updated until this variable is again set to false
// in UnregisterThread().
static __thread bool thread_updater_initialized_;
static thread_local bool thread_updater_initialized_;

// The thread-local cached ThreadStatusUpdater that caches the
// thread_status_updater_ of the first Env that uses any ThreadStatusUtil
Expand All @@ -109,7 +109,7 @@ class ThreadStatusUtil {
// When thread_updater_initialized_ is set to true, this variable
// will not be updated until this thread_updater_initialized_ is
// again set to false in UnregisterThread().
static __thread ThreadStatusUpdater* thread_updater_local_cache_;
static thread_local ThreadStatusUpdater* thread_updater_local_cache_;
#else
static bool thread_updater_initialized_;
static ThreadStatusUpdater* thread_updater_local_cache_;
Expand Down
5 changes: 0 additions & 5 deletions port/port_posix.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@

#define ROCKSDB_NOEXCEPT noexcept

// thread_local is part of C++11 and later (TODO: clean up this define)
#ifndef __thread
#define __thread thread_local
#endif

#undef PLATFORM_IS_LITTLE_ENDIAN
#if defined(OS_MACOSX)
#include <machine/endian.h>
Expand Down
5 changes: 0 additions & 5 deletions port/win/port_win.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,6 @@ using ssize_t = SSIZE_T;
#ifdef _MSC_VER
#define __attribute__(A)

// thread_local is part of C++11 and later (TODO: clean up this define)
#ifndef __thread
#define __thread thread_local
#endif

#endif

namespace ROCKSDB_NAMESPACE {
Expand Down
6 changes: 1 addition & 5 deletions util/random.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,7 @@
#include "port/likely.h"
#include "util/thread_local.h"

#ifdef ROCKSDB_SUPPORT_THREAD_LOCAL
#define STORAGE_DECL static __thread
#else
#define STORAGE_DECL static
#endif
#define STORAGE_DECL static thread_local

namespace ROCKSDB_NAMESPACE {

Expand Down
27 changes: 4 additions & 23 deletions util/thread_local.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,20 +140,15 @@ class ThreadLocalPtr::StaticMeta {
// The private mutex. Developers should always use Mutex() instead of
// using this variable directly.
port::Mutex mutex_;
#ifdef ROCKSDB_SUPPORT_THREAD_LOCAL
// Thread local storage
static __thread ThreadData* tls_;
#endif
static thread_local ThreadData* tls_;

// Used to make thread exit trigger possible if !defined(OS_MACOSX).
// Otherwise, used to retrieve thread data.
pthread_key_t pthread_key_;
};


#ifdef ROCKSDB_SUPPORT_THREAD_LOCAL
__thread ThreadData* ThreadLocalPtr::StaticMeta::tls_ = nullptr;
#endif
thread_local ThreadData* ThreadLocalPtr::StaticMeta::tls_ = nullptr;

// Windows doesn't support a per-thread destructor with its
// TLS primitives. So, we build it manually by inserting a
Expand Down Expand Up @@ -263,13 +258,10 @@ ThreadLocalPtr::StaticMeta* ThreadLocalPtr::Instance() {
// the following variable will go first, then OnThreadExit, therefore causing
// invalid access.
//
// The above problem can be solved by using thread_local to store tls_ instead
// of using __thread. The major difference between thread_local and __thread
// is that thread_local supports dynamic construction and destruction of
// The above problem can be solved by using thread_local to store tls_.
// thread_local supports dynamic construction and destruction of
// non-primitive typed variables. As a result, we can guarantee the
// destruction order even when the main thread dies before any child threads.
// However, thread_local is not supported in all compilers that accept -std=c++11
// (e.g., eg Mac with XCode < 8. XCode 8+ supports thread_local).
static ThreadLocalPtr::StaticMeta* inst = new ThreadLocalPtr::StaticMeta();
return inst;
}
Expand Down Expand Up @@ -328,10 +320,6 @@ ThreadLocalPtr::StaticMeta::StaticMeta()
#if !defined(OS_WIN)
static struct A {
~A() {
#ifndef ROCKSDB_SUPPORT_THREAD_LOCAL
ThreadData* tls_ =
static_cast<ThreadData*>(pthread_getspecific(Instance()->pthread_key_));
#endif
if (tls_) {
OnThreadExit(tls_);
}
Expand Down Expand Up @@ -366,13 +354,6 @@ void ThreadLocalPtr::StaticMeta::RemoveThreadData(
}

ThreadData* ThreadLocalPtr::StaticMeta::GetThreadLocal() {
#ifndef ROCKSDB_SUPPORT_THREAD_LOCAL
// Make this local variable name look like a member variable so that we
// can share all the code below
ThreadData* tls_ =
static_cast<ThreadData*>(pthread_getspecific(Instance()->pthread_key_));
#endif

if (UNLIKELY(tls_ == nullptr)) {
auto* inst = Instance();
tls_ = new ThreadData(inst);
Expand Down

0 comments on commit 0a43061

Please sign in to comment.