From fc5b1e70e7f9d03d436c9cb684614640f0c98b5d Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Thu, 12 Sep 2024 10:40:52 -0400 Subject: [PATCH] threads: Adds the ability to set a thread priority on newly-created threads (#36019) The thread priority is set right before the thread's callback routine is executed. This functionality will be used in a subsequent change to enable setting the Platform Cert Validator thread priority in Envoy Mobile. --------- Signed-off-by: Ali Beyad --- envoy/thread/thread.h | 11 +++- mobile/library/common/internal_engine.cc | 22 +++----- mobile/library/common/internal_engine.h | 8 ++- mobile/test/common/internal_engine_test.cc | 6 +-- source/common/common/BUILD | 1 + source/common/common/posix/thread_impl.cc | 61 ++++++++++++++++++++-- source/common/common/posix/thread_impl.h | 13 ++++- source/common/common/win32/thread_impl.cc | 4 ++ test/common/common/thread_test.cc | 26 +++++++++ 9 files changed, 124 insertions(+), 28 deletions(-) diff --git a/envoy/thread/thread.h b/envoy/thread/thread.h index 7b969f3e2c01..6ec193a88ec1 100644 --- a/envoy/thread/thread.h +++ b/envoy/thread/thread.h @@ -55,7 +55,16 @@ using ThreadPtr = std::unique_ptr; // Options specified during thread creation. struct Options { - std::string name_; // A name supplied for the thread. On Linux this is limited to 15 chars. + // A name supplied for the thread. On Linux this is limited to 15 chars. + std::string name_; + // An optional thread priority for the thread. The value will mean different things on different + // platforms. For example, on Linux or Android, the values can range from -20 to 19. On Apple + // platforms, the value can range from 1 to 100, which is used to divide by 100 to get a [0,1] + // value that can be used on Apple's NSThread.setThreadPriority method. + // + // If no value is set, the thread will be created with the default thread priority for the + // platform. + absl::optional priority_{absl::nullopt}; }; using OptionsOptConstRef = const absl::optional&; diff --git a/mobile/library/common/internal_engine.cc b/mobile/library/common/internal_engine.cc index 708daf1112e8..4ca9a9075e61 100644 --- a/mobile/library/common/internal_engine.cc +++ b/mobile/library/common/internal_engine.cc @@ -95,20 +95,10 @@ envoy_status_t InternalEngine::cancelStream(envoy_stream_t stream) { // copy-constructible type, so it's not possible to move capture `std::unique_ptr` with // `std::function`. envoy_status_t InternalEngine::run(std::shared_ptr options) { - main_thread_ = thread_factory_->createThread( - [this, options]() mutable -> void { - if (thread_priority_) { - // Set the thread priority before invoking the thread routine. - const int rc = setpriority(PRIO_PROCESS, thread_factory_->currentThreadId().getId(), - *thread_priority_); - if (rc != 0) { - ENVOY_LOG(debug, "failed to set thread priority: {}", Envoy::errorDetails(errno)); - } - } - - main(options); - }, - /* options= */ absl::nullopt, /* crash_on_failure= */ false); + Thread::Options thread_options; + thread_options.priority_ = thread_priority_; + main_thread_ = thread_factory_->createThread([this, options]() mutable -> void { main(options); }, + thread_options, /* crash_on_failure= */ false); return (main_thread_ != nullptr) ? ENVOY_SUCCESS : ENVOY_FAILURE; } @@ -330,7 +320,9 @@ envoy_status_t InternalEngine::recordCounterInc(absl::string_view elements, envo }); } -Event::ProvisionalDispatcher& InternalEngine::dispatcher() { return *dispatcher_; } +Event::ProvisionalDispatcher& InternalEngine::dispatcher() const { return *dispatcher_; } + +Thread::PosixThreadFactory& InternalEngine::threadFactory() const { return *thread_factory_; } void statsAsText(const std::map& all_stats, const std::vector& histograms, diff --git a/mobile/library/common/internal_engine.h b/mobile/library/common/internal_engine.h index 2c0323cfb75c..7566d2f4cba5 100644 --- a/mobile/library/common/internal_engine.h +++ b/mobile/library/common/internal_engine.h @@ -56,7 +56,12 @@ class InternalEngine : public Logger::Loggable { * Accessor for the provisional event dispatcher. * @return Event::ProvisionalDispatcher&, the engine dispatcher. */ - Event::ProvisionalDispatcher& dispatcher(); + Event::ProvisionalDispatcher& dispatcher() const; + + /** + * Accessor for the thread factory. + */ + Thread::PosixThreadFactory& threadFactory() const; envoy_stream_t initStream(); @@ -158,6 +163,7 @@ class InternalEngine : public Logger::Loggable { Stats::Store& getStatsStore(); private: + // Needs access to the private constructor. GTEST_FRIEND_CLASS(InternalEngineTest, ThreadCreationFailed); InternalEngine(std::unique_ptr callbacks, std::unique_ptr logger, diff --git a/mobile/test/common/internal_engine_test.cc b/mobile/test/common/internal_engine_test.cc index c9b909bad027..c15cf0c43423 100644 --- a/mobile/test/common/internal_engine_test.cc +++ b/mobile/test/common/internal_engine_test.cc @@ -476,7 +476,7 @@ class ThreadPriorityInternalEngineTest : public InternalEngineTest { stream_callbacks.on_headers_ = [&](const Http::ResponseHeaderMap&, bool /* end_stream */, envoy_stream_intel) { // Gets the thread priority, so we can check that it's the same thread priority we set. - context.thread_priority = getpriority(PRIO_PROCESS, 0); + context.thread_priority = engine->threadFactory().currentThreadPriority(); }; stream_callbacks.on_complete_ = [&](envoy_stream_intel, envoy_final_stream_intel) { context.on_complete_notification.Notify(); @@ -496,15 +496,11 @@ class ThreadPriorityInternalEngineTest : public InternalEngineTest { } }; -// The setpriority() call fails on some Apple environments. -// TODO(abeyad): investigate what to do for Apple. -#ifndef __APPLE__ TEST_F(ThreadPriorityInternalEngineTest, SetThreadPriority) { const int expected_thread_priority = 10; const int actual_thread_priority = startEngineWithPriority(expected_thread_priority); EXPECT_EQ(actual_thread_priority, expected_thread_priority); } -#endif TEST_F(ThreadPriorityInternalEngineTest, SetOutOfRangeThreadPriority) { // 42 is outside the range of acceptable thread priorities. diff --git a/source/common/common/BUILD b/source/common/common/BUILD index ec85b28e5ba1..a11235facb7f 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -433,6 +433,7 @@ envoy_cc_posix_library( strip_include_prefix = "posix", deps = [ ":assert_lib", + ":utility_lib", "//envoy/thread:thread_interface", ], ) diff --git a/source/common/common/posix/thread_impl.cc b/source/common/common/posix/thread_impl.cc index 012474e1a66c..66ba6f2d5c78 100644 --- a/source/common/common/posix/thread_impl.cc +++ b/source/common/common/posix/thread_impl.cc @@ -3,11 +3,16 @@ #include "envoy/thread/thread.h" #include "source/common/common/assert.h" +#include "source/common/common/utility.h" #include "absl/strings/str_cat.h" #if defined(__linux__) +#include #include +#elif defined(__APPLE__) +#include +#include #endif namespace Envoy { @@ -34,6 +39,29 @@ int64_t getCurrentThreadId() { return tid; } +void setThreadPriority(const int64_t tid, const int priority) { +#if defined(__linux__) + const int rc = setpriority(PRIO_PROCESS, tid, priority); + if (rc != 0) { + ENVOY_LOG_MISC(warn, "failed to set thread priority: {}", Envoy::errorDetails(errno)); + } +#elif defined(__APPLE__) + UNREFERENCED_PARAMETER(tid); + // Use NSThread via the Objective-C runtime to set the thread priority; it's the best way to set + // the thread priority on Apple platforms, and directly invoking `setpriority()` on iOS fails with + // permissions issues, as discovered through manual testing. + Class nsthread = objc_getClass("NSThread"); + id (*getCurrentNSThread)(Class, SEL) = reinterpret_cast(objc_msgSend); + id current_thread = getCurrentNSThread(nsthread, sel_registerName("currentThread")); + void (*setNSThreadPriority)(id, SEL, double) = + reinterpret_cast(objc_msgSend); + double ns_priority = static_cast(priority) / 100.0; + setNSThreadPriority(current_thread, sel_registerName("setThreadPriority:"), ns_priority); +#else +#error "Enable and test pthread id retrieval code for you arch in pthread/thread_impl.cc" +#endif +} + } // namespace // See https://www.man7.org/linux/man-pages/man3/pthread_setname_np.3.html. @@ -41,11 +69,14 @@ int64_t getCurrentThreadId() { // so we need to truncate the string_view to 15 bytes. #define PTHREAD_MAX_THREADNAME_LEN_INCLUDING_NULL_BYTE 16 -ThreadHandle::ThreadHandle(std::function thread_routine) - : thread_routine_(thread_routine) {} +ThreadHandle::ThreadHandle(std::function thread_routine, + absl::optional thread_priority) + : thread_routine_(thread_routine), thread_priority_(thread_priority) {} /** Returns the thread routine. */ -std::function& ThreadHandle::routine() { return thread_routine_; }; +std::function& ThreadHandle::routine() { return thread_routine_; } + +absl::optional ThreadHandle::priority() const { return thread_priority_; } /** Returns the thread handle. */ pthread_t& ThreadHandle::handle() { return thread_handle_; } @@ -140,7 +171,11 @@ int PosixThreadFactory::createPthread(ThreadHandle* thread_handle) { return pthread_create( &thread_handle->handle(), nullptr, [](void* arg) -> void* { - static_cast(arg)->routine()(); + ThreadHandle* handle = static_cast(arg); + if (handle->priority()) { + setThreadPriority(getCurrentThreadId(), *handle->priority()); + } + handle->routine()(); return nullptr; }, reinterpret_cast(thread_handle)); @@ -148,7 +183,8 @@ int PosixThreadFactory::createPthread(ThreadHandle* thread_handle) { PosixThreadPtr PosixThreadFactory::createThread(std::function thread_routine, OptionsOptConstRef options, bool crash_on_failure) { - auto thread_handle = new ThreadHandle(thread_routine); + auto thread_handle = + new ThreadHandle(thread_routine, options ? options->priority_ : absl::nullopt); const int rc = createPthread(thread_handle); if (rc != 0) { delete thread_handle; @@ -164,6 +200,21 @@ PosixThreadPtr PosixThreadFactory::createThread(std::function thread_rou ThreadId PosixThreadFactory::currentThreadId() { return ThreadId(getCurrentThreadId()); }; +int PosixThreadFactory::currentThreadPriority() { +#if defined(__linux__) + return static_cast(getpriority(PRIO_PROCESS, getCurrentThreadId())); +#elif defined(__APPLE__) + Class nsthread = objc_getClass("NSThread"); + SEL selector = sel_registerName("threadPriority"); + double (*getNSThreadPriority)(Class, SEL) = + reinterpret_cast(objc_msgSend); + double thread_priority = getNSThreadPriority(nsthread, selector); + return static_cast(std::round(thread_priority * 100.0)); +#else +#error "Enable and test pthread id retrieval code for you arch in pthread/thread_impl.cc" +#endif +} + ThreadId PosixThreadFactory::currentPthreadId() { #if defined(__linux__) return static_cast(static_cast(pthread_self())); diff --git a/source/common/common/posix/thread_impl.h b/source/common/common/posix/thread_impl.h index 027051115bca..5f8d9a6de408 100644 --- a/source/common/common/posix/thread_impl.h +++ b/source/common/common/posix/thread_impl.h @@ -12,16 +12,20 @@ namespace Thread { class ThreadHandle { public: - explicit ThreadHandle(std::function thread_routine); + ThreadHandle(std::function thread_routine, absl::optional thread_priority); /** Returns the thread routine. */ std::function& routine(); + /** Returns the thread priority, if any. */ + absl::optional priority() const; + /** Returns the thread handle. */ pthread_t& handle(); private: std::function thread_routine_; + const absl::optional thread_priority_; pthread_t thread_handle_; }; @@ -95,6 +99,13 @@ class PosixThreadFactory : public ThreadFactory { */ ThreadId currentThreadId(); + /** + * On Linux and Android, this will return an integer value between [-20, 19]. + * On Apple platforms, thread priorities range from [0,1] but this API normalizes the values to + * [0, 100] for consistency with the Options.priority_ values. + */ + int currentThreadPriority(); + /** Returns the current pthread ID. It uses `pthread_self()`. */ virtual ThreadId currentPthreadId(); diff --git a/source/common/common/win32/thread_impl.cc b/source/common/common/win32/thread_impl.cc index e7e548883a80..c1beac6a5290 100644 --- a/source/common/common/win32/thread_impl.cc +++ b/source/common/common/win32/thread_impl.cc @@ -22,6 +22,10 @@ ThreadImplWin32::ThreadImplWin32(std::function thread_routine, OptionsOp return 0; }, this, 0, nullptr)); + if (options && options.thread_priority_ && + !SetThreadPriority(thread_handle_, *options.thread_priority_)) { + ENVOY_LOG_MISC(warn, "Could not set the thread priority to {}", *options.thread_priority_); + } RELEASE_ASSERT(thread_handle_ != 0, ""); } diff --git a/test/common/common/thread_test.cc b/test/common/common/thread_test.cc index ff3a8bd5a370..92c017afcae6 100644 --- a/test/common/common/thread_test.cc +++ b/test/common/common/thread_test.cc @@ -275,6 +275,32 @@ TEST(PosixThreadTest, Joinable) { EXPECT_FALSE(thread->joinable()); } +TEST(PosixThreadTest, ThreadPriority) { + auto thread_factory = PosixThreadFactory::create(); + Options options; + options.priority_ = 15; + double thread_priority; + auto thread = thread_factory->createThread( + [&]() { thread_priority = thread_factory->currentThreadPriority(); }, options, + /* crash_on_failure= */ false); + thread->join(); + + EXPECT_EQ(thread_priority, options.priority_); +} + +TEST(PosixThreadTest, InvalidThreadPriority) { + auto thread_factory = PosixThreadFactory::create(); + Options options; + options.priority_ = -200; + double thread_priority; + auto thread = thread_factory->createThread( + [&]() { thread_priority = thread_factory->currentThreadPriority(); }, options, + /* crash_on_failure= */ false); + thread->join(); + + EXPECT_NE(thread_priority, options.priority_); +} + class PosixThreadFactoryFailCreate : public PosixThreadFactory { protected: int createPthread(ThreadHandle*) override { return 1; }