From 1fb6f004e17588811dd1d4577c5284413ea1d7dc Mon Sep 17 00:00:00 2001 From: Anirudh Subramanian Date: Mon, 21 Oct 2019 11:37:29 -0700 Subject: [PATCH] Build dmlc-core with old thread_local implementation (#16526) --- 3rdparty/dmlc-core | 2 +- CMakeLists.txt | 3 + Makefile | 2 + tests/cpp/engine/thread_local_test.cc | 80 +++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 tests/cpp/engine/thread_local_test.cc diff --git a/3rdparty/dmlc-core b/3rdparty/dmlc-core index 9088d2ee02cd..0e13243556d7 160000 --- a/3rdparty/dmlc-core +++ b/3rdparty/dmlc-core @@ -1 +1 @@ -Subproject commit 9088d2ee02cdfe393fd0569af4aefebe94f8b105 +Subproject commit 0e13243556d7de754a923c8f97998721d694f29b diff --git a/CMakeLists.txt b/CMakeLists.txt index 70b09917de13..99ad34bed3dc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -101,6 +101,9 @@ if("$ENV{VERBOSE}" STREQUAL "1") set(CMAKE_VERBOSE_MAKEFILE ON) endif() +#Switch off modern thread local for dmlc-core, please see: https://github.com/dmlc/dmlc-core/issues/571#issuecomment-543467484 +add_definitions(-DDMLC_MODERN_THREAD_LOCAL=0) + if(MSVC) add_definitions(-DWIN32_LEAN_AND_MEAN) diff --git a/Makefile b/Makefile index e1eeefed4bb7..be0d34051873 100644 --- a/Makefile +++ b/Makefile @@ -94,6 +94,8 @@ include $(DMLC_CORE)/make/dmlc.mk # all tge possible warning tread WARNFLAGS= -Wall -Wsign-compare CFLAGS = -DMSHADOW_FORCE_STREAM $(WARNFLAGS) +# use old thread local implementation in DMLC-CORE +CFLAGS += -DDMLC_MODERN_THREAD_LOCAL=0 ifeq ($(DEV), 1) CFLAGS += -g -Werror diff --git a/tests/cpp/engine/thread_local_test.cc b/tests/cpp/engine/thread_local_test.cc new file mode 100644 index 000000000000..e074e18af2e9 --- /dev/null +++ b/tests/cpp/engine/thread_local_test.cc @@ -0,0 +1,80 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/*! + * Copyright (c) 2019 by Contributors + * \file engine_thread_local_test.cc + * \brief Tests thread safety and lifetime of thread local store +*/ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +struct A { + std::vector a; +}; +int num_threads = 10; +int num_elements = num_threads * 10; + +static int ThreadSafetyTest(int num, std::vector* tmp_inputs, std::vector* res) { + A *ret = dmlc::ThreadLocalStore::Get(); + for (size_t i = num * 10; i < num * 10 + 10; ++i) { + (*tmp_inputs)[i] = i; + } + ret->a.clear(); + ret->a.reserve(10); + for (size_t i = num * 10; i < num * 10 + 10; ++i) { + ret->a.push_back((*tmp_inputs)[i]); + } + (*res)[num] = dmlc::BeginPtr(ret->a); + return 0; +} + +TEST(ThreadLocal, verify_thread_safety) { + std::vector tmp_inputs; + tmp_inputs.resize(num_elements); + std::vector outputs; + outputs.resize(num_threads); + auto func = [&](int num) { + ThreadSafetyTest(num, &tmp_inputs, &outputs); + }; + std::vector worker_threads(num_threads); + int count = 0; + for (auto&& i : worker_threads) { + i = std::thread(func, count); + count++; + } + for (auto&& i : worker_threads) { + i.join(); + } + + for (size_t i = 0; i < num_elements; i++) { + CHECK(outputs[i/10][i%10] == i); + } +}