mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 63368: Added MemoryProfiler class to Libprocess.
Date Tue, 31 Oct 2017 01:53:50 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review189680
-----------------------------------------------------------



Great to see this coming together! I did a quick high level pass, ran out of time, but here
are the comments so far:


3rdparty/libprocess/include/process/memory_profiler.hpp
Lines 26 (patched)
<https://reviews.apache.org/r/63368/#comment266893>

    I realize you're probably copying the style of the cpu profiler, but I added that long
ago and would instead write it using a Process wrapper now, so just the MemoryProfiler in
the header and the MemoryProfilerProcess in the .cpp.
    
    Also, some documenation in the header file would be great! In particular, what this is
(some endpoints that provide an API for memory profiling using jemalloc?), and also some of
the cross-function semantics, like lifecycle (start -> get profile -> get statistics
-> ... -> stop).



3rdparty/libprocess/include/process/memory_profiler.hpp
Lines 49-54 (patched)
<https://reviews.apache.org/r/63368/#comment266895>

    Does this drop down a file? Is it possible to avoid that? Who will clean up this file?



3rdparty/libprocess/include/process/memory_profiler.hpp
Lines 56-62 (patched)
<https://reviews.apache.org/r/63368/#comment266896>

    When I read 'dump' I think it will write some things to output or files, not returning
an object that contains data (maybe this is just me?). Ditto above.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 287-320 (patched)
<https://reviews.apache.org/r/63368/#comment266897>

    Would love to see some more comprehensive help documentation:
    
    -Are these all GET requests with no parameters?
    -Can we say what is returned (json? text? some image format?) and show some example responses
for those that return json or text?


- Benjamin Mahler


On Oct. 27, 2017, 7:04 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Oct. 27, 2017, 7:04 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 03a0ca87f31744c716c99e05aa07242fed480675 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp dc3375ce62556322eb2bc60ade61f313ade123b8

>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 71ae7129ffbd0e22eda2863b17bbcf588298c37b 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


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