github-actions[bot] commented on code in PR #45284:
URL: https://github.com/apache/doris/pull/45284#discussion_r1880044835
##########
be/src/util/cgroup_util.cpp:
##########
@@ -264,4 +268,136 @@ void CGroupUtil::read_int_metric_from_cgroup_file(
}
}
+Status CGroupUtil::read_string_line_from_cgroup_file(const
std::filesystem::path& file_path,
+ std::string* line_ptr) {
+ std::ifstream file_stream(file_path, std::ios::in);
+ if (!file_stream.is_open()) {
+ return Status::CgroupError("Error open {}", file_path.string());
+ }
+ string line;
+ getline(file_stream, line);
+ if (file_stream.fail() || file_stream.bad()) {
+ return Status::CgroupError("Error reading {}: {}", file_path.string(),
get_str_err_msg());
+ }
+ *line_ptr = line;
+ return Status::OK();
+}
+
+Status CGroupUtil::parse_cpuset_line(std::string cpuset_line, int*
cpu_count_ptr) {
+ if (cpuset_line.empty()) {
+ return Status::CgroupError("cpuset line is empty");
+ }
+ std::vector<string> ranges;
+ boost::split(ranges, cpuset_line, boost::is_any_of(","));
+ int cpu_count = 0;
+
+ for (const std::string& range : ranges) {
+ std::vector<std::string> cpu_values;
+ boost::split(cpu_values, range, boost::is_any_of("-"));
+
+ if (cpu_values.size() == 2) {
+ int start = std::stoi(cpu_values[0]);
+ int end = std::stoi(cpu_values[1]);
+ cpu_count += (end - start) + 1;
+ } else {
+ cpu_count++;
+ }
+ }
+ *cpu_count_ptr = cpu_count;
+ return Status::OK();
+}
+
+int CGroupUtil::get_cgroup_limited_cpu_number(int physical_cores) {
Review Comment:
warning: function 'get_cgroup_limited_cpu_number' has cognitive complexity
of 51 (threshold 50) [readability-function-cognitive-complexity]
```cpp
int CGroupUtil::get_cgroup_limited_cpu_number(int physical_cores) {
^
```
<details>
<summary>Additional context</summary>
**be/src/util/cgroup_util.cpp:310:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
if (physical_cores <= 0) {
^
```
**be/src/util/cgroup_util.cpp:319:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
if (CGroupUtil::cgroupsv2_enable()) {
^
```
**be/src/util/cgroup_util.cpp:321:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
if (cgroupv2_process_path.empty()) {
^
```
**be/src/util/cgroup_util.cpp:325:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
while (current_cgroup_path != default_cgroups_mount.parent_path()) {
^
```
**be/src/util/cgroup_util.cpp:327:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
if (cpu_max_file.is_open()) {
^
```
**be/src/util/cgroup_util.cpp:331:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
if (cpu_limit_str != "max" && cpu_period != 0) {
^
```
**be/src/util/cgroup_util.cpp:331:** +1
```cpp
if (cpu_limit_str != "max" && cpu_period != 0) {
^
```
**be/src/util/cgroup_util.cpp:340:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
while (current_cgroup_path != default_cgroups_mount.parent_path()) {
^
```
**be/src/util/cgroup_util.cpp:343:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
if (cpuset_cpus_file.is_open()) {
^
```
**be/src/util/cgroup_util.cpp:346:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
if (cpuset_line.empty()) {
^
```
**be/src/util/cgroup_util.cpp:355:** +1, nesting level increased to 1
```cpp
} else if (CGroupUtil::cgroupsv1_enable()) {
^
```
**be/src/util/cgroup_util.cpp:360:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
if (cpu_quota_ret.ok() && cpu_quota_path.size() != 0) {
^
```
**be/src/util/cgroup_util.cpp:360:** +1
```cpp
if (cpu_quota_ret.ok() && cpu_quota_path.size() != 0) {
^
```
**be/src/util/cgroup_util.cpp:362:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
while (current_cgroup_path !=
default_cgroups_mount.parent_path()) {
^
```
**be/src/util/cgroup_util.cpp:365:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
if (cpu_quota_file.is_open() && cpu_period_file.is_open()) {
^
```
**be/src/util/cgroup_util.cpp:365:** +1
```cpp
if (cpu_quota_file.is_open() && cpu_period_file.is_open()) {
^
```
**be/src/util/cgroup_util.cpp:370:** +5, including nesting penalty of 4,
nesting level increased to 5
```cpp
if (cpu_quota_value > 0 && cpu_period_value > 0) {
^
```
**be/src/util/cgroup_util.cpp:370:** +1
```cpp
if (cpu_quota_value > 0 && cpu_period_value > 0) {
^
```
**be/src/util/cgroup_util.cpp:386:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
if (cpuset_ret.ok()) {
^
```
**be/src/util/cgroup_util.cpp:390:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
if (cpuset_ret.ok() && !cpuset_line.empty()) {
^
```
**be/src/util/cgroup_util.cpp:390:** +1
```cpp
if (cpuset_ret.ok() && !cpuset_line.empty()) {
^
```
**be/src/util/cgroup_util.cpp:392:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
if (cpuset_count > 0) {
^
```
</details>
##########
be/src/util/cgroup_util.cpp:
##########
@@ -264,4 +268,136 @@
}
}
+Status CGroupUtil::read_string_line_from_cgroup_file(const
std::filesystem::path& file_path,
+ std::string* line_ptr) {
+ std::ifstream file_stream(file_path, std::ios::in);
+ if (!file_stream.is_open()) {
+ return Status::CgroupError("Error open {}", file_path.string());
+ }
+ string line;
+ getline(file_stream, line);
+ if (file_stream.fail() || file_stream.bad()) {
+ return Status::CgroupError("Error reading {}: {}", file_path.string(),
get_str_err_msg());
+ }
+ *line_ptr = line;
+ return Status::OK();
+}
+
+Status CGroupUtil::parse_cpuset_line(std::string cpuset_line, int*
cpu_count_ptr) {
+ if (cpuset_line.empty()) {
+ return Status::CgroupError("cpuset line is empty");
+ }
+ std::vector<string> ranges;
+ boost::split(ranges, cpuset_line, boost::is_any_of(","));
+ int cpu_count = 0;
+
+ for (const std::string& range : ranges) {
+ std::vector<std::string> cpu_values;
+ boost::split(cpu_values, range, boost::is_any_of("-"));
+
+ if (cpu_values.size() == 2) {
+ int start = std::stoi(cpu_values[0]);
+ int end = std::stoi(cpu_values[1]);
+ cpu_count += (end - start) + 1;
+ } else {
+ cpu_count++;
+ }
+ }
+ *cpu_count_ptr = cpu_count;
+ return Status::OK();
+}
+
+int CGroupUtil::get_cgroup_limited_cpu_number(int physical_cores) {
Review Comment:
warning: function 'get_cgroup_limited_cpu_number' exceeds recommended
size/complexity thresholds [readability-function-size]
```cpp
int CGroupUtil::get_cgroup_limited_cpu_number(int physical_cores) {
^
```
<details>
<summary>Additional context</summary>
**be/src/util/cgroup_util.cpp:309:** 91 lines including whitespace and
comments (threshold 80)
```cpp
int CGroupUtil::get_cgroup_limited_cpu_number(int physical_cores) {
^
```
</details>
##########
be/src/util/cgroup_util.cpp:
##########
@@ -264,4 +268,136 @@
}
}
+Status CGroupUtil::read_string_line_from_cgroup_file(const
std::filesystem::path& file_path,
+ std::string* line_ptr) {
+ std::ifstream file_stream(file_path, std::ios::in);
+ if (!file_stream.is_open()) {
+ return Status::CgroupError("Error open {}", file_path.string());
+ }
+ string line;
+ getline(file_stream, line);
+ if (file_stream.fail() || file_stream.bad()) {
+ return Status::CgroupError("Error reading {}: {}", file_path.string(),
get_str_err_msg());
+ }
+ *line_ptr = line;
+ return Status::OK();
+}
+
+Status CGroupUtil::parse_cpuset_line(std::string cpuset_line, int*
cpu_count_ptr) {
+ if (cpuset_line.empty()) {
+ return Status::CgroupError("cpuset line is empty");
+ }
+ std::vector<string> ranges;
+ boost::split(ranges, cpuset_line, boost::is_any_of(","));
+ int cpu_count = 0;
+
+ for (const std::string& range : ranges) {
+ std::vector<std::string> cpu_values;
+ boost::split(cpu_values, range, boost::is_any_of("-"));
+
+ if (cpu_values.size() == 2) {
+ int start = std::stoi(cpu_values[0]);
+ int end = std::stoi(cpu_values[1]);
+ cpu_count += (end - start) + 1;
+ } else {
+ cpu_count++;
+ }
+ }
+ *cpu_count_ptr = cpu_count;
+ return Status::OK();
+}
+
+int CGroupUtil::get_cgroup_limited_cpu_number(int physical_cores) {
+ if (physical_cores <= 0) {
+ return physical_cores;
+ }
+ int ret = physical_cores;
+#if defined(OS_LINUX)
+ // For cgroup v2
+ // Child cgroup's cpu.max may bigger than parent group's cpu.max,
+ // so it should look up from current cgroup to top group.
+ // For cpuset, child cgroup's cpuset.cpus could not bigger thant parent's
cpuset.cpus.
+ if (CGroupUtil::cgroupsv2_enable()) {
+ std::string cgroupv2_process_path = CGroupUtil::cgroupv2_of_process();
+ if (cgroupv2_process_path.empty()) {
+ return ret;
+ }
+ std::filesystem::path current_cgroup_path = (default_cgroups_mount /
cgroupv2_process_path);
+ while (current_cgroup_path != default_cgroups_mount.parent_path()) {
+ std::ifstream cpu_max_file(current_cgroup_path / "cpu.max");
+ if (cpu_max_file.is_open()) {
+ std::string cpu_limit_str;
+ float cpu_period;
+ cpu_max_file >> cpu_limit_str >> cpu_period;
+ if (cpu_limit_str != "max" && cpu_period != 0) {
+ float cpu_limit = std::stof(cpu_limit_str);
+ ret = std::min(static_cast<int>(std::ceil(cpu_limit /
cpu_period)), ret);
Review Comment:
warning: call to 'ceil' promotes float to double
[performance-type-promotion-in-math-fn]
be/src/util/cgroup_util.cpp:20:
```diff
- #include <fstream>
+ #include <cmath>
+ #include <fstream>
```
```suggestion
ret = std::min(static_cast<int>(std::ceil(cpu_limit /
cpu_period)), ret);
```
##########
be/src/util/cgroup_util.cpp:
##########
@@ -264,4 +268,136 @@
}
}
+Status CGroupUtil::read_string_line_from_cgroup_file(const
std::filesystem::path& file_path,
+ std::string* line_ptr) {
+ std::ifstream file_stream(file_path, std::ios::in);
+ if (!file_stream.is_open()) {
+ return Status::CgroupError("Error open {}", file_path.string());
+ }
+ string line;
+ getline(file_stream, line);
+ if (file_stream.fail() || file_stream.bad()) {
+ return Status::CgroupError("Error reading {}: {}", file_path.string(),
get_str_err_msg());
+ }
+ *line_ptr = line;
+ return Status::OK();
+}
+
+Status CGroupUtil::parse_cpuset_line(std::string cpuset_line, int*
cpu_count_ptr) {
+ if (cpuset_line.empty()) {
+ return Status::CgroupError("cpuset line is empty");
+ }
+ std::vector<string> ranges;
+ boost::split(ranges, cpuset_line, boost::is_any_of(","));
+ int cpu_count = 0;
+
+ for (const std::string& range : ranges) {
+ std::vector<std::string> cpu_values;
+ boost::split(cpu_values, range, boost::is_any_of("-"));
+
+ if (cpu_values.size() == 2) {
+ int start = std::stoi(cpu_values[0]);
+ int end = std::stoi(cpu_values[1]);
+ cpu_count += (end - start) + 1;
+ } else {
+ cpu_count++;
+ }
+ }
+ *cpu_count_ptr = cpu_count;
+ return Status::OK();
+}
+
+int CGroupUtil::get_cgroup_limited_cpu_number(int physical_cores) {
+ if (physical_cores <= 0) {
+ return physical_cores;
+ }
+ int ret = physical_cores;
+#if defined(OS_LINUX)
+ // For cgroup v2
+ // Child cgroup's cpu.max may bigger than parent group's cpu.max,
+ // so it should look up from current cgroup to top group.
+ // For cpuset, child cgroup's cpuset.cpus could not bigger thant parent's
cpuset.cpus.
+ if (CGroupUtil::cgroupsv2_enable()) {
+ std::string cgroupv2_process_path = CGroupUtil::cgroupv2_of_process();
+ if (cgroupv2_process_path.empty()) {
+ return ret;
+ }
+ std::filesystem::path current_cgroup_path = (default_cgroups_mount /
cgroupv2_process_path);
+ while (current_cgroup_path != default_cgroups_mount.parent_path()) {
+ std::ifstream cpu_max_file(current_cgroup_path / "cpu.max");
+ if (cpu_max_file.is_open()) {
+ std::string cpu_limit_str;
+ float cpu_period;
+ cpu_max_file >> cpu_limit_str >> cpu_period;
+ if (cpu_limit_str != "max" && cpu_period != 0) {
+ float cpu_limit = std::stof(cpu_limit_str);
+ ret = std::min(static_cast<int>(std::ceil(cpu_limit /
cpu_period)), ret);
+ }
+ }
+ current_cgroup_path = current_cgroup_path.parent_path();
+ }
+
+ current_cgroup_path = (default_cgroups_mount / cgroupv2_process_path);
+ while (current_cgroup_path != default_cgroups_mount.parent_path()) {
+ std::ifstream cpuset_cpus_file(current_cgroup_path /
"cpuset.cpus.effective");
+ current_cgroup_path = current_cgroup_path.parent_path();
+ if (cpuset_cpus_file.is_open()) {
+ std::string cpuset_line;
+ cpuset_cpus_file >> cpuset_line;
+ if (cpuset_line.empty()) {
+ continue;
+ }
+ int cpus_count = 0;
+ static_cast<void>(CGroupUtil::parse_cpuset_line(cpuset_line,
&cpus_count));
+ ret = std::min(cpus_count, ret);
+ break;
+ }
+ }
+ } else if (CGroupUtil::cgroupsv1_enable()) {
+ // cpu quota, should find first not empty config from current path to
top.
+ // because if a process attach to current cgroup, its cpu quota may
not be set.
+ std::string cpu_quota_path = "";
+ Status cpu_quota_ret = CGroupUtil::find_abs_cgroupv1_path("cpu",
&cpu_quota_path);
+ if (cpu_quota_ret.ok() && cpu_quota_path.size() != 0) {
Review Comment:
warning: the 'empty' method should be used to check for emptiness instead of
'size' [readability-container-size-empty]
```suggestion
if (cpu_quota_ret.ok() && !cpu_quota_path.empty()) {
```
<details>
<summary>Additional context</summary>
**/usr/include/c++/11/bits/basic_string.h:1022:** method
'basic_string'::empty() defined here
```cpp
empty() const _GLIBCXX_NOEXCEPT
^
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
