----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63368/#review201184 -----------------------------------------------------------
3rdparty/libprocess/src/memory_profiler.cpp Lines 286 (patched) <https://reviews.apache.org/r/63368/#comment282300> Can we add a subfolder with current process pid to address scenarios where multiple libprocess processes are profiled on the same machine? 3rdparty/libprocess/src/memory_profiler.cpp Lines 319 (patched) <https://reviews.apache.org/r/63368/#comment282273> Space between `>%` please 3rdparty/libprocess/src/memory_profiler.cpp Lines 325-328 (patched) <https://reviews.apache.org/r/63368/#comment282290> This can also happen if there are no useful data in the profile. Is it what you mean by "empty"? 3rdparty/libprocess/src/memory_profiler.cpp Lines 326 (patched) <https://reviews.apache.org/r/63368/#comment282271> Add period after error. 3rdparty/libprocess/src/memory_profiler.cpp Lines 667-672 (patched) <https://reviews.apache.org/r/63368/#comment282294> Current logic allows users to download profiles from the _previous_ run while the _current_ run is active. Is it done on purpose? I can imagine users expecting download to return intermediate results of the _current_ run, hence I'd rather either clean the state before starting a new run or disallow implicit, i.e., without id, download request if a profiling is running. 3rdparty/libprocess/src/memory_profiler.cpp Lines 983 (patched) <https://reviews.apache.org/r/63368/#comment282274> Should not it be `jemallocRawProfile.path()`? 3rdparty/libprocess/src/memory_profiler.cpp Lines 1026 (patched) <https://reviews.apache.org/r/63368/#comment282292> Can you please use the jsonify library? Example: https://github.com/apache/mesos/blob/bd688e4cf9f6f35c9d75aad50b99e1fdeb8104da/src/master/http.cpp#L1541-L1585 3rdparty/libprocess/src/memory_profiler.cpp Lines 1031-1032 (patched) <https://reviews.apache.org/r/63368/#comment282282> Let's print the directory where profiles are stored as well. 3rdparty/libprocess/src/memory_profiler.cpp Lines 1075-1077 (patched) <https://reviews.apache.org/r/63368/#comment282284> s/build/build_options - Alexander Rukletsov On April 10, 2018, 2:39 p.m., Benno Evers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63368/ > ----------------------------------------------------------- > > (Updated April 10, 2018, 2:39 p.m.) > > > Review request for mesos, Alexander Rukletsov and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > This class exposes profiling functionality of jemalloc memory allocator > when it is detected to be the memory allocator of the current process. > > In particular, it gives developers an easy method to collect and > access heap profiles which report which pieces of code were > responsible for allocating memory. > > > Diffs > ----- > > 3rdparty/libprocess/Makefile.am c895d3ac7b9cc5ffd6c8b57ff24def866eb0213d > 3rdparty/libprocess/include/process/process.hpp > c36df991b6a2c120ab0562e8ff907f9fbf8630d1 > 3rdparty/libprocess/src/CMakeLists.txt > 0ce7dac5deea94623530820d173ce3ffe5b42ea4 > 3rdparty/libprocess/src/memory_profiler.hpp PRE-CREATION > 3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION > 3rdparty/libprocess/src/process.cpp > 9eb37465cd86f408d69f5f98fb76c4f4b93b9acd > CHANGELOG c02d56d6612c432bb079a15fde84cb30d7455971 > > > Diff: https://reviews.apache.org/r/63368/diff/10/ > > > Testing > ------- > > > Thanks, > > Benno Evers > >
