[GitHub] [arrow] pitrou commented on a diff in pull request #13516: ARROW-16981: [C++] Expose jemalloc statistics for logging

[GitBox <gi...@apache.org>]

pitrou commented on code in PR #13516:
URL: https://github.com/apache/arrow/pull/13516#discussion_r966987312


##########
cpp/src/arrow/memory_pool.h:
##########
@@ -175,6 +176,25 @@ ARROW_EXPORT Status jemalloc_memory_pool(MemoryPool** out);
 ARROW_EXPORT
 Status jemalloc_set_decay_ms(int ms);
 
+/// \brief Get basic allocation statistics from jemalloc
+/// See the MALLCTL NAMESPACE section in jemalloc project documentation for
+/// available stats.
+Status jemalloc_get_stat(const char* name, size_t& out);
+Status jemalloc_get_stat(const char* name, uint64_t& out);

Review Comment:
   As per the coding conventions, we avoid non-const references, so the output parameter should be a pointer.
   Also IMHO it doesn't make sense to expose different integer sizes, or to default to unsigned. So I would make it:
   ```c++
   Status jemalloc_get_stat(const char* name, int64_t* out);
   ```
   or perhaps even (but it's less flexible if there ever are non-integer statistics:
   ```c++
   Result<int64_t> jemalloc_get_stat(const char* name);
   ```
   



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org