Copilot commented on code in PR #3602:
URL: https://github.com/apache/brpc/pull/3602#discussion_r4267441486
##########
src/bvar/default_variables.cpp:
##########
@@ -107,25 +111,76 @@ static bool read_proc_status(ProcStat &stat) {
}
return true;
#elif defined(OS_MACOSX)
- // TODO(zhujiashun): get remaining state in MacOS.
- memset(&stat, 0, sizeof(stat));
- static pid_t pid = getpid();
- std::ostringstream oss;
- char cmdbuf[128];
- snprintf(cmdbuf, sizeof(cmdbuf),
- "ps -p %ld -o pid,ppid,pgid,sess"
- ",tpgid,flags,pri,nice | tail -n1", (long)pid);
- if (butil::read_command_output(oss, cmdbuf) != 0) {
- LOG(ERROR) << "Fail to read stat";
- return false;
+ const pid_t pid = getpid();
+ stat.pid = pid;
+ stat.ppid = getppid();
+ stat.pgrp = getpgrp();
+ const pid_t session = getsid(0);
+ stat.session = session >= 0 ? session : 0;
+ errno = 0;
+ const int nice = getpriority(PRIO_PROCESS, 0);
+ if (errno == 0)
+ stat.nice = nice;
+ struct proc_bsdinfo bsd = {};
+ if (proc_pidinfo(pid, PROC_PIDTBSDINFO, 0, &bsd, sizeof(bsd)) ==
+ sizeof(bsd)) {
+ stat.flags = bsd.pbi_flags;
+ stat.nice = bsd.pbi_nice;
+ stat.tty_nr = static_cast<int>(bsd.e_tdev);
+ stat.tpgid = static_cast<int>(bsd.e_tpgid);
+
+ // BSD process states are coarser than Linux's task states.
+ switch (bsd.pbi_status) {
+ case SIDL:
+ stat.state = 'I';
+ break;
+ case SRUN:
+ stat.state = 'R';
+ break;
+ case SSLEEP:
+ stat.state = 'S';
+ break;
+ case SSTOP:
+ stat.state = 'T';
+ break;
+ case SZOMB:
+ stat.state = 'Z';
+ break;
+ default:
+ stat.state = '?';
+ break;
+ }
+ }
+ struct proc_taskinfo task = {};
+ if (proc_pidinfo(pid, PROC_PIDTASKINFO, 0, &task, sizeof(task)) ==
+ sizeof(task)) {
+ stat.priority = task.pti_priority;
+ stat.num_threads = task.pti_threadnum;
+ }
+
+ // ProcStat follows Linux /proc/self/stat, where utime, stime, cutime,
+ // and cstime are measured in clock ticks.
+ static const long ticks_per_second = sysconf(_SC_CLK_TCK);
+ const auto to_ticks = [](const timeval &time) -> unsigned long {
+ if (ticks_per_second <= 0) {
+ return 0;
+ }
+ return static_cast<uint64_t>(time.tv_sec) * ticks_per_second +
+ static_cast<uint64_t>(time.tv_usec) * ticks_per_second / 1000000;
+ };
Review Comment:
This lambda does not capture `ticks_per_second`, so it won’t compile (use of
an enclosing-scope variable without capture). Capture it (e.g.,
`[ticks_per_second]`) or turn `to_ticks` into a regular function that takes
`ticks_per_second` as a parameter.
##########
src/bvar/default_variables.cpp:
##########
@@ -350,29 +401,24 @@ static int get_fd_count(int limit) {
for (; dr.Next() && count <= limit + 3; ++count) {}
return count - 3 /* skipped ., .. and the fd in dr*/;
#elif defined(OS_MACOSX)
- // TODO(zhujiashun): following code will cause core dump with some
- // probability under mac when program exits. Fix it.
- /*
- static pid_t pid = getpid();
- std::ostringstream oss;
- char cmdbuf[128];
- snprintf(cmdbuf, sizeof(cmdbuf),
- "lsof -p %ld | grep -v \"txt\" | wc -l", (long)pid);
- if (butil::read_command_output(oss, cmdbuf) != 0) {
- LOG(ERROR) << "Fail to read open files";
+ if (limit <= 0) {
+ return 0;
+ }
+ std::vector<proc_fdinfo> fds(limit);
+ errno = 0;
+ // This query opens no additional FD.
+ const int bytes = proc_pidinfo(getpid(), PROC_PIDLISTFDS, 0,
+ fds.data(), fds.size() *
sizeof(proc_fdinfo));
Review Comment:
This allocates a `std::vector` on every call, which can add avoidable
CPU/memory churn if FD metrics are sampled frequently. Consider reusing a
buffer (e.g., `thread_local std::vector<proc_fdinfo>` resized to `limit`, or a
helper that caches the allocation) so repeated sampling doesn’t repeatedly
allocate/free.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
