pitrou commented on code in PR #12702:
URL: https://github.com/apache/arrow/pull/12702#discussion_r853881766
##########
cpp/src/arrow/util/io_util.cc:
##########
@@ -1867,5 +1879,45 @@ uint64_t GetOptionalThreadId() {
return (tid == 0) ? tid - 1 : tid;
}
+// Returns the current resident set size (physical memory use) measured
+// in bytes, or zero if the value cannot be determined on this OS.
+int64_t GetCurrentRSS() {
+#if defined(_WIN32)
+ // Windows --------------------------------------------------
+ PROCESS_MEMORY_COUNTERS info;
+ GetProcessMemoryInfo(GetCurrentProcess(), &info, sizeof(info));
+ return static_cast<int64_t>(info.WorkingSetSize);
+
+#elif defined(__APPLE__)
+ // OSX ------------------------------------------------------
+ struct mach_task_basic_info info;
+ mach_msg_type_number_t infoCount = MACH_TASK_BASIC_INFO_COUNT;
+ if (task_info(mach_task_self(), MACH_TASK_BASIC_INFO, (task_info_t)&info,
&infoCount) !=
+ KERN_SUCCESS) {
+ ARROW_LOG(WARNING) << "Can't resolve RSS value";
+ return static_cast<int64_t>(0L);
+ }
+ return static_cast<int64_t>(info.resident_size);
+
+#elif defined(__linux__) || defined(__linux) || defined(linux) ||
defined(__gnu_linux__)
+ // Linux ----------------------------------------------------
+ int64_t rss = 0L;
+
+ std::ifstream fp("/proc/self/statm");
+ if (fp.is_open()) {
+ fp >> rss;
+ } else {
+ ARROW_LOG(WARNING) << "Can't resolve RSS value from /proc/self/statm";
+ return static_cast<int64_t>(0L);
+ }
+ if (fp.is_open()) fp.close();
+ return static_cast<int64_t>(rss) *
static_cast<int64_t>(sysconf(_SC_PAGESIZE));
Review Comment:
Simplify this code a bit, also the `ifstream` destructor should close the
file for you:
```suggestion
if (fp) {
fp >> rss;
return rss * sysconf(_SC_PAGESIZE);
} else {
ARROW_LOG(WARNING) << "Can't resolve RSS value from /proc/self/statm";
return 0;
}
```
##########
cpp/src/arrow/util/io_util.cc:
##########
@@ -1867,5 +1879,45 @@ uint64_t GetOptionalThreadId() {
return (tid == 0) ? tid - 1 : tid;
}
+// Returns the current resident set size (physical memory use) measured
+// in bytes, or zero if the value cannot be determined on this OS.
+int64_t GetCurrentRSS() {
+#if defined(_WIN32)
+ // Windows --------------------------------------------------
+ PROCESS_MEMORY_COUNTERS info;
+ GetProcessMemoryInfo(GetCurrentProcess(), &info, sizeof(info));
+ return static_cast<int64_t>(info.WorkingSetSize);
+
+#elif defined(__APPLE__)
+ // OSX ------------------------------------------------------
+ struct mach_task_basic_info info;
+ mach_msg_type_number_t infoCount = MACH_TASK_BASIC_INFO_COUNT;
+ if (task_info(mach_task_self(), MACH_TASK_BASIC_INFO, (task_info_t)&info,
&infoCount) !=
+ KERN_SUCCESS) {
+ ARROW_LOG(WARNING) << "Can't resolve RSS value";
+ return static_cast<int64_t>(0L);
Review Comment:
Nit: no need for an explicit cast
```suggestion
return 0;
```
##########
cpp/src/arrow/util/io_util.cc:
##########
@@ -101,6 +101,18 @@
#include "arrow/util/utf8.h"
#endif
+#ifdef _WIN32
+#include <psapi.h>
+#include <windows.h>
+
+#elif __APPLE__
+#include <mach/mach.h>
+
+#elif __linux__ || __linux || linux || __gnu_linux__
Review Comment:
AFAIK, `__linux__` is sufficient.
##########
cpp/src/arrow/util/tracing_internal.h:
##########
@@ -114,23 +117,38 @@ class SpanImpl {
opentelemetry::trace::StartSpanOptions SpanOptionsWithParent(
const util::tracing::Span& parent_span);
-#define START_SPAN(target_span, ...)
\
- auto opentelemetry_scope##__LINE__ =
\
- ::arrow::internal::tracing::GetTracer()->WithActiveSpan(
\
- target_span
\
- .Set(::arrow::util::tracing::Span::Impl{
\
-
::arrow::internal::tracing::GetTracer()->StartSpan(__VA_ARGS__)}) \
- .span)
+#define FIRST_ARG(first, ...) first
+
+#define START_COMPUTE_SPAN(target_span, ...) \
+ START_SPAN(target_span, __VA_ARGS__); \
+ START_SPAN(target_span, FIRST_ARG(__VA_ARGS__), {GET_MEMORY_POOL_INFO});
Review Comment:
I don't really understand why this starting two spans (or starting the same
span twice)? @lidavidm Does this look right?
##########
cpp/src/arrow/util/tracing_internal.h:
##########
@@ -114,23 +117,38 @@ class SpanImpl {
opentelemetry::trace::StartSpanOptions SpanOptionsWithParent(
const util::tracing::Span& parent_span);
-#define START_SPAN(target_span, ...)
\
- auto opentelemetry_scope##__LINE__ =
\
- ::arrow::internal::tracing::GetTracer()->WithActiveSpan(
\
- target_span
\
- .Set(::arrow::util::tracing::Span::Impl{
\
-
::arrow::internal::tracing::GetTracer()->StartSpan(__VA_ARGS__)}) \
- .span)
+#define FIRST_ARG(first, ...) first
+
+#define START_COMPUTE_SPAN(target_span, ...) \
+ START_SPAN(target_span, __VA_ARGS__); \
+ START_SPAN(target_span, FIRST_ARG(__VA_ARGS__), {GET_MEMORY_POOL_INFO});
+
+#define START_COMPUTE_SPAN_WITH_PARENT(target_span, parent_span, ...) \
+ START_SPAN_WITH_PARENT(target_span, parent_span, __VA_ARGS__); \
+ START_SPAN_WITH_PARENT(target_span, parent_span, FIRST_ARG(__VA_ARGS__), \
+ {GET_MEMORY_POOL_INFO});
+
+#define START_SPAN(target_span, ...)
\
+ {
\
+ auto opentelemetry_scope##__LINE__ =
\
+ ::arrow::internal::tracing::GetTracer()->WithActiveSpan(
\
+ target_span
\
+ .Set(::arrow::util::tracing::Span::Impl{
\
+
::arrow::internal::tracing::GetTracer()->StartSpan(__VA_ARGS__)}) \
+ .span);
\
+ }
Review Comment:
I'm not sure how those work in OpenTelemetry, but this seems to close the
scope at the end of the macro, is this right? @lidavidm
##########
cpp/src/arrow/util/io_util.cc:
##########
@@ -1867,5 +1879,45 @@ uint64_t GetOptionalThreadId() {
return (tid == 0) ? tid - 1 : tid;
}
+// Returns the current resident set size (physical memory use) measured
+// in bytes, or zero if the value cannot be determined on this OS.
+int64_t GetCurrentRSS() {
+#if defined(_WIN32)
+ // Windows --------------------------------------------------
+ PROCESS_MEMORY_COUNTERS info;
+ GetProcessMemoryInfo(GetCurrentProcess(), &info, sizeof(info));
+ return static_cast<int64_t>(info.WorkingSetSize);
+
+#elif defined(__APPLE__)
+ // OSX ------------------------------------------------------
+ struct mach_task_basic_info info;
+ mach_msg_type_number_t infoCount = MACH_TASK_BASIC_INFO_COUNT;
+ if (task_info(mach_task_self(), MACH_TASK_BASIC_INFO, (task_info_t)&info,
&infoCount) !=
+ KERN_SUCCESS) {
+ ARROW_LOG(WARNING) << "Can't resolve RSS value";
+ return static_cast<int64_t>(0L);
+ }
+ return static_cast<int64_t>(info.resident_size);
+
+#elif defined(__linux__) || defined(__linux) || defined(linux) ||
defined(__gnu_linux__)
+ // Linux ----------------------------------------------------
+ int64_t rss = 0L;
+
+ std::ifstream fp("/proc/self/statm");
+ if (fp.is_open()) {
+ fp >> rss;
+ } else {
+ ARROW_LOG(WARNING) << "Can't resolve RSS value from /proc/self/statm";
+ return static_cast<int64_t>(0L);
+ }
+ if (fp.is_open()) fp.close();
+ return static_cast<int64_t>(rss) *
static_cast<int64_t>(sysconf(_SC_PAGESIZE));
+
+#else
+ // AIX, BSD, Solaris, and Unknown OS ------------------------
+ return static_cast<int64_t>(0L); // Unsupported.
Review Comment:
Nit: no need for an explicit cast.
```suggestion
return 0; // Unsupported.
```
--
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]
