From 78e513e15f97db4beaab10d5fc121add3a608ffe Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 11 Sep 2023 15:09:18 -0700 Subject: [PATCH 01/18] bad thread affinity. --- .../platform/android/android_shell_holder.cc | 68 ++++++++++++++++--- 1 file changed, 60 insertions(+), 8 deletions(-) diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 80eb4839fc439..998d803b3364f 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -12,16 +12,11 @@ #include #include -#include #include #include #include "flutter/fml/logging.h" -#include "flutter/fml/make_copyable.h" #include "flutter/fml/message_loop.h" -#include "flutter/fml/native_library.h" -#include "flutter/fml/platform/android/jni_util.h" -#include "flutter/lib/ui/painting/image_generator_registry.h" #include "flutter/shell/common/rasterizer.h" #include "flutter/shell/common/run_configuration.h" #include "flutter/shell/common/thread_host.h" @@ -41,14 +36,43 @@ static void AndroidPlatformThreadConfigSetter( // set thread priority switch (config.priority) { case fml::Thread::ThreadPriority::BACKGROUND: { + cpu_set_t set; + auto count = 8; + CPU_ZERO(&set); + + // Assume the first half of the CPUs are slower. + for (auto i = 0; i < 4; i++) { + CPU_SET(i, &set); + } + if (sched_setaffinity(gettid(), sizeof(set), &set) != 0) { + FML_LOG(ERROR) << "Failed to set affinity"; + } if (::setpriority(PRIO_PROCESS, 0, 10) != 0) { FML_LOG(ERROR) << "Failed to set IO task runner priority"; } break; } case fml::Thread::ThreadPriority::DISPLAY: { - if (::setpriority(PRIO_PROCESS, 0, -1) != 0) { - FML_LOG(ERROR) << "Failed to set UI task runner priority"; + // Android describes -8 as "most important display threads, for + // compositing the screen and retrieving input events". Conservatively + // set the raster thread to slightly lower priority than it. + cpu_set_t set; + CPU_ZERO(&set); + + // Assume the last two CPUs are the fastest... + auto count = 8; + CPU_SET(count - 2, &set); + CPU_SET(count - 1, &set); + if (sched_setaffinity(gettid(), sizeof(set), &set) != 0) { + FML_LOG(ERROR) << "Failed to set affinity"; + } + if (::setpriority(PRIO_PROCESS, 0, -10) != 0) { + FML_LOG(ERROR) << "Failed to set priority to -8"; + // Defensive fallback. Depending on the OEM, it may not be possible + // to set priority to -5. + if (::setpriority(PRIO_PROCESS, 0, -2) != 0) { + FML_LOG(ERROR) << "Failed to set raster task runner priority"; + } } break; } @@ -56,7 +80,18 @@ static void AndroidPlatformThreadConfigSetter( // Android describes -8 as "most important display threads, for // compositing the screen and retrieving input events". Conservatively // set the raster thread to slightly lower priority than it. - if (::setpriority(PRIO_PROCESS, 0, -5) != 0) { + cpu_set_t set; + auto count = 8; + CPU_ZERO(&set); + + // Assume the last two CPUs are the fastest... + CPU_SET(count - 2, &set); + CPU_SET(count - 1, &set); + if (sched_setaffinity(gettid(), sizeof(set), &set) != 0) { + FML_LOG(ERROR) << "Failed to set affinity"; + } + if (::setpriority(PRIO_PROCESS, 0, -10) != 0) { + FML_LOG(ERROR) << "Failed to set priority to -8"; // Defensive fallback. Depending on the OEM, it may not be possible // to set priority to -5. if (::setpriority(PRIO_PROCESS, 0, -2) != 0) { @@ -66,6 +101,23 @@ static void AndroidPlatformThreadConfigSetter( break; } default: + cpu_set_t set; + sched_getaffinity(gettid(), sizeof(set), &set); + auto count = 8; + // Remove whatever affinities we set the engine and ui threads to. + CPU_ZERO(&set); + // Assume the first half of the CPUs are slower. + for (auto i = 0; i < 4; i++) { + CPU_SET(i, &set); + } + if (sched_setaffinity(gettid(), sizeof(set), &set) != 0) { + FML_LOG(ERROR) << "Failed to set affinity"; + } + + if (sched_setaffinity(0, sizeof(set), &set) != 0) { + FML_LOG(ERROR) << "Failed to set affinity"; + } + if (::setpriority(PRIO_PROCESS, 0, 0) != 0) { FML_LOG(ERROR) << "Failed to set priority"; } From a871abdb88b518c33f61745ec186e3861784a51c Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 15 Sep 2023 12:33:26 -0700 Subject: [PATCH 02/18] Add failed CPU parsing. --- fml/BUILD.gn | 2 + fml/affinity.cc | 182 ++++++++++++++++++++++++++++++++++++++++++++++++ fml/affinity.h | 24 +++++++ 3 files changed, 208 insertions(+) create mode 100644 fml/affinity.cc create mode 100644 fml/affinity.h diff --git a/fml/BUILD.gn b/fml/BUILD.gn index a585864159444..260e60d350363 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -8,6 +8,8 @@ import("//flutter/testing/testing.gni") source_set("fml") { sources = [ + "affinity.cc", + "affinity.h", "ascii_trie.cc", "ascii_trie.h", "backtrace.h", diff --git a/fml/affinity.cc b/fml/affinity.cc new file mode 100644 index 0000000000000..aa696f98fcd62 --- /dev/null +++ b/fml/affinity.cc @@ -0,0 +1,182 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/fml/affinity.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "flutter/fml/logging.h" + +namespace fml { + +std::once_flag flag1; +static char* data_ = nullptr; +static intptr_t datalen_ = 0; + +void InitCPUInfo() { + // Get the size of the cpuinfo file by reading it until the end. This is + // required because files under /proc do not always return a valid size + // when using fseek(0, SEEK_END) + ftell(). Nor can they be mmap()-ed. + static const char PATHNAME[] = "/proc/cpuinfo"; + FILE* fp = fopen(PATHNAME, "r"); + if (fp != nullptr) { + for (;;) { + char buffer[256]; + size_t n = fread(buffer, 1, sizeof(buffer), fp); + if (n == 0) { + break; + } + datalen_ += n; + } + fclose(fp); + } + + // Read the contents of the cpuinfo file. + data_ = reinterpret_cast(malloc(datalen_ + 1)); + fp = fopen(PATHNAME, "r"); + if (fp != nullptr) { + for (intptr_t offset = 0; offset < datalen_;) { + size_t n = fread(data_ + offset, 1, datalen_ - offset, fp); + if (n == 0) { + break; + } + offset += n; + } + fclose(fp); + } + + // Zero-terminate the data. + data_[datalen_] = '\0'; +} + +char* FieldStart(const char* field) { + // Look for first field occurrence, and ensure it starts the line. + size_t fieldlen = strlen(field); + char* p = data_; + for (;;) { + p = strstr(p, field); + if (p == nullptr) { + return nullptr; + } + if (p == data_ || p[-1] == '\n') { + break; + } + p += fieldlen; + } + + // Skip to the first colon followed by a space. + p = strchr(p + fieldlen, ':'); + if (p == nullptr || (isspace(p[1]) == 0)) { + return nullptr; + } + p += 2; + + return p; +} + +bool FieldContains(const char* field, const char* search_string) { + FML_DCHECK(data_ != nullptr); + FML_DCHECK(search_string != nullptr); + + char* p = FieldStart(field); + if (p == nullptr) { + return false; + } + + // Find the end of the line. + char* q = strchr(p, '\n'); + if (q == nullptr) { + q = data_ + datalen_; + } + + char saved_end = *q; + *q = '\0'; + bool ret = (strcasestr(p, search_string) != nullptr); + *q = saved_end; + + return ret; +} + +// Extract the content of a the first occurrence of a given field in +// the content of the cpuinfo file and return it as a heap-allocated +// string that must be freed by the caller using free. +// Return nullptr if not found. +const char* ExtractField(const char* field) { + FML_DCHECK(field != nullptr); + FML_DCHECK(data_ != nullptr); + + char* p = FieldStart(field); + if (p == nullptr) { + return nullptr; + } + + // Find the end of the line. + char* q = strchr(p, '\n'); + if (q == nullptr) { + q = data_ + datalen_; + } + + intptr_t len = q - p; + char* result = reinterpret_cast(malloc(len + 1)); + // Copy the line into result, leaving enough room for a null-terminator. + char saved_end = *q; + *q = '\0'; + strncpy(result, p, len); + result[len] = '\0'; + *q = saved_end; + + return result; +} + +bool HasField(const char* field) { + FML_DCHECK(field != nullptr); + FML_DCHECK(data_ != nullptr); + return (FieldStart(field) != nullptr); +} + + +bool RequestAffinity(CpuAffinity affinity) { + // Populate CPU Info if undefined. + std::call_once(flag1, InitCPUInfo); + + // Determine physical core count and speed. + std::vector slow_cores; + std::vector fast_cores; + + // If we either cannot determine slow versus fast cores, or if there is no + // distinction in core speed, return without setting affinity. + if (slow_cores.size() == 0u || fast_cores.size() == 0u) { + FML_LOG(ERROR) << "early reurn"; + return true; + } + + cpu_set_t set; + auto count = 8; // TODO. + CPU_ZERO(&set); + + switch (affinity) { + case CpuAffinity::kPerformance: + // Assume the last N cores are performance; + for (auto i = 0; i < count / 2; i++) { + CPU_SET(i, &set); + } + break; + case CpuAffinity::kEfficiency: + // Assume the first N cores are efficiency; + for (auto i = count / 2; i < count; i++) { + CPU_SET(i, &set); + } + break; + } + return sched_setaffinity(gettid(), sizeof(set), &set) == 0; +} + +} // namespace fml diff --git a/fml/affinity.h b/fml/affinity.h new file mode 100644 index 0000000000000..d29da50ca7d07 --- /dev/null +++ b/fml/affinity.h @@ -0,0 +1,24 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +namespace fml { + +/// The CPU Affinity provides a hint to the operating system on which cores a +/// particular thread should be scheduled on. The operating system may or may +/// not honor these requests. +enum class CpuAffinity { + /// @brief Request CPU affinity for the performance cores. + /// + /// Generally speaking, only the UI and Raster thread should + /// use this option. + kPerformance, + + /// @brief Request CPU affinity for the efficiency cores. + kEfficiency, +}; + +/// @brief Request that the current thread switch to the given `affinity`. +bool RequestAffinity(CpuAffinity affinity); + +} From aa4329da13ea6fbb82e48eff1acd84f86c886624 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 15 Sep 2023 12:34:08 -0700 Subject: [PATCH 03/18] ++ --- fml/affinity.cc | 5 ++--- fml/affinity.h | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/fml/affinity.cc b/fml/affinity.cc index aa696f98fcd62..f585ebf2790e0 100644 --- a/fml/affinity.cc +++ b/fml/affinity.cc @@ -4,13 +4,13 @@ #include "flutter/fml/affinity.h" +#include #include #include +#include #include #include #include -#include -#include #include #include #include "flutter/fml/logging.h" @@ -142,7 +142,6 @@ bool HasField(const char* field) { return (FieldStart(field) != nullptr); } - bool RequestAffinity(CpuAffinity affinity) { // Populate CPU Info if undefined. std::call_once(flag1, InitCPUInfo); diff --git a/fml/affinity.h b/fml/affinity.h index d29da50ca7d07..d70bf1f1eb7ec 100644 --- a/fml/affinity.h +++ b/fml/affinity.h @@ -21,4 +21,4 @@ enum class CpuAffinity { /// @brief Request that the current thread switch to the given `affinity`. bool RequestAffinity(CpuAffinity affinity); -} +} // namespace fml From cdf81f525edbdde6d8d74dcdba787cb6634c4de7 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 15 Sep 2023 18:26:24 -0700 Subject: [PATCH 04/18] ++ --- fml/affinity.cc | 183 +++++--------- .../platform/android/android_shell_holder.cc | 225 +++++++++++++----- 2 files changed, 231 insertions(+), 177 deletions(-) diff --git a/fml/affinity.cc b/fml/affinity.cc index f585ebf2790e0..e3cb1f1c01ee6 100644 --- a/fml/affinity.cc +++ b/fml/affinity.cc @@ -4,156 +4,97 @@ #include "flutter/fml/affinity.h" -#include -#include #include +#include #include #include #include #include -#include -#include +#include +#include + #include "flutter/fml/logging.h" namespace fml { -std::once_flag flag1; -static char* data_ = nullptr; -static intptr_t datalen_ = 0; +#if FML_OS_ANDROID + +struct CpuIndexAndSpeed { + size_t index; + int32_t speed; +}; + +class CPUSpeedTracker { + public: + explicit CPUSpeedTracker(std::vector data) + : cpu_speeds_(data) {} + + private: + std::vector cpu_speeds_; +}; + +std::vector InitCPUInfo(size_t cpu_count) { + std::vector cpu_speeds; + FML_LOG(ERROR) << "CPU COUNT: " << cpu_count; -void InitCPUInfo() { // Get the size of the cpuinfo file by reading it until the end. This is // required because files under /proc do not always return a valid size // when using fseek(0, SEEK_END) + ftell(). Nor can they be mmap()-ed. - static const char PATHNAME[] = "/proc/cpuinfo"; - FILE* fp = fopen(PATHNAME, "r"); - if (fp != nullptr) { - for (;;) { - char buffer[256]; - size_t n = fread(buffer, 1, sizeof(buffer), fp); - if (n == 0) { - break; + for (auto i = 0u; i < cpu_count; i++) { + int data_length = 0; + auto path = "/sys/devices/system/cpu/cpu" + std::to_string(i) + + "/cpufreq/cpuinfo_max_freq"; + FML_LOG(ERROR) << path; + FILE* fp = fopen(path.c_str(), "r"); + if (fp != nullptr) { + for (;;) { + char buffer[256]; + size_t n = fread(buffer, 1, sizeof(buffer), fp); + if (n == 0) { + break; + } + data_length += n; } - datalen_ += n; + fclose(fp); } - fclose(fp); - } - // Read the contents of the cpuinfo file. - data_ = reinterpret_cast(malloc(datalen_ + 1)); - fp = fopen(PATHNAME, "r"); - if (fp != nullptr) { - for (intptr_t offset = 0; offset < datalen_;) { - size_t n = fread(data_ + offset, 1, datalen_ - offset, fp); - if (n == 0) { - break; + // Read the contents of the cpuinfo file. + char* data = reinterpret_cast(malloc(data_length + 1)); + fp = fopen(path.c_str(), "r"); + if (fp != nullptr) { + for (intptr_t offset = 0; offset < data_length;) { + size_t n = fread(data + offset, 1, data_length - offset, fp); + if (n == 0) { + break; + } + offset += n; } - offset += n; + fclose(fp); } - fclose(fp); - } + FML_LOG(ERROR) << data; - // Zero-terminate the data. - data_[datalen_] = '\0'; -} - -char* FieldStart(const char* field) { - // Look for first field occurrence, and ensure it starts the line. - size_t fieldlen = strlen(field); - char* p = data_; - for (;;) { - p = strstr(p, field); - if (p == nullptr) { - return nullptr; + auto speed = std::stoi(data); + if (speed > 0) { + cpu_speeds.push_back({.index = i, .speed = speed}); } - if (p == data_ || p[-1] == '\n') { - break; - } - p += fieldlen; } - // Skip to the first colon followed by a space. - p = strchr(p + fieldlen, ':'); - if (p == nullptr || (isspace(p[1]) == 0)) { - return nullptr; + for (const auto data : cpu_speeds) { + FML_LOG(ERROR) << "CPU INDEX: " << data.index << " " << data.speed; } - p += 2; - return p; -} - -bool FieldContains(const char* field, const char* search_string) { - FML_DCHECK(data_ != nullptr); - FML_DCHECK(search_string != nullptr); - - char* p = FieldStart(field); - if (p == nullptr) { - return false; - } - - // Find the end of the line. - char* q = strchr(p, '\n'); - if (q == nullptr) { - q = data_ + datalen_; - } - - char saved_end = *q; - *q = '\0'; - bool ret = (strcasestr(p, search_string) != nullptr); - *q = saved_end; - - return ret; -} - -// Extract the content of a the first occurrence of a given field in -// the content of the cpuinfo file and return it as a heap-allocated -// string that must be freed by the caller using free. -// Return nullptr if not found. -const char* ExtractField(const char* field) { - FML_DCHECK(field != nullptr); - FML_DCHECK(data_ != nullptr); - - char* p = FieldStart(field); - if (p == nullptr) { - return nullptr; - } - - // Find the end of the line. - char* q = strchr(p, '\n'); - if (q == nullptr) { - q = data_ + datalen_; - } - - intptr_t len = q - p; - char* result = reinterpret_cast(malloc(len + 1)); - // Copy the line into result, leaving enough room for a null-terminator. - char saved_end = *q; - *q = '\0'; - strncpy(result, p, len); - result[len] = '\0'; - *q = saved_end; - - return result; -} - -bool HasField(const char* field) { - FML_DCHECK(field != nullptr); - FML_DCHECK(data_ != nullptr); - return (FieldStart(field) != nullptr); + return cpu_speeds; } bool RequestAffinity(CpuAffinity affinity) { // Populate CPU Info if undefined. - std::call_once(flag1, InitCPUInfo); + auto data = InitCPUInfo(std::thread::hardware_concurrency()); // Determine physical core count and speed. - std::vector slow_cores; - std::vector fast_cores; // If we either cannot determine slow versus fast cores, or if there is no // distinction in core speed, return without setting affinity. - if (slow_cores.size() == 0u || fast_cores.size() == 0u) { - FML_LOG(ERROR) << "early reurn"; + if (data.size() == 0u) { return true; } @@ -178,4 +119,12 @@ bool RequestAffinity(CpuAffinity affinity) { return sched_setaffinity(gettid(), sizeof(set), &set) == 0; } +#else + +bool RequestAffinity(CpuAffinity affinity) { + return false; +} + +#endif + } // namespace fml diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 998d803b3364f..5e23a79af6ec5 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -12,11 +12,16 @@ #include #include +#include #include #include #include "flutter/fml/logging.h" +#include "flutter/fml/make_copyable.h" #include "flutter/fml/message_loop.h" +#include "flutter/fml/native_library.h" +#include "flutter/fml/platform/android/jni_util.h" +#include "flutter/lib/ui/painting/image_generator_registry.h" #include "flutter/shell/common/rasterizer.h" #include "flutter/shell/common/run_configuration.h" #include "flutter/shell/common/thread_host.h" @@ -27,6 +32,159 @@ namespace flutter { +/// The CPU Affinity provides a hint to the operating system on which cores a +/// particular thread should be scheduled on. The operating system may or may +/// not honor these requests. +enum class CpuAffinity { + /// @brief Request CPU affinity for the performance cores. + /// + /// Generally speaking, only the UI and Raster thread should + /// use this option. + kPerformance, + + /// @brief Request CPU affinity for the efficiency cores. + kEfficiency, + + /// @brief Request affinity for all non-performance cores. + kNotPerformance, +}; + +struct CpuIndexAndSpeed { + size_t index; + int32_t speed; +}; + +class CPUSpeedTracker { + public: + explicit CPUSpeedTracker(std::vector data) + : cpu_speeds_(data) { + std::optional max_speed = std::nullopt; + std::optional min_speed = std::nullopt; + for (const auto& data : cpu_speeds_) { + if (!max_speed.has_value() || data.speed > max_speed.value()) { + max_speed = data.speed; + } + if (!min_speed.has_value() || data.speed < min_speed.value()) { + min_speed = data.speed; + } + } + if (!max_speed.has_value() || !min_speed.has_value() || + min_speed.value() == max_speed.value()) { + return; + } + for (const auto& data : cpu_speeds_) { + if (data.speed == max_speed.value()) { + performance_.push_back(data.index); + } else { + not_performance_.push_back(data.index); + } + if (data.speed == min_speed.value()) { + efficiency_.push_back(data.index); + } + } + + valid_ = true; + } + + bool IsValid() const { return valid_; } + + const std::vector& GetIndexes(CpuAffinity affinity) const { + switch (affinity) { + case CpuAffinity::kPerformance: + return performance_; + case CpuAffinity::kEfficiency: + return efficiency_; + case CpuAffinity::kNotPerformance: + return not_performance_; + } + } + + private: + bool valid_ = false; + std::vector cpu_speeds_; + std::vector efficiency_; + std::vector performance_; + std::vector not_performance_; +}; + +std::once_flag cpu_tracker_flag_; +static CPUSpeedTracker* tracker_; + +void InitCPUInfo(size_t cpu_count) { + std::vector cpu_speeds; + + // Get the size of the cpuinfo file by reading it until the end. This is + // required because files under /proc do not always return a valid size + // when using fseek(0, SEEK_END) + ftell(). Nor can they be mmap()-ed. + for (auto i = 0u; i < cpu_count; i++) { + int data_length = 0; + auto path = "/sys/devices/system/cpu/cpu" + std::to_string(i) + + "/cpufreq/cpuinfo_max_freq"; + FILE* fp = fopen(path.c_str(), "r"); + if (fp != nullptr) { + for (;;) { + char buffer[256]; + size_t n = fread(buffer, 1, sizeof(buffer), fp); + if (n == 0) { + break; + } + data_length += n; + } + fclose(fp); + } + + // Read the contents of the cpuinfo file. + char* data = reinterpret_cast(malloc(data_length + 1)); + fp = fopen(path.c_str(), "r"); + if (fp != nullptr) { + for (intptr_t offset = 0; offset < data_length;) { + size_t n = fread(data + offset, 1, data_length - offset, fp); + if (n == 0) { + break; + } + offset += n; + } + fclose(fp); + } + + // Parse the CPU info numbers. + if (data != nullptr) { + auto speed = std::stoi(data); + if (speed > 0) { + cpu_speeds.push_back({.index = i, .speed = speed}); + } + } + } + + tracker_ = new CPUSpeedTracker(cpu_speeds); +} + +bool RequestAffinity(CpuAffinity affinity) { + // Populate CPU Info if undefined. + auto count = std::thread::hardware_concurrency(); + std::call_once(cpu_tracker_flag_, [count]() { + InitCPUInfo(count); + }); + if (tracker_ == nullptr) { + return true; + } + + // Determine physical core count and speed. + + // If we either cannot determine slow versus fast cores, or if there is no + // distinction in core speed, return without setting affinity. + if (!tracker_->IsValid()) { + return true; + } + + cpu_set_t set; + CPU_ZERO(&set); + for (const auto index : tracker_->GetIndexes(affinity)) { + CPU_SET(index, &set); + } + return sched_setaffinity(gettid(), sizeof(set), &set) == 0; +} + /// Inheriting ThreadConfigurer and use Android platform thread API to configure /// the thread priorities static void AndroidPlatformThreadConfigSetter( @@ -36,62 +194,25 @@ static void AndroidPlatformThreadConfigSetter( // set thread priority switch (config.priority) { case fml::Thread::ThreadPriority::BACKGROUND: { - cpu_set_t set; - auto count = 8; - CPU_ZERO(&set); - - // Assume the first half of the CPUs are slower. - for (auto i = 0; i < 4; i++) { - CPU_SET(i, &set); - } - if (sched_setaffinity(gettid(), sizeof(set), &set) != 0) { - FML_LOG(ERROR) << "Failed to set affinity"; - } + RequestAffinity(CpuAffinity::kEfficiency); if (::setpriority(PRIO_PROCESS, 0, 10) != 0) { FML_LOG(ERROR) << "Failed to set IO task runner priority"; } break; } case fml::Thread::ThreadPriority::DISPLAY: { - // Android describes -8 as "most important display threads, for - // compositing the screen and retrieving input events". Conservatively - // set the raster thread to slightly lower priority than it. - cpu_set_t set; - CPU_ZERO(&set); - - // Assume the last two CPUs are the fastest... - auto count = 8; - CPU_SET(count - 2, &set); - CPU_SET(count - 1, &set); - if (sched_setaffinity(gettid(), sizeof(set), &set) != 0) { - FML_LOG(ERROR) << "Failed to set affinity"; - } - if (::setpriority(PRIO_PROCESS, 0, -10) != 0) { - FML_LOG(ERROR) << "Failed to set priority to -8"; - // Defensive fallback. Depending on the OEM, it may not be possible - // to set priority to -5. - if (::setpriority(PRIO_PROCESS, 0, -2) != 0) { - FML_LOG(ERROR) << "Failed to set raster task runner priority"; - } + RequestAffinity(CpuAffinity::kPerformance); + if (::setpriority(PRIO_PROCESS, 0, -1) != 0) { + FML_LOG(ERROR) << "Failed to set UI task runner priority"; } break; } case fml::Thread::ThreadPriority::RASTER: { + RequestAffinity(CpuAffinity::kPerformance); // Android describes -8 as "most important display threads, for // compositing the screen and retrieving input events". Conservatively // set the raster thread to slightly lower priority than it. - cpu_set_t set; - auto count = 8; - CPU_ZERO(&set); - - // Assume the last two CPUs are the fastest... - CPU_SET(count - 2, &set); - CPU_SET(count - 1, &set); - if (sched_setaffinity(gettid(), sizeof(set), &set) != 0) { - FML_LOG(ERROR) << "Failed to set affinity"; - } - if (::setpriority(PRIO_PROCESS, 0, -10) != 0) { - FML_LOG(ERROR) << "Failed to set priority to -8"; + if (::setpriority(PRIO_PROCESS, 0, -5) != 0) { // Defensive fallback. Depending on the OEM, it may not be possible // to set priority to -5. if (::setpriority(PRIO_PROCESS, 0, -2) != 0) { @@ -101,23 +222,7 @@ static void AndroidPlatformThreadConfigSetter( break; } default: - cpu_set_t set; - sched_getaffinity(gettid(), sizeof(set), &set); - auto count = 8; - // Remove whatever affinities we set the engine and ui threads to. - CPU_ZERO(&set); - // Assume the first half of the CPUs are slower. - for (auto i = 0; i < 4; i++) { - CPU_SET(i, &set); - } - if (sched_setaffinity(gettid(), sizeof(set), &set) != 0) { - FML_LOG(ERROR) << "Failed to set affinity"; - } - - if (sched_setaffinity(0, sizeof(set), &set) != 0) { - FML_LOG(ERROR) << "Failed to set affinity"; - } - + RequestAffinity(CpuAffinity::kNotPerformance); if (::setpriority(PRIO_PROCESS, 0, 0) != 0) { FML_LOG(ERROR) << "Failed to set priority"; } From 094d8bc8f2e4478328bf5723d0ea3da5c3187f20 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 15 Sep 2023 18:26:57 -0700 Subject: [PATCH 05/18] ++ --- shell/platform/android/android_shell_holder.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 5e23a79af6ec5..450288b748f29 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -162,9 +162,7 @@ void InitCPUInfo(size_t cpu_count) { bool RequestAffinity(CpuAffinity affinity) { // Populate CPU Info if undefined. auto count = std::thread::hardware_concurrency(); - std::call_once(cpu_tracker_flag_, [count]() { - InitCPUInfo(count); - }); + std::call_once(cpu_tracker_flag_, [count]() { InitCPUInfo(count); }); if (tracker_ == nullptr) { return true; } From 13f4353007b18fbfeffcd430977c71574767af31 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 15 Sep 2023 18:27:39 -0700 Subject: [PATCH 06/18] ++ --- fml/affinity.cc | 117 ------------------------------------------------ 1 file changed, 117 deletions(-) diff --git a/fml/affinity.cc b/fml/affinity.cc index e3cb1f1c01ee6..b39f2800dc882 100644 --- a/fml/affinity.cc +++ b/fml/affinity.cc @@ -4,127 +4,10 @@ #include "flutter/fml/affinity.h" -#include -#include -#include -#include -#include -#include -#include -#include - -#include "flutter/fml/logging.h" - namespace fml { -#if FML_OS_ANDROID - -struct CpuIndexAndSpeed { - size_t index; - int32_t speed; -}; - -class CPUSpeedTracker { - public: - explicit CPUSpeedTracker(std::vector data) - : cpu_speeds_(data) {} - - private: - std::vector cpu_speeds_; -}; - -std::vector InitCPUInfo(size_t cpu_count) { - std::vector cpu_speeds; - FML_LOG(ERROR) << "CPU COUNT: " << cpu_count; - - // Get the size of the cpuinfo file by reading it until the end. This is - // required because files under /proc do not always return a valid size - // when using fseek(0, SEEK_END) + ftell(). Nor can they be mmap()-ed. - for (auto i = 0u; i < cpu_count; i++) { - int data_length = 0; - auto path = "/sys/devices/system/cpu/cpu" + std::to_string(i) + - "/cpufreq/cpuinfo_max_freq"; - FML_LOG(ERROR) << path; - FILE* fp = fopen(path.c_str(), "r"); - if (fp != nullptr) { - for (;;) { - char buffer[256]; - size_t n = fread(buffer, 1, sizeof(buffer), fp); - if (n == 0) { - break; - } - data_length += n; - } - fclose(fp); - } - - // Read the contents of the cpuinfo file. - char* data = reinterpret_cast(malloc(data_length + 1)); - fp = fopen(path.c_str(), "r"); - if (fp != nullptr) { - for (intptr_t offset = 0; offset < data_length;) { - size_t n = fread(data + offset, 1, data_length - offset, fp); - if (n == 0) { - break; - } - offset += n; - } - fclose(fp); - } - FML_LOG(ERROR) << data; - - auto speed = std::stoi(data); - if (speed > 0) { - cpu_speeds.push_back({.index = i, .speed = speed}); - } - } - - for (const auto data : cpu_speeds) { - FML_LOG(ERROR) << "CPU INDEX: " << data.index << " " << data.speed; - } - - return cpu_speeds; -} - -bool RequestAffinity(CpuAffinity affinity) { - // Populate CPU Info if undefined. - auto data = InitCPUInfo(std::thread::hardware_concurrency()); - - // Determine physical core count and speed. - - // If we either cannot determine slow versus fast cores, or if there is no - // distinction in core speed, return without setting affinity. - if (data.size() == 0u) { - return true; - } - - cpu_set_t set; - auto count = 8; // TODO. - CPU_ZERO(&set); - - switch (affinity) { - case CpuAffinity::kPerformance: - // Assume the last N cores are performance; - for (auto i = 0; i < count / 2; i++) { - CPU_SET(i, &set); - } - break; - case CpuAffinity::kEfficiency: - // Assume the first N cores are efficiency; - for (auto i = count / 2; i < count; i++) { - CPU_SET(i, &set); - } - break; - } - return sched_setaffinity(gettid(), sizeof(set), &set) == 0; -} - -#else - bool RequestAffinity(CpuAffinity affinity) { return false; } -#endif - } // namespace fml From f9a70a739b20e1fe45796c2717867262061b669a Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 16 Sep 2023 14:18:20 -0700 Subject: [PATCH 07/18] refactor to FML. --- fml/BUILD.gn | 8 +- fml/affinity.h | 24 --- fml/platform/android/cpu_affinity.cc | 159 +++++++++++++++++ fml/platform/android/cpu_affinity.h | 69 ++++++++ .../android/cpu_affinity_unittests.cc} | 11 +- .../platform/android/android_shell_holder.cc | 160 +----------------- 6 files changed, 247 insertions(+), 184 deletions(-) delete mode 100644 fml/affinity.h create mode 100644 fml/platform/android/cpu_affinity.cc create mode 100644 fml/platform/android/cpu_affinity.h rename fml/{affinity.cc => platform/android/cpu_affinity_unittests.cc} (54%) diff --git a/fml/BUILD.gn b/fml/BUILD.gn index 260e60d350363..d050d8c403c6d 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -8,8 +8,6 @@ import("//flutter/testing/testing.gni") source_set("fml") { sources = [ - "affinity.cc", - "affinity.h", "ascii_trie.cc", "ascii_trie.h", "backtrace.h", @@ -172,6 +170,8 @@ source_set("fml") { if (is_android) { sources += [ + "platform/android/cpu_affinity.cc", + "platform/android/cpu_affinity.h", "platform/android/jni_util.cc", "platform/android/jni_util.h", "platform/android/jni_weak_ref.cc", @@ -369,6 +369,10 @@ if (enable_unittests) { ] } + if (is_android) { + sources += [ "platform/android/cpu_affiity_unittests.cc" ] + } + deps = [ ":fml_fixtures", "//flutter/fml", diff --git a/fml/affinity.h b/fml/affinity.h deleted file mode 100644 index d70bf1f1eb7ec..0000000000000 --- a/fml/affinity.h +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -namespace fml { - -/// The CPU Affinity provides a hint to the operating system on which cores a -/// particular thread should be scheduled on. The operating system may or may -/// not honor these requests. -enum class CpuAffinity { - /// @brief Request CPU affinity for the performance cores. - /// - /// Generally speaking, only the UI and Raster thread should - /// use this option. - kPerformance, - - /// @brief Request CPU affinity for the efficiency cores. - kEfficiency, -}; - -/// @brief Request that the current thread switch to the given `affinity`. -bool RequestAffinity(CpuAffinity affinity); - -} // namespace fml diff --git a/fml/platform/android/cpu_affinity.cc b/fml/platform/android/cpu_affinity.cc new file mode 100644 index 0000000000000..aedb20adb2bd3 --- /dev/null +++ b/fml/platform/android/cpu_affinity.cc @@ -0,0 +1,159 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/fml/platform/android/cpu_affinity.h" + +#include "flutter/fml/logging.h" + +#include +#include +#include +#include +#include +#include +#include + +namespace fml { + +/// The CPUSpeedTracker is initialized once the first time a thread affinity is +/// requested. +std::once_flag cpu_tracker_flag_; +static CPUSpeedTracker* tracker_; + +CPUSpeedTracker::CPUSpeedTracker(std::vector data) + : cpu_speeds_(std::move(data)) { + std::optional max_speed = std::nullopt; + std::optional min_speed = std::nullopt; + for (const auto& data : cpu_speeds_) { + if (!max_speed.has_value() || data.speed > max_speed.value()) { + max_speed = data.speed; + } + if (!min_speed.has_value() || data.speed < min_speed.value()) { + min_speed = data.speed; + } + } + if (!max_speed.has_value() || !min_speed.has_value() || + min_speed.value() == max_speed.value()) { + return; + } + + for (const auto& data : cpu_speeds_) { + if (data.speed == max_speed.value()) { + performance_.push_back(data.index); + } else { + not_performance_.push_back(data.index); + } + if (data.speed == min_speed.value()) { + efficiency_.push_back(data.index); + } + } + + valid_ = true; +} + +bool CPUSpeedTracker::IsValid() const { + return valid_; +} + +const std::vector& CPUSpeedTracker::GetIndices( + CpuAffinity affinity) const { + switch (affinity) { + case CpuAffinity::kPerformance: + return performance_; + case CpuAffinity::kEfficiency: + return efficiency_; + case CpuAffinity::kNotPerformance: + return not_performance_; + } +} + +// Get the size of the cpuinfo file by reading it until the end. This is +// required because files under /proc do not always return a valid size +// when using fseek(0, SEEK_END) + ftell(). Nor can they be mmap()-ed. +std::optional ReadIntFromFile(const std::string& path) { + int data_length = 0u; + FILE* fp = fopen(path.c_str(), "r"); + if (fp == nullptr) { + return std::nullopt; + } + for (;;) { + char buffer[256]; + size_t n = fread(buffer, 1, sizeof(buffer), fp); + if (n == 0) { + break; + } + data_length += n; + } + fclose(fp); + + // Read the contents of the cpuinfo file. + if (data_length <= 0) { + return std::nullopt; + } + + char* data = reinterpret_cast(malloc(data_length + 1)); + fp = fopen(path.c_str(), "r"); + if (fp == nullptr) { + return std::nullopt; + } + + for (intptr_t offset = 0; offset < data_length;) { + size_t n = fread(data + offset, 1, data_length - offset, fp); + if (n == 0) { + break; + } + offset += n; + } + fclose(fp); + + if (data == nullptr) { + return std::nullopt; + } + + auto speed = std::stoi(data); + free(data); + if (speed > 0) { + return speed; + } + return std::nullopt; +} + +// For each CPU index provided, attempts to open the file +// /sys/devices/system/cpu/cpu$NUM/cpufreq/cpuinfo_max_freq and parse a number +// containing the CPU frequency. +void InitCPUInfo(size_t cpu_count) { + std::vector cpu_speeds; + + for (auto i = 0u; i < cpu_count; i++) { + auto path = "/sys/devices/system/cpu/cpu" + std::to_string(i) + + "/cpufreq/cpuinfo_max_freq"; + auto speed = ReadIntFromFile(path); + if (speed.has_value()) { + cpu_speeds.push_back({.index = i, .speed = speed.value()}); + } + } + tracker_ = new CPUSpeedTracker(cpu_speeds); +} + +bool RequestAffinity(CpuAffinity affinity) { + // Populate CPU Info if uninitialized. + auto count = std::thread::hardware_concurrency(); + std::call_once(cpu_tracker_flag_, [count]() { InitCPUInfo(count); }); + if (tracker_ == nullptr) { + return true; + } + + if (!tracker_->IsValid()) { + return true; + } + + cpu_set_t set; + CPU_ZERO(&set); + for (const auto index : tracker_->GetIndices(affinity)) { + CPU_SET(index, &set); + } + return sched_setaffinity(gettid(), sizeof(set), &set) == 0; +} + +} // namespace fml diff --git a/fml/platform/android/cpu_affinity.h b/fml/platform/android/cpu_affinity.h new file mode 100644 index 0000000000000..d1be17ca056b7 --- /dev/null +++ b/fml/platform/android/cpu_affinity.h @@ -0,0 +1,69 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#pragma once + +#include +#include + +namespace fml { + +/// The CPU Affinity provides a hint to the operating system on which cores a +/// particular thread should be scheduled on. The operating system may or may +/// not honor these requests. +enum class CpuAffinity { + /// @brief Request CPU affinity for the performance cores. + /// + /// Generally speaking, only the UI and Raster thread should + /// use this option. + kPerformance, + + /// @brief Request CPU affinity for the efficiency cores. + kEfficiency, + + /// @brief Request affinity for all non-performance cores. + kNotPerformance, +}; + +/// @brief Request the given affinity for the current thread. +/// +/// Returns true if successfull, or if it was a no-op +bool RequestAffinity(CpuAffinity affinity); + +struct CpuIndexAndSpeed { + size_t index; + int32_t speed; +}; + +/// @brief A class that computes the correct CPU indices for a requested CPU +/// affinity. +/// +/// Note: this is visible for testing. +class CPUSpeedTracker { + public: + explicit CPUSpeedTracker(std::vector data); + + /// @brief The class is valid if it has more than one CPU index and a distinct + /// set of + /// efficiency or performance CPUs. If all CPUs are the same speed this + /// returns false, and all requests to set affinity are ignored. + bool IsValid() const; + + /// @brief Return the set of CPU indices for the requested CPU affinity. + /// + /// If the tracker is valid, this will always return a non-empty set. + const std::vector& GetIndices(CpuAffinity affinity) const; + + private: + bool valid_ = false; + std::vector cpu_speeds_; + std::vector efficiency_; + std::vector performance_; + std::vector not_performance_; +}; + +/// Visible for testing. +std::optional ReadIntFromFile(const std::string& path); + +} // namespace fml diff --git a/fml/affinity.cc b/fml/platform/android/cpu_affinity_unittests.cc similarity index 54% rename from fml/affinity.cc rename to fml/platform/android/cpu_affinity_unittests.cc index b39f2800dc882..112517f2c5ea9 100644 --- a/fml/affinity.cc +++ b/fml/platform/android/cpu_affinity_unittests.cc @@ -2,12 +2,17 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/fml/affinity.h" +#include "cpu_affinity.h" + +#include "gtest/gtest.h" +#include "logging.h" namespace fml { +namespace testing { -bool RequestAffinity(CpuAffinity affinity) { - return false; +TEST(CpuAffinity, CanDoThing) { + EXPECT_EQ(true, false); } +} // namespace testing } // namespace fml diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 450288b748f29..39aef72727d9d 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -20,6 +20,7 @@ #include "flutter/fml/make_copyable.h" #include "flutter/fml/message_loop.h" #include "flutter/fml/native_library.h" +#include "flutter/fml/platform/android/cpu_affinity.h" #include "flutter/fml/platform/android/jni_util.h" #include "flutter/lib/ui/painting/image_generator_registry.h" #include "flutter/shell/common/rasterizer.h" @@ -32,157 +33,6 @@ namespace flutter { -/// The CPU Affinity provides a hint to the operating system on which cores a -/// particular thread should be scheduled on. The operating system may or may -/// not honor these requests. -enum class CpuAffinity { - /// @brief Request CPU affinity for the performance cores. - /// - /// Generally speaking, only the UI and Raster thread should - /// use this option. - kPerformance, - - /// @brief Request CPU affinity for the efficiency cores. - kEfficiency, - - /// @brief Request affinity for all non-performance cores. - kNotPerformance, -}; - -struct CpuIndexAndSpeed { - size_t index; - int32_t speed; -}; - -class CPUSpeedTracker { - public: - explicit CPUSpeedTracker(std::vector data) - : cpu_speeds_(data) { - std::optional max_speed = std::nullopt; - std::optional min_speed = std::nullopt; - for (const auto& data : cpu_speeds_) { - if (!max_speed.has_value() || data.speed > max_speed.value()) { - max_speed = data.speed; - } - if (!min_speed.has_value() || data.speed < min_speed.value()) { - min_speed = data.speed; - } - } - if (!max_speed.has_value() || !min_speed.has_value() || - min_speed.value() == max_speed.value()) { - return; - } - for (const auto& data : cpu_speeds_) { - if (data.speed == max_speed.value()) { - performance_.push_back(data.index); - } else { - not_performance_.push_back(data.index); - } - if (data.speed == min_speed.value()) { - efficiency_.push_back(data.index); - } - } - - valid_ = true; - } - - bool IsValid() const { return valid_; } - - const std::vector& GetIndexes(CpuAffinity affinity) const { - switch (affinity) { - case CpuAffinity::kPerformance: - return performance_; - case CpuAffinity::kEfficiency: - return efficiency_; - case CpuAffinity::kNotPerformance: - return not_performance_; - } - } - - private: - bool valid_ = false; - std::vector cpu_speeds_; - std::vector efficiency_; - std::vector performance_; - std::vector not_performance_; -}; - -std::once_flag cpu_tracker_flag_; -static CPUSpeedTracker* tracker_; - -void InitCPUInfo(size_t cpu_count) { - std::vector cpu_speeds; - - // Get the size of the cpuinfo file by reading it until the end. This is - // required because files under /proc do not always return a valid size - // when using fseek(0, SEEK_END) + ftell(). Nor can they be mmap()-ed. - for (auto i = 0u; i < cpu_count; i++) { - int data_length = 0; - auto path = "/sys/devices/system/cpu/cpu" + std::to_string(i) + - "/cpufreq/cpuinfo_max_freq"; - FILE* fp = fopen(path.c_str(), "r"); - if (fp != nullptr) { - for (;;) { - char buffer[256]; - size_t n = fread(buffer, 1, sizeof(buffer), fp); - if (n == 0) { - break; - } - data_length += n; - } - fclose(fp); - } - - // Read the contents of the cpuinfo file. - char* data = reinterpret_cast(malloc(data_length + 1)); - fp = fopen(path.c_str(), "r"); - if (fp != nullptr) { - for (intptr_t offset = 0; offset < data_length;) { - size_t n = fread(data + offset, 1, data_length - offset, fp); - if (n == 0) { - break; - } - offset += n; - } - fclose(fp); - } - - // Parse the CPU info numbers. - if (data != nullptr) { - auto speed = std::stoi(data); - if (speed > 0) { - cpu_speeds.push_back({.index = i, .speed = speed}); - } - } - } - - tracker_ = new CPUSpeedTracker(cpu_speeds); -} - -bool RequestAffinity(CpuAffinity affinity) { - // Populate CPU Info if undefined. - auto count = std::thread::hardware_concurrency(); - std::call_once(cpu_tracker_flag_, [count]() { InitCPUInfo(count); }); - if (tracker_ == nullptr) { - return true; - } - - // Determine physical core count and speed. - - // If we either cannot determine slow versus fast cores, or if there is no - // distinction in core speed, return without setting affinity. - if (!tracker_->IsValid()) { - return true; - } - - cpu_set_t set; - CPU_ZERO(&set); - for (const auto index : tracker_->GetIndexes(affinity)) { - CPU_SET(index, &set); - } - return sched_setaffinity(gettid(), sizeof(set), &set) == 0; -} - /// Inheriting ThreadConfigurer and use Android platform thread API to configure /// the thread priorities static void AndroidPlatformThreadConfigSetter( @@ -192,21 +42,21 @@ static void AndroidPlatformThreadConfigSetter( // set thread priority switch (config.priority) { case fml::Thread::ThreadPriority::BACKGROUND: { - RequestAffinity(CpuAffinity::kEfficiency); + fml::RequestAffinity(fml::CpuAffinity::kEfficiency); if (::setpriority(PRIO_PROCESS, 0, 10) != 0) { FML_LOG(ERROR) << "Failed to set IO task runner priority"; } break; } case fml::Thread::ThreadPriority::DISPLAY: { - RequestAffinity(CpuAffinity::kPerformance); + fml::RequestAffinity(fml::CpuAffinity::kPerformance); if (::setpriority(PRIO_PROCESS, 0, -1) != 0) { FML_LOG(ERROR) << "Failed to set UI task runner priority"; } break; } case fml::Thread::ThreadPriority::RASTER: { - RequestAffinity(CpuAffinity::kPerformance); + fml::RequestAffinity(fml::CpuAffinity::kPerformance); // Android describes -8 as "most important display threads, for // compositing the screen and retrieving input events". Conservatively // set the raster thread to slightly lower priority than it. @@ -220,7 +70,7 @@ static void AndroidPlatformThreadConfigSetter( break; } default: - RequestAffinity(CpuAffinity::kNotPerformance); + fml::RequestAffinity(fml::CpuAffinity::kNotPerformance); if (::setpriority(PRIO_PROCESS, 0, 0) != 0) { FML_LOG(ERROR) << "Failed to set priority"; } From 2135bcea19c8d68161a3af370a7817c1cf7112eb Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 16 Sep 2023 15:27:55 -0700 Subject: [PATCH 08/18] Unittesting. --- ci/licenses_golden/licenses_flutter | 4 + fml/BUILD.gn | 7 +- fml/cpu_affinity.cc | 115 ++++++++++++++++++ fml/cpu_affinity.h | 64 ++++++++++ fml/cpu_affinity_unittests.cc | 90 ++++++++++++++ fml/platform/android/cpu_affinity.cc | 100 --------------- fml/platform/android/cpu_affinity.h | 58 +-------- .../android/cpu_affinity_unittests.cc | 18 --- 8 files changed, 279 insertions(+), 177 deletions(-) create mode 100644 fml/cpu_affinity.cc create mode 100644 fml/cpu_affinity.h create mode 100644 fml/cpu_affinity_unittests.cc delete mode 100644 fml/platform/android/cpu_affinity_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index f37b0279e3c8d..26de8fd8841d1 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -916,6 +916,8 @@ ORIGIN: ../../../flutter/fml/message_loop_task_queues_benchmark.cc + ../../../fl ORIGIN: ../../../flutter/fml/native_library.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/paths.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/paths.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/fml/platform/android/cpu_affinity.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/fml/platform/android/cpu_affinity.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/platform/android/jni_util.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/platform/android/jni_util.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/platform/android/jni_weak_ref.cc + ../../../flutter/LICENSE @@ -3660,6 +3662,8 @@ FILE: ../../../flutter/fml/message_loop_task_queues_benchmark.cc FILE: ../../../flutter/fml/native_library.h FILE: ../../../flutter/fml/paths.cc FILE: ../../../flutter/fml/paths.h +FILE: ../../../flutter/fml/platform/android/cpu_affinity.cc +FILE: ../../../flutter/fml/platform/android/cpu_affinity.h FILE: ../../../flutter/fml/platform/android/jni_util.cc FILE: ../../../flutter/fml/platform/android/jni_util.h FILE: ../../../flutter/fml/platform/android/jni_weak_ref.cc diff --git a/fml/BUILD.gn b/fml/BUILD.gn index d050d8c403c6d..887d7637b7bb7 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -19,6 +19,8 @@ source_set("fml") { "concurrent_message_loop.cc", "concurrent_message_loop.h", "container.h", + "cpu_affinity.cc", + "cpu_affinity.h", "delayed_task.cc", "delayed_task.h", "eintr_wrapper.h", @@ -324,6 +326,7 @@ if (enable_unittests) { "closure_unittests.cc", "command_line_unittest.cc", "container_unittests.cc", + "cpu_affinity_unittests.cc", "endianness_unittests.cc", "file_unittest.cc", "hash_combine_unittests.cc", @@ -369,10 +372,6 @@ if (enable_unittests) { ] } - if (is_android) { - sources += [ "platform/android/cpu_affiity_unittests.cc" ] - } - deps = [ ":fml_fixtures", "//flutter/fml", diff --git a/fml/cpu_affinity.cc b/fml/cpu_affinity.cc new file mode 100644 index 0000000000000..18235941280d9 --- /dev/null +++ b/fml/cpu_affinity.cc @@ -0,0 +1,115 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/fml/cpu_affinity.h" + +#include +#include +#include + +namespace fml { + +CPUSpeedTracker::CPUSpeedTracker(std::vector data) + : cpu_speeds_(std::move(data)) { + std::optional max_speed = std::nullopt; + std::optional min_speed = std::nullopt; + for (const auto& data : cpu_speeds_) { + if (!max_speed.has_value() || data.speed > max_speed.value()) { + max_speed = data.speed; + } + if (!min_speed.has_value() || data.speed < min_speed.value()) { + min_speed = data.speed; + } + } + if (!max_speed.has_value() || !min_speed.has_value() || + min_speed.value() == max_speed.value()) { + return; + } + + for (const auto& data : cpu_speeds_) { + if (data.speed == max_speed.value()) { + performance_.push_back(data.index); + } else { + not_performance_.push_back(data.index); + } + if (data.speed == min_speed.value()) { + efficiency_.push_back(data.index); + } + } + + valid_ = true; +} + +bool CPUSpeedTracker::IsValid() const { + return valid_; +} + +const std::vector& CPUSpeedTracker::GetIndices( + CpuAffinity affinity) const { + switch (affinity) { + case CpuAffinity::kPerformance: + return performance_; + case CpuAffinity::kEfficiency: + return efficiency_; + case CpuAffinity::kNotPerformance: + return not_performance_; + } +} + +// Get the size of the cpuinfo file by reading it until the end. This is +// required because files under /proc do not always return a valid size +// when using fseek(0, SEEK_END) + ftell(). Nor can they be mmap()-ed. +std::optional ReadIntFromFile(const std::string& path) { + int data_length = 0u; + FILE* fp = fopen(path.c_str(), "r"); + if (fp == nullptr) { + return std::nullopt; + } + for (;;) { + char buffer[256]; + size_t n = fread(buffer, 1, sizeof(buffer), fp); + if (n == 0) { + break; + } + data_length += n; + } + fclose(fp); + + // Read the contents of the cpuinfo file. + if (data_length <= 0) { + return std::nullopt; + } + + char* data = reinterpret_cast(malloc(data_length + 1)); + fp = fopen(path.c_str(), "r"); + if (fp == nullptr) { + free(data); + return std::nullopt; + } + + for (intptr_t offset = 0; offset < data_length;) { + size_t n = fread(data + offset, 1, data_length - offset, fp); + if (n == 0) { + break; + } + offset += n; + } + fclose(fp); + + if (data == nullptr) { + return std::nullopt; + } + + // Dont use stoi because if this data isnt a parseable number then it + // will abort, as we compile with exceptions disabled. + int speed = 0; + std::istringstream input(data); + input >> speed; + if (speed > 0) { + return speed; + } + return std::nullopt; +} + +} // namespace fml diff --git a/fml/cpu_affinity.h b/fml/cpu_affinity.h new file mode 100644 index 0000000000000..a633cf9db1da1 --- /dev/null +++ b/fml/cpu_affinity.h @@ -0,0 +1,64 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#pragma once + +#include +#include + +namespace fml { + +/// The CPU Affinity provides a hint to the operating system on which cores a +/// particular thread should be scheduled on. The operating system may or may +/// not honor these requests. +enum class CpuAffinity { + /// @brief Request CPU affinity for the performance cores. + /// + /// Generally speaking, only the UI and Raster thread should + /// use this option. + kPerformance, + + /// @brief Request CPU affinity for the efficiency cores. + kEfficiency, + + /// @brief Request affinity for all non-performance cores. + kNotPerformance, +}; + +struct CpuIndexAndSpeed { + size_t index; + int32_t speed; +}; + +/// @brief A class that computes the correct CPU indices for a requested CPU +/// affinity. +/// +/// Note: this is visible for testing. +class CPUSpeedTracker { + public: + explicit CPUSpeedTracker(std::vector data); + + /// @brief The class is valid if it has more than one CPU index and a distinct + /// set of + /// efficiency or performance CPUs. If all CPUs are the same speed this + /// returns false, and all requests to set affinity are ignored. + bool IsValid() const; + + /// @brief Return the set of CPU indices for the requested CPU affinity. + /// + /// If the tracker is valid, this will always return a non-empty set. + const std::vector& GetIndices(CpuAffinity affinity) const; + + private: + bool valid_ = false; + std::vector cpu_speeds_; + std::vector efficiency_; + std::vector performance_; + std::vector not_performance_; +}; + +/// Visible for testing. +std::optional ReadIntFromFile(const std::string& path); + +} // namespace fml diff --git a/fml/cpu_affinity_unittests.cc b/fml/cpu_affinity_unittests.cc new file mode 100644 index 0000000000000..eb2f010bc66d6 --- /dev/null +++ b/fml/cpu_affinity_unittests.cc @@ -0,0 +1,90 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "cpu_affinity.h" + +#include "fml/file.h" +#include "fml/mapping.h" +#include "gtest/gtest.h" +#include "logging.h" + +namespace fml { +namespace testing { + +TEST(CpuAffinity, NormalSlowMedFastCores) { + auto speeds = {CpuIndexAndSpeed{.index = 0, .speed = 1}, + CpuIndexAndSpeed{.index = 1, .speed = 2}, + CpuIndexAndSpeed{.index = 2, .speed = 3}}; + auto tracker = CPUSpeedTracker(speeds); + + ASSERT_TRUE(tracker.IsValid()); + ASSERT_EQ(tracker.GetIndices(CpuAffinity::kEfficiency)[0], 0u); + ASSERT_EQ(tracker.GetIndices(CpuAffinity::kPerformance)[0], 2u); + ASSERT_EQ(tracker.GetIndices(CpuAffinity::kNotPerformance).size(), 2u); + ASSERT_EQ(tracker.GetIndices(CpuAffinity::kNotPerformance)[0], 0u); + ASSERT_EQ(tracker.GetIndices(CpuAffinity::kNotPerformance)[1], 1u); +} + +TEST(CpuAffinity, NoCpuData) { + auto tracker = CPUSpeedTracker({}); + + ASSERT_FALSE(tracker.IsValid()); +} + +TEST(CpuAffinity, AllSameSpeed) { + auto speeds = {CpuIndexAndSpeed{.index = 0, .speed = 1}, + CpuIndexAndSpeed{.index = 1, .speed = 1}, + CpuIndexAndSpeed{.index = 2, .speed = 1}}; + auto tracker = CPUSpeedTracker(speeds); + + ASSERT_FALSE(tracker.IsValid()); +} + +TEST(CpuAffinity, SingleCore) { + auto speeds = {CpuIndexAndSpeed{.index = 0, .speed = 1}}; + auto tracker = CPUSpeedTracker(speeds); + + ASSERT_FALSE(tracker.IsValid()); +} + +TEST(CpuAffinity, FileParsing) { + fml::ScopedTemporaryDirectory base_dir; + ASSERT_TRUE(base_dir.fd().is_valid()); + + // Generate a fake CPU speed file + fml::DataMapping test_data(std::string("12345")); + ASSERT_TRUE(fml::WriteAtomically(base_dir.fd(), "test_file", test_data)); + + auto file = fml::OpenFileReadOnly(base_dir.fd(), "test_file"); + ASSERT_TRUE(file.is_valid()); + + // Open file and parse speed. + auto result = ReadIntFromFile(base_dir.path() + "/test_file"); + ASSERT_TRUE(result.has_value()); + ASSERT_EQ(result.value_or(0), 12345); +} + +TEST(CpuAffinity, FileParsingWithNonNumber) { + fml::ScopedTemporaryDirectory base_dir; + ASSERT_TRUE(base_dir.fd().is_valid()); + + // Generate a fake CPU speed file + fml::DataMapping test_data(std::string("whoa this isnt a number")); + ASSERT_TRUE(fml::WriteAtomically(base_dir.fd(), "test_file", test_data)); + + auto file = fml::OpenFileReadOnly(base_dir.fd(), "test_file"); + ASSERT_TRUE(file.is_valid()); + + // Open file and parse speed. + auto result = ReadIntFromFile(base_dir.path() + "/test_file"); + ASSERT_FALSE(result.has_value()); +} + +TEST(CpuAffinity, MissingFileParsing) { + auto result = ReadIntFromFile("/does_not_exist"); + ASSERT_FALSE(result.has_value()); +} + +} // namespace testing +} // namespace fml diff --git a/fml/platform/android/cpu_affinity.cc b/fml/platform/android/cpu_affinity.cc index aedb20adb2bd3..d69831eedfca7 100644 --- a/fml/platform/android/cpu_affinity.cc +++ b/fml/platform/android/cpu_affinity.cc @@ -4,8 +4,6 @@ #include "flutter/fml/platform/android/cpu_affinity.h" -#include "flutter/fml/logging.h" - #include #include #include @@ -21,104 +19,6 @@ namespace fml { std::once_flag cpu_tracker_flag_; static CPUSpeedTracker* tracker_; -CPUSpeedTracker::CPUSpeedTracker(std::vector data) - : cpu_speeds_(std::move(data)) { - std::optional max_speed = std::nullopt; - std::optional min_speed = std::nullopt; - for (const auto& data : cpu_speeds_) { - if (!max_speed.has_value() || data.speed > max_speed.value()) { - max_speed = data.speed; - } - if (!min_speed.has_value() || data.speed < min_speed.value()) { - min_speed = data.speed; - } - } - if (!max_speed.has_value() || !min_speed.has_value() || - min_speed.value() == max_speed.value()) { - return; - } - - for (const auto& data : cpu_speeds_) { - if (data.speed == max_speed.value()) { - performance_.push_back(data.index); - } else { - not_performance_.push_back(data.index); - } - if (data.speed == min_speed.value()) { - efficiency_.push_back(data.index); - } - } - - valid_ = true; -} - -bool CPUSpeedTracker::IsValid() const { - return valid_; -} - -const std::vector& CPUSpeedTracker::GetIndices( - CpuAffinity affinity) const { - switch (affinity) { - case CpuAffinity::kPerformance: - return performance_; - case CpuAffinity::kEfficiency: - return efficiency_; - case CpuAffinity::kNotPerformance: - return not_performance_; - } -} - -// Get the size of the cpuinfo file by reading it until the end. This is -// required because files under /proc do not always return a valid size -// when using fseek(0, SEEK_END) + ftell(). Nor can they be mmap()-ed. -std::optional ReadIntFromFile(const std::string& path) { - int data_length = 0u; - FILE* fp = fopen(path.c_str(), "r"); - if (fp == nullptr) { - return std::nullopt; - } - for (;;) { - char buffer[256]; - size_t n = fread(buffer, 1, sizeof(buffer), fp); - if (n == 0) { - break; - } - data_length += n; - } - fclose(fp); - - // Read the contents of the cpuinfo file. - if (data_length <= 0) { - return std::nullopt; - } - - char* data = reinterpret_cast(malloc(data_length + 1)); - fp = fopen(path.c_str(), "r"); - if (fp == nullptr) { - return std::nullopt; - } - - for (intptr_t offset = 0; offset < data_length;) { - size_t n = fread(data + offset, 1, data_length - offset, fp); - if (n == 0) { - break; - } - offset += n; - } - fclose(fp); - - if (data == nullptr) { - return std::nullopt; - } - - auto speed = std::stoi(data); - free(data); - if (speed > 0) { - return speed; - } - return std::nullopt; -} - // For each CPU index provided, attempts to open the file // /sys/devices/system/cpu/cpu$NUM/cpufreq/cpuinfo_max_freq and parse a number // containing the CPU frequency. diff --git a/fml/platform/android/cpu_affinity.h b/fml/platform/android/cpu_affinity.h index d1be17ca056b7..a7ada2d5cb66b 100644 --- a/fml/platform/android/cpu_affinity.h +++ b/fml/platform/android/cpu_affinity.h @@ -4,66 +4,14 @@ #pragma once -#include -#include +#include "flutter/fml/cpu_affinity.h" namespace fml { -/// The CPU Affinity provides a hint to the operating system on which cores a -/// particular thread should be scheduled on. The operating system may or may -/// not honor these requests. -enum class CpuAffinity { - /// @brief Request CPU affinity for the performance cores. - /// - /// Generally speaking, only the UI and Raster thread should - /// use this option. - kPerformance, - - /// @brief Request CPU affinity for the efficiency cores. - kEfficiency, - - /// @brief Request affinity for all non-performance cores. - kNotPerformance, -}; - /// @brief Request the given affinity for the current thread. /// -/// Returns true if successfull, or if it was a no-op +/// Returns true if successfull, or if it was a no-op. This function is +/// only supported on Android devices. bool RequestAffinity(CpuAffinity affinity); -struct CpuIndexAndSpeed { - size_t index; - int32_t speed; -}; - -/// @brief A class that computes the correct CPU indices for a requested CPU -/// affinity. -/// -/// Note: this is visible for testing. -class CPUSpeedTracker { - public: - explicit CPUSpeedTracker(std::vector data); - - /// @brief The class is valid if it has more than one CPU index and a distinct - /// set of - /// efficiency or performance CPUs. If all CPUs are the same speed this - /// returns false, and all requests to set affinity are ignored. - bool IsValid() const; - - /// @brief Return the set of CPU indices for the requested CPU affinity. - /// - /// If the tracker is valid, this will always return a non-empty set. - const std::vector& GetIndices(CpuAffinity affinity) const; - - private: - bool valid_ = false; - std::vector cpu_speeds_; - std::vector efficiency_; - std::vector performance_; - std::vector not_performance_; -}; - -/// Visible for testing. -std::optional ReadIntFromFile(const std::string& path); - } // namespace fml diff --git a/fml/platform/android/cpu_affinity_unittests.cc b/fml/platform/android/cpu_affinity_unittests.cc deleted file mode 100644 index 112517f2c5ea9..0000000000000 --- a/fml/platform/android/cpu_affinity_unittests.cc +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "cpu_affinity.h" - -#include "gtest/gtest.h" -#include "logging.h" - -namespace fml { -namespace testing { - -TEST(CpuAffinity, CanDoThing) { - EXPECT_EQ(true, false); -} - -} // namespace testing -} // namespace fml From e8b5a7e6122a43f857d2840f99d3684b3eca4eb1 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 16 Sep 2023 15:42:16 -0700 Subject: [PATCH 09/18] ++ --- fml/cpu_affinity.h | 1 + 1 file changed, 1 insertion(+) diff --git a/fml/cpu_affinity.h b/fml/cpu_affinity.h index a633cf9db1da1..01e5f47977969 100644 --- a/fml/cpu_affinity.h +++ b/fml/cpu_affinity.h @@ -6,6 +6,7 @@ #include #include +#include namespace fml { From e9d762d9eea9a59849144e6abb4c5088d1f65be7 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 16 Sep 2023 15:48:02 -0700 Subject: [PATCH 10/18] ++ --- fml/cpu_affinity.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fml/cpu_affinity.h b/fml/cpu_affinity.h index 01e5f47977969..4428ab9dd3b2d 100644 --- a/fml/cpu_affinity.h +++ b/fml/cpu_affinity.h @@ -5,8 +5,8 @@ #pragma once #include -#include #include +#include namespace fml { From 2a336e9298cd72c8f0ff9a43dc8a03e501fd9d41 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 16 Sep 2023 15:50:02 -0700 Subject: [PATCH 11/18] licenses --- ci/licenses_golden/excluded_files | 1 + ci/licenses_golden/licenses_flutter | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index 08a10f984b050..b29272b445613 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -86,6 +86,7 @@ ../../../flutter/fml/closure_unittests.cc ../../../flutter/fml/command_line_unittest.cc ../../../flutter/fml/container_unittests.cc +../../../flutter/fml/cpu_affinity_unittests.cc ../../../flutter/fml/endianness_unittests.cc ../../../flutter/fml/file_unittest.cc ../../../flutter/fml/hash_combine_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 26de8fd8841d1..f7973c61da552 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -870,6 +870,8 @@ ORIGIN: ../../../flutter/fml/concurrent_message_loop.cc + ../../../flutter/LICEN ORIGIN: ../../../flutter/fml/concurrent_message_loop.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/concurrent_message_loop_factory.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/container.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/fml/cpu_affinity.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/fml/cpu_affinity.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/dart/dart_converter.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/dart/dart_converter.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/delayed_task.cc + ../../../flutter/LICENSE @@ -3616,6 +3618,8 @@ FILE: ../../../flutter/fml/concurrent_message_loop.cc FILE: ../../../flutter/fml/concurrent_message_loop.h FILE: ../../../flutter/fml/concurrent_message_loop_factory.cc FILE: ../../../flutter/fml/container.h +FILE: ../../../flutter/fml/cpu_affinity.cc +FILE: ../../../flutter/fml/cpu_affinity.h FILE: ../../../flutter/fml/dart/dart_converter.cc FILE: ../../../flutter/fml/dart/dart_converter.h FILE: ../../../flutter/fml/delayed_task.cc From 653e28b53926e66dadb6d624a67aafc46836eff5 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 16 Sep 2023 16:50:02 -0700 Subject: [PATCH 12/18] ++ --- fml/cpu_affinity.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/fml/cpu_affinity.cc b/fml/cpu_affinity.cc index 18235941280d9..d4a018f83a8e6 100644 --- a/fml/cpu_affinity.cc +++ b/fml/cpu_affinity.cc @@ -61,7 +61,7 @@ const std::vector& CPUSpeedTracker::GetIndices( // required because files under /proc do not always return a valid size // when using fseek(0, SEEK_END) + ftell(). Nor can they be mmap()-ed. std::optional ReadIntFromFile(const std::string& path) { - int data_length = 0u; + size_t data_length = 0u; FILE* fp = fopen(path.c_str(), "r"); if (fp == nullptr) { return std::nullopt; @@ -76,19 +76,18 @@ std::optional ReadIntFromFile(const std::string& path) { } fclose(fp); - // Read the contents of the cpuinfo file. if (data_length <= 0) { return std::nullopt; } + // Read the contents of the cpuinfo file. char* data = reinterpret_cast(malloc(data_length + 1)); fp = fopen(path.c_str(), "r"); if (fp == nullptr) { free(data); return std::nullopt; } - - for (intptr_t offset = 0; offset < data_length;) { + for (uintptr_t offset = 0; offset < data_length;) { size_t n = fread(data + offset, 1, data_length - offset, fp); if (n == 0) { break; @@ -98,8 +97,11 @@ std::optional ReadIntFromFile(const std::string& path) { fclose(fp); if (data == nullptr) { + free(data); return std::nullopt; } + // zero end of buffer. + data[data_length] = 0; // Dont use stoi because if this data isnt a parseable number then it // will abort, as we compile with exceptions disabled. From d54b976133d52c6a97d7879bb50daeb6211f2221 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 16 Sep 2023 17:13:58 -0700 Subject: [PATCH 13/18] ++ --- fml/cpu_affinity.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fml/cpu_affinity.cc b/fml/cpu_affinity.cc index d4a018f83a8e6..4d79f94308bc7 100644 --- a/fml/cpu_affinity.cc +++ b/fml/cpu_affinity.cc @@ -100,7 +100,7 @@ std::optional ReadIntFromFile(const std::string& path) { free(data); return std::nullopt; } - // zero end of buffer. + // Ensure zeroed end of buffer before reading. data[data_length] = 0; // Dont use stoi because if this data isnt a parseable number then it @@ -108,6 +108,8 @@ std::optional ReadIntFromFile(const std::string& path) { int speed = 0; std::istringstream input(data); input >> speed; + free(data); + if (speed > 0) { return speed; } From 44e8afa332574a79626a90e63c559b6db7b3009f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 18 Sep 2023 16:08:03 -0700 Subject: [PATCH 14/18] lol ifstream --- fml/cpu_affinity.cc | 51 +++++---------------------------------------- 1 file changed, 5 insertions(+), 46 deletions(-) diff --git a/fml/cpu_affinity.cc b/fml/cpu_affinity.cc index 4d79f94308bc7..fb4e491312487 100644 --- a/fml/cpu_affinity.cc +++ b/fml/cpu_affinity.cc @@ -4,8 +4,8 @@ #include "flutter/fml/cpu_affinity.h" +#include #include -#include #include namespace fml { @@ -61,55 +61,14 @@ const std::vector& CPUSpeedTracker::GetIndices( // required because files under /proc do not always return a valid size // when using fseek(0, SEEK_END) + ftell(). Nor can they be mmap()-ed. std::optional ReadIntFromFile(const std::string& path) { - size_t data_length = 0u; - FILE* fp = fopen(path.c_str(), "r"); - if (fp == nullptr) { - return std::nullopt; - } - for (;;) { - char buffer[256]; - size_t n = fread(buffer, 1, sizeof(buffer), fp); - if (n == 0) { - break; - } - data_length += n; - } - fclose(fp); - - if (data_length <= 0) { - return std::nullopt; - } - - // Read the contents of the cpuinfo file. - char* data = reinterpret_cast(malloc(data_length + 1)); - fp = fopen(path.c_str(), "r"); - if (fp == nullptr) { - free(data); - return std::nullopt; - } - for (uintptr_t offset = 0; offset < data_length;) { - size_t n = fread(data + offset, 1, data_length - offset, fp); - if (n == 0) { - break; - } - offset += n; - } - fclose(fp); - - if (data == nullptr) { - free(data); - return std::nullopt; - } - // Ensure zeroed end of buffer before reading. - data[data_length] = 0; + // size_t data_length = 0u; + std::ifstream file; + file.open(path.c_str()); // Dont use stoi because if this data isnt a parseable number then it // will abort, as we compile with exceptions disabled. int speed = 0; - std::istringstream input(data); - input >> speed; - free(data); - + file >> speed; if (speed > 0) { return speed; } From e7dc40bebe50b2942139a7b8b870b1dadb68dca4 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 18 Sep 2023 16:51:46 -0700 Subject: [PATCH 15/18] int64_t --- fml/cpu_affinity.cc | 6 +++--- fml/cpu_affinity.h | 6 ++++-- fml/platform/android/cpu_affinity.h | 4 ++++ 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/fml/cpu_affinity.cc b/fml/cpu_affinity.cc index fb4e491312487..5f8854361ae2b 100644 --- a/fml/cpu_affinity.cc +++ b/fml/cpu_affinity.cc @@ -12,8 +12,8 @@ namespace fml { CPUSpeedTracker::CPUSpeedTracker(std::vector data) : cpu_speeds_(std::move(data)) { - std::optional max_speed = std::nullopt; - std::optional min_speed = std::nullopt; + std::optional max_speed = std::nullopt; + std::optional min_speed = std::nullopt; for (const auto& data : cpu_speeds_) { if (!max_speed.has_value() || data.speed > max_speed.value()) { max_speed = data.speed; @@ -67,7 +67,7 @@ std::optional ReadIntFromFile(const std::string& path) { // Dont use stoi because if this data isnt a parseable number then it // will abort, as we compile with exceptions disabled. - int speed = 0; + int64_t speed = 0; file >> speed; if (speed > 0) { return speed; diff --git a/fml/cpu_affinity.h b/fml/cpu_affinity.h index 4428ab9dd3b2d..48855e3b3bcb5 100644 --- a/fml/cpu_affinity.h +++ b/fml/cpu_affinity.h @@ -28,12 +28,14 @@ enum class CpuAffinity { }; struct CpuIndexAndSpeed { + // The index of the given CPU. size_t index; - int32_t speed; + // CPU speed in kHZ + int64_t speed; }; /// @brief A class that computes the correct CPU indices for a requested CPU -/// affinity. +/// affinity. /// /// Note: this is visible for testing. class CPUSpeedTracker { diff --git a/fml/platform/android/cpu_affinity.h b/fml/platform/android/cpu_affinity.h index a7ada2d5cb66b..03c59ad5e9126 100644 --- a/fml/platform/android/cpu_affinity.h +++ b/fml/platform/android/cpu_affinity.h @@ -12,6 +12,10 @@ namespace fml { /// /// Returns true if successfull, or if it was a no-op. This function is /// only supported on Android devices. +/// +/// Affinity requests are based on documented CPU speed. This speed data +/// is parsed from cpuinfo_max_freq files, see also: +/// https://www.kernel.org/doc/Documentation/cpu-freq/user-guide.txt bool RequestAffinity(CpuAffinity affinity); } // namespace fml From 6b5e84871e75626be3871b0369d98f68ee98b48b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 18 Sep 2023 16:58:24 -0700 Subject: [PATCH 16/18] doc cleanup --- fml/cpu_affinity.h | 11 ++++++----- fml/platform/android/cpu_affinity.cc | 14 +++++++------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/fml/cpu_affinity.h b/fml/cpu_affinity.h index 48855e3b3bcb5..ee169d7551803 100644 --- a/fml/cpu_affinity.h +++ b/fml/cpu_affinity.h @@ -37,15 +37,16 @@ struct CpuIndexAndSpeed { /// @brief A class that computes the correct CPU indices for a requested CPU /// affinity. /// -/// Note: this is visible for testing. +/// @note This is visible for testing. class CPUSpeedTracker { public: explicit CPUSpeedTracker(std::vector data); /// @brief The class is valid if it has more than one CPU index and a distinct - /// set of - /// efficiency or performance CPUs. If all CPUs are the same speed this - /// returns false, and all requests to set affinity are ignored. + /// set of efficiency or performance CPUs. + /// + /// If all CPUs are the same speed this returns false, and all requests + /// to set affinity are ignored. bool IsValid() const; /// @brief Return the set of CPU indices for the requested CPU affinity. @@ -61,7 +62,7 @@ class CPUSpeedTracker { std::vector not_performance_; }; -/// Visible for testing. +/// @note Visible for testing. std::optional ReadIntFromFile(const std::string& path); } // namespace fml diff --git a/fml/platform/android/cpu_affinity.cc b/fml/platform/android/cpu_affinity.cc index d69831eedfca7..737f8204b9b6e 100644 --- a/fml/platform/android/cpu_affinity.cc +++ b/fml/platform/android/cpu_affinity.cc @@ -16,8 +16,8 @@ namespace fml { /// The CPUSpeedTracker is initialized once the first time a thread affinity is /// requested. -std::once_flag cpu_tracker_flag_; -static CPUSpeedTracker* tracker_; +std::once_flag gCPUTrackerFlag; +static CPUSpeedTracker* gCPUTracker; // For each CPU index provided, attempts to open the file // /sys/devices/system/cpu/cpu$NUM/cpufreq/cpuinfo_max_freq and parse a number @@ -33,24 +33,24 @@ void InitCPUInfo(size_t cpu_count) { cpu_speeds.push_back({.index = i, .speed = speed.value()}); } } - tracker_ = new CPUSpeedTracker(cpu_speeds); + gCPUTracker = new CPUSpeedTracker(cpu_speeds); } bool RequestAffinity(CpuAffinity affinity) { // Populate CPU Info if uninitialized. auto count = std::thread::hardware_concurrency(); - std::call_once(cpu_tracker_flag_, [count]() { InitCPUInfo(count); }); - if (tracker_ == nullptr) { + std::call_once(gCPUTrackerFlag, [count]() { InitCPUInfo(count); }); + if (gCPUTracker == nullptr) { return true; } - if (!tracker_->IsValid()) { + if (!gCPUTracker->IsValid()) { return true; } cpu_set_t set; CPU_ZERO(&set); - for (const auto index : tracker_->GetIndices(affinity)) { + for (const auto index : gCPUTracker->GetIndices(affinity)) { CPU_SET(index, &set); } return sched_setaffinity(gettid(), sizeof(set), &set) == 0; From 01688633d509bfe4730bd1c6e807e33d1442c50b Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 18 Sep 2023 17:30:45 -0700 Subject: [PATCH 17/18] Update fml/cpu_affinity.h Co-authored-by: Zachary Anderson --- fml/cpu_affinity.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fml/cpu_affinity.h b/fml/cpu_affinity.h index ee169d7551803..3ea45a4fa3d3b 100644 --- a/fml/cpu_affinity.h +++ b/fml/cpu_affinity.h @@ -63,6 +63,6 @@ class CPUSpeedTracker { }; /// @note Visible for testing. -std::optional ReadIntFromFile(const std::string& path); +std::optional ReadIntFromFile(const std::string& path); } // namespace fml From 9a04811f96748e9bde8fee0fbb4e026c2a11f244 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 18 Sep 2023 17:30:53 -0700 Subject: [PATCH 18/18] Update fml/cpu_affinity.cc Co-authored-by: Zachary Anderson --- fml/cpu_affinity.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fml/cpu_affinity.cc b/fml/cpu_affinity.cc index 5f8854361ae2b..c4e89613b252a 100644 --- a/fml/cpu_affinity.cc +++ b/fml/cpu_affinity.cc @@ -60,7 +60,7 @@ const std::vector& CPUSpeedTracker::GetIndices( // Get the size of the cpuinfo file by reading it until the end. This is // required because files under /proc do not always return a valid size // when using fseek(0, SEEK_END) + ftell(). Nor can they be mmap()-ed. -std::optional ReadIntFromFile(const std::string& path) { +std::optional ReadIntFromFile(const std::string& path) { // size_t data_length = 0u; std::ifstream file; file.open(path.c_str());