Improve/fix named threads (#131)

* Remove name of main thread to avoid duplicate main thread names

* Use feature flag to disable named threads on unsupported platforms

CMakeLists.txt

@@ -71,7 +71,7 @@ if(CELERITY_SYCL_IMPL STREQUAL "DPC++")
   message(STATUS "DPC++ will target ${CELERITY_DPCPP_TARGETS}")
 endif()
 
-set(CELERITY_IS_OLD_COMPUTECPP_COMPILER OFF)
+set(CELERITY_DETAIL_IS_OLD_COMPUTECPP_COMPILER OFF)
 if(CELERITY_SYCL_IMPL STREQUAL "ComputeCpp")
   # Determining the compiler version would usually be a job for FindComputeCpp, but since we're vendoring it, we try
   # to avoid introducing unnecessary changes.
@@ -97,7 +97,7 @@ if(CELERITY_SYCL_IMPL STREQUAL "ComputeCpp")
     list(APPEND COMPUTECPP_DEVICE_COMPILER_FLAGS -sycl-std=2020)
   else()
     list(APPEND COMPUTECPP_DEVICE_COMPILER_FLAGS -DSYCL_LANGUAGE_VERSION=COMPUTECPP_SYCL_VERSION_2020)
-    set(CELERITY_IS_OLD_COMPUTECPP_COMPILER ON)
+    set(CELERITY_DETAIL_IS_OLD_COMPUTECPP_COMPILER ON)
   endif()
 endif()
 
@@ -199,12 +199,16 @@ set(SOURCES
   "${CMAKE_CURRENT_BINARY_DIR}/src/version.cc"
 )
 
+set(CELERITY_DETAIL_HAS_NAMED_THREADS OFF)
+
 if(WIN32)
   set(SOURCES ${SOURCES} src/platform_specific/affinity.win.cc)
   set(SOURCES ${SOURCES} src/platform_specific/named_threads.win.cc)
+  set(CELERITY_DETAIL_HAS_NAMED_THREADS ON)
 elseif(UNIX)
   if(NOT APPLE)
     set(SOURCES ${SOURCES} src/platform_specific/affinity.unix.cc)
+    set(CELERITY_DETAIL_HAS_NAMED_THREADS ON)
   endif()
   set(SOURCES ${SOURCES} src/platform_specific/named_threads.unix.cc)
 endif()
@@ -264,7 +268,8 @@ target_compile_definitions(celerity_runtime PUBLIC
   CELERITY_FEATURE_SIMPLE_SCALAR_REDUCTIONS=$<BOOL:${CELERITY_FEATURE_SIMPLE_SCALAR_REDUCTIONS}>
   CELERITY_FEATURE_LOCAL_ACCESSOR=$<BOOL:${CELERITY_FEATURE_LOCAL_ACCESSOR}>
   CELERITY_FEATURE_UNNAMED_KERNELS=$<BOOL:${CELERITY_FEATURE_UNNAMED_KERNELS}>
-  CELERITY_IS_OLD_COMPUTECPP_COMPILER=$<BOOL:${CELERITY_IS_OLD_COMPUTECPP_COMPILER}>
+  CELERITY_DETAIL_HAS_NAMED_THREADS=$<BOOL:${CELERITY_DETAIL_HAS_NAMED_THREADS}>
+  CELERITY_DETAIL_IS_OLD_COMPUTECPP_COMPILER=$<BOOL:${CELERITY_DETAIL_IS_OLD_COMPUTECPP_COMPILER}>
 )
 
 # Collect version information from git in src/version.cc. This target is always out of date, but the timestamp

include/handler.h

@@ -59,15 +59,15 @@ namespace detail {
 	template <typename KernelName>
 	constexpr bool is_unnamed_kernel = std::is_same_v<KernelName, unnamed_kernel>;
 
-#if CELERITY_IS_OLD_COMPUTECPP_COMPILER
+#if CELERITY_DETAIL_IS_OLD_COMPUTECPP_COMPILER
 	template <typename KernelName>
 	struct kernel_name_wrapper;
 #endif
 
 	template <typename KernelName>
 	struct bound_kernel_name {
 		static_assert(!is_unnamed_kernel<KernelName>);
-#if CELERITY_IS_OLD_COMPUTECPP_COMPILER
+#if CELERITY_DETAIL_IS_OLD_COMPUTECPP_COMPILER
 		using type = kernel_name_wrapper<KernelName>; // Suppress -Rsycl-kernel-naming diagnostic for local types
 #else
 		using type = KernelName;

src/platform_specific/named_threads.unix.cc

@@ -13,18 +13,24 @@ constexpr auto PTHREAD_MAX_THREAD_NAME_LEN = 16;
 
 std::thread::native_handle_type get_current_thread_handle() { return pthread_self(); }
 
-void set_thread_name(const std::thread::native_handle_type thread_handle, const std::string& name) {
+void set_thread_name([[maybe_unused]] const std::thread::native_handle_type thread_handle, [[maybe_unused]] const std::string& name) {
+#if CELERITY_DETAIL_HAS_NAMED_THREADS
 	auto truncated_name = name;
 	truncated_name.resize(PTHREAD_MAX_THREAD_NAME_LEN - 1); // -1 because of null terminator
 	[[maybe_unused]] const auto res = pthread_setname_np(thread_handle, truncated_name.c_str());
 	assert(res == 0 && "Failed to set thread name");
+#endif
 }
 
-std::string get_thread_name(const std::thread::native_handle_type thread_handle) {
+std::string get_thread_name([[maybe_unused]] const std::thread::native_handle_type thread_handle) {
+#if CELERITY_DETAIL_HAS_NAMED_THREADS
 	char name[PTHREAD_MAX_THREAD_NAME_LEN] = {};
 	[[maybe_unused]] const auto res = pthread_getname_np(thread_handle, name, PTHREAD_MAX_THREAD_NAME_LEN);
 	assert(res == 0 && "Failed to get thread name");
 	return name; // Automatically strips null terminator
+#else
+	return {};
+#endif
 }
 
 } // namespace celerity::detail

src/platform_specific/named_threads.win.cc

@@ -14,7 +14,7 @@ static inline std::string convert_string(const std::wstring& str) {
 	const auto* src = str.c_str();
 	auto mbstate = std::mbstate_t{};
 	const auto len = std::wcsrtombs(nullptr, &src, 0, &mbstate);
-	auto dst = std::string(len, L'\0'); // Automatically includes space for the null terminator
+	auto dst = std::string(len, '\0'); // Automatically includes space for the null terminator
 	std::wcsrtombs(dst.data(), &src, dst.size(), &mbstate);
 	return dst;
 }

src/runtime.cc

@@ -182,7 +182,6 @@ namespace detail {
 		m_is_active = true;
 		if(is_master_node()) { m_schdlr->startup(); }
 		m_exec->startup();
-		set_thread_name(get_current_thread_handle(), "cy-main");
 	}
 
 	void runtime::shutdown() {

test/buffer_manager_tests.cc

@@ -50,7 +50,7 @@ namespace detail {
 	}
 
 // ComputeCPP based on Clang 8 segfaults in these tests
-#if !CELERITY_IS_OLD_COMPUTECPP_COMPILER
+#if !CELERITY_DETAIL_IS_OLD_COMPUTECPP_COMPILER
 
 	TEST_CASE_METHOD(test_utils::buffer_manager_fixture, "buffer_manager keeps track of buffers", "[buffer_manager]") {
 		std::vector<std::pair<buffer_manager::buffer_lifecycle_event, buffer_id>> cb_calls;
@@ -1098,7 +1098,7 @@ namespace detail {
 		}
 	}
 
-#endif // CELERITY_IS_OLD_COMPUTECPP_COMPILER
+#endif // CELERITY_DETAIL_IS_OLD_COMPUTECPP_COMPILER
 
 } // namespace detail
 } // namespace celerity

test/runtime_tests.cc

@@ -965,6 +965,8 @@ namespace detail {
 		CHECK(exterior == std::vector{1, 2});
 	}
 
+#if CELERITY_DETAIL_HAS_NAMED_THREADS
+
 	TEST_CASE_METHOD(test_utils::runtime_fixture, "thread names are set", "[threads]") {
 		distr_queue q;
 
@@ -980,9 +982,6 @@ namespace detail {
 		const auto executor_thread_name = get_thread_name(executor_testspy::get_exec_thrd(exec).native_handle());
 		CHECK(executor_thread_name == "cy-executor");
 
-		const auto main_thread_name = get_thread_name(get_current_thread_handle());
-		CHECK(main_thread_name == "cy-main");
-
 		q.submit([](handler& cgh) {
 			cgh.host_task(experimental::collective, [&](experimental::collective_partition) {
 				const auto base_name = std::string("cy-worker-");
@@ -992,5 +991,7 @@ namespace detail {
 		});
 	}
 
+#endif
+
 } // namespace detail
 } // namespace celerity