You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rukletsov <ru...@gmail.com> on 2017/11/10 16:31:04 UTC
Re: Review Request 63368: Added MemoryProfiler class to Libprocess.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review190728
-----------------------------------------------------------
3rdparty/libprocess/include/process/memory_profiler.hpp
Lines 22 (patched)
<https://reviews.apache.org/r/63368/#comment268331>
System headers first please
3rdparty/libprocess/src/memory_profiler.cpp
Lines 44-52 (patched)
<https://reviews.apache.org/r/63368/#comment268334>
My gut feeling is that this will not fly on Windows. Is the plan to exclude this file from compilation with msvc?
3rdparty/libprocess/src/memory_profiler.cpp
Lines 287-296 (patched)
<https://reviews.apache.org/r/63368/#comment268335>
Looks like these guys did not make it into MVP.
3rdparty/libprocess/src/memory_profiler.cpp
Lines 516-519 (patched)
<https://reviews.apache.org/r/63368/#comment268336>
I remember there was something about caching in hpp...?
3rdparty/libprocess/src/memory_profiler.cpp
Lines 527 (patched)
<https://reviews.apache.org/r/63368/#comment268337>
Are we aure `jeprof` is available? Or we don't care?
3rdparty/libprocess/src/memory_profiler.cpp
Lines 531 (patched)
<https://reviews.apache.org/r/63368/#comment268338>
Also "if.isElse" : ). Let's s/success/schell/
- Alexander Rukletsov
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
>
>
Re: Review Request 63368: Added MemoryProfiler class to Libprocess.
Posted by Benno Evers <be...@mesosphere.com>.
> On Nov. 10, 2017, 4:31 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 44-52 (patched)
> > <https://reviews.apache.org/r/63368/diff/1/?file=1870338#file1870338line44>
> >
> > My gut feeling is that this will not fly on Windows. Is the plan to exclude this file from compilation with msvc?
If this cannot be made to work then yes, but I think weak attributes should be supported on windows, maybe with some additional define for the syntax.
> On Nov. 10, 2017, 4:31 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 527 (patched)
> > <https://reviews.apache.org/r/63368/diff/1/?file=1870338#file1870338line527>
> >
> > Are we aure `jeprof` is available? Or we don't care?
We don't care: It is documented that jeprof needs to be in the PATH for this endpoint to work, and if it isn't then we will return an error that says 'jeprof: command not found'.
- Benno
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review190728
-----------------------------------------------------------
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 ba9e78148932304ce1961b88e5f97180abd35586
> 3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION
> 3rdparty/libprocess/include/process/process.hpp 8b8323f5fb4617aa9c22bbdb56af3d1cb0b06ea3
> 3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION
> 3rdparty/libprocess/src/process.cpp 2126c272a69bfa53b4b3cbbb1a55a3f81a5da8ad
>
>
> Diff: https://reviews.apache.org/r/63368/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 63368: Added MemoryProfiler class to Libprocess.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Nov. 10, 2017, 4:31 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 287-296 (patched)
> > <https://reviews.apache.org/r/63368/diff/1/?file=1870338#file1870338line287>
> >
> > Looks like these guys did not make it into MVP.
Ah, now I see them in the next review
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review190728
-----------------------------------------------------------
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
>
>