mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Rukletsov <ruklet...@gmail.com>
Subject Re: Review Request 63368: Added MemoryProfiler class to Libprocess.
Date Mon, 16 Apr 2018 10:32:04 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message