You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benno Evers <be...@mesosphere.com> on 2017/10/27 19:04:03 UTC

Review Request 63368: Added MemoryProfiler class to Libprocess.

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

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 Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 243-249 (patched)
> > <https://reviews.apache.org/r/63368/diff/3/?file=1951296#file1951296line243>
> >
> >     I think it is safe to use this function because it is only accessed from `MemoryProfiler::DiskArtifact::path()` which is in turn only accessed from `MemoryProfiler` methods, which are always serialized. You should call this in a comment or even employ `process::Once` now to make sure it is thread-safe and hence future-proof.

I will add a comment. I think using `Once` can be counter-productive, because the serialization provided by `libprocess` is fundamental enough that most likely everything would break if it wouldn't exist - leading the reader to wonder why it isn't enough in this function so that `Once` needs to be used.


- Benno


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


On Feb. 6, 2018, 5:45 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2018, 5:45 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 3071b7ce2b82a4ce0ea62e4d2b3518a6f5114803 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt f002c157dc2ca64da66bc4e61f5095f2b533ae1f 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp ba9bc291bb6741e32b3a066fe90771311d21852a 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Benno Evers <be...@mesosphere.com>.

> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 197-240 (patched)
> > <https://reviews.apache.org/r/63368/diff/3/?file=1951296#file1951296line197>
> >
> >     I would still merge these two functions. Since it is for internal use only, it is fine to expect the user to understand when to expect a previous value (looks like there is one such instance in the current code).
> >     
> >     How about this?
> >     ```
> >     template<typename T>
> >     Result<T> writeJemallocSetting(const char* name, const T& value, bool requestPrevious)
> >     {
> >       if (!detectJemalloc()) {
> >         return Error(JEMALLOC_NOT_DETECTED_MESSAGE);
> >       }
> >     
> >       T previous;
> >       T* pprevious = requestPrevious ? &previous : nullptr;
> >       size_t size = sizeof(previous);
> >       size_t* pszie = requestPrevious ? &size : nullptr;
> >       
> >       int error = mallctl(
> >           name, pprevious, psize, const_cast<T*>(&value), sizeof(value));
> >     
> >       if (error) {
> >         return Error(strings::format(
> >             "Couldn't write value %s for option %s: %s",
> >             value, name, ::strerror(error)).get());
> >       }
> >     
> >       return requestPrevious ? previous : None();
> >     }
> >     ```
> >     
> >     And if you make `value` optional, you can have a single function plus maybe three syntactic sugar functions with descriptive names that call into the basic one.

To be honest this looks quite overloaded to me. If you insist I will change it, but I think two simple functions are better than one complicted.


> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 264 (patched)
> > <https://reviews.apache.org/r/63368/diff/3/?file=1951296#file1951296line264>
> >
> >     Say "using std::string" in the beginning of the file and save hundreds of characters!
> 
> Benno Evers wrote:
>     I'm not a fan of using `using`-directives just to save some typing, as it puts a large cognitive burden on the reader: Is `string` referring to name imported to the global namespace via `using`, a class-local or a function-local typedef, maybe defined in some `stout`-header? In any case, one has to jump to a different context to double-check. Using `std::string`, on the other hand, is not ambigous.
> 
> Alexander Rukletsov wrote:
>     ```
>     alex@alexr: ~/Projects/mesos $ grep -r -i --include *.cpp "std::string" ./src | wc -l      
>          465
>     
>     alex@alexr: ~/Projects/mesos $ grep -r -i --include *.cpp "using std::string" ./src | wc -l
>          371
>     ```
>     We have ~370 .cpp files with `using std::string` and \<100 mentions of `std::string`. We also do not provide our own string type; ambiguous local typedefs shall not be used.
>     
>     In addition to the above, I personally find meaningless prefixes and characters, like `std::`, contributing to the cognitive load of the code.

> We also do not provide our own string type; ambiguous local typedefs shall not be used.

You may know this, but I don't think you can assume every reader of the code does. Also, the second rule may be followed `de-facto`, but I don't think it's written down anywhere.


- Benno


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


On Feb. 6, 2018, 5:45 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2018, 5:45 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 3071b7ce2b82a4ce0ea62e4d2b3518a6f5114803 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt f002c157dc2ca64da66bc4e61f5095f2b533ae1f 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/5/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Benno Evers <be...@mesosphere.com>.

> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 630 (patched)
> > <https://reviews.apache.org/r/63368/diff/3/?file=1951296#file1951296line630>
> >
> >     Use `profilingTimer->timeout().remaining()` directly.

There are two issues with that:
1) The redirectTime is currently selected to be 2 seconds shorter than the time remaining in the profiling timer, so by default it is the redirect that is stopping the profiling
2) Aesthetically, if a user is selecting `duration=5secs` it seems odd to display `You will be redirected in 4.99993 seconds`.


> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 650-653 (patched)
> > <https://reviews.apache.org/r/63368/diff/3/?file=1951296#file1951296line650>
> >
> >     Why do you need to check if you can read the key? Can this be omitted or maybe moved to the bottom of the function?

The key must be read *before* we stop profiling, since we are interested in the previous value of this setting. This should realistically never fail, so if it does the circumstances are so weird that we probably shouldn't continue working with this jemalloc (e.g. the user is using a custom-compiled version where this setting was removed/renamed)


> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 680 (patched)
> > <https://reviews.apache.org/r/63368/diff/3/?file=1951296#file1951296line680>
> >
> >     I think the right criteria here is (was_active && not_active_now). If you want to fo with a single read of a jemalloc setting, then still use the value right after obtaining it.

We only get here if `stopAndGenerateRawProfile()` did return success, so `not_active_now` should always be true.


> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 897-903 (patched)
> > <https://reviews.apache.org/r/63368/diff/3/?file=1951296#file1951296line897>
> >
> >     Maybe this instead?
> >     ```
> >       // State unrelated to jemalloc.
> >       JSON::Object state;
> >       {
> >         JSON::Object profilerState;
> >         profilerState.values["jemalloc_detected"] = detected;
> >         profilerState.values["profiling_active"] = heapProfilingActive;
> >     
> >         state.values["memory_profiler"] = std::move(profilerState);
> >       }
> >     ```

I've rearranged it, but I'm not entirely sure about this because this pattern starts nesting in the later parts of this function.


- Benno


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


On Feb. 6, 2018, 5:45 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2018, 5:45 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 3071b7ce2b82a4ce0ea62e4d2b3518a6f5114803 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt f002c157dc2ca64da66bc4e61f5095f2b533ae1f 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp ba9bc291bb6741e32b3a066fe90771311d21852a 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 243-249 (patched)
> > <https://reviews.apache.org/r/63368/diff/3/?file=1951296#file1951296line243>
> >
> >     I think it is safe to use this function because it is only accessed from `MemoryProfiler::DiskArtifact::path()` which is in turn only accessed from `MemoryProfiler` methods, which are always serialized. You should call this in a comment or even employ `process::Once` now to make sure it is thread-safe and hence future-proof.
> 
> Benno Evers wrote:
>     I will add a comment. I think using `Once` can be counter-productive, because the serialization provided by `libprocess` is fundamental enough that most likely everything would break if it wouldn't exist - leading the reader to wonder why it isn't enough in this function so that `Once` needs to be used.

The reason I suggested to use `Once` is because this is a free function and it is not immediately clear that it is called only from a single actor instance.


> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 264 (patched)
> > <https://reviews.apache.org/r/63368/diff/3/?file=1951296#file1951296line264>
> >
> >     Say "using std::string" in the beginning of the file and save hundreds of characters!
> 
> Benno Evers wrote:
>     I'm not a fan of using `using`-directives just to save some typing, as it puts a large cognitive burden on the reader: Is `string` referring to name imported to the global namespace via `using`, a class-local or a function-local typedef, maybe defined in some `stout`-header? In any case, one has to jump to a different context to double-check. Using `std::string`, on the other hand, is not ambigous.

```
alex@alexr: ~/Projects/mesos $ grep -r -i --include *.cpp "std::string" ./src | wc -l      
     465

alex@alexr: ~/Projects/mesos $ grep -r -i --include *.cpp "using std::string" ./src | wc -l
     371
```
We have ~370 .cpp files with `using std::string` and \<100 mentions of `std::string`. We also do not provide our own string type; ambiguous local typedefs shall not be used.

In addition to the above, I personally find meaningless prefixes and characters, like `std::`, contributing to the cognitive load of the code.


> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 630 (patched)
> > <https://reviews.apache.org/r/63368/diff/3/?file=1951296#file1951296line630>
> >
> >     Use `profilingTimer->timeout().remaining()` directly.
> 
> Benno Evers wrote:
>     There are two issues with that:
>     1) The redirectTime is currently selected to be 2 seconds shorter than the time remaining in the profiling timer, so by default it is the redirect that is stopping the profiling
>     2) Aesthetically, if a user is selecting `duration=5secs` it seems odd to display `You will be redirected in 4.99993 seconds`.

Re 2): You will have this case anyway if profiling is in progress when the request arrives; you can also round it up.

Re 1): I'm not sure I understand why you use this 2s delay.


- Alexander


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


On Feb. 6, 2018, 5:45 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2018, 5:45 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 3071b7ce2b82a4ce0ea62e4d2b3518a6f5114803 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt f002c157dc2ca64da66bc4e61f5095f2b533ae1f 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp ba9bc291bb6741e32b3a066fe90771311d21852a 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Benno Evers <be...@mesosphere.com>.

> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 264 (patched)
> > <https://reviews.apache.org/r/63368/diff/3/?file=1951296#file1951296line264>
> >
> >     Say "using std::string" in the beginning of the file and save hundreds of characters!

I'm not a fan of using `using`-directives just to save some typing, as it puts a large cognitive burden on the reader: Is `string` referring to name imported to the global namespace via `using`, a class-local or a function-local typedef, maybe defined in some `stout`-header? In any case, one has to jump to a different context to double-check. Using `std::string`, on the other hand, is not ambigous.


- Benno


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


On Feb. 6, 2018, 5:45 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2018, 5:45 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 3071b7ce2b82a4ce0ea62e4d2b3518a6f5114803 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt f002c157dc2ca64da66bc4e61f5095f2b533ae1f 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp ba9bc291bb6741e32b3a066fe90771311d21852a 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review196894
-----------------------------------------------------------




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

    When describing what a function does or returns, we use 3rd person present simple tense: activates, returns, generates,... Please adjust here and everywhere.



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

    Why? What is this id about?
    
    Also please mention that `id` is optional and the last know will be returned if `id` is omitted.



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

    Period at the end.



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

    FYI: Fits one line. However, the comment seems incorrect: this function does not return any path, rather, it is a side effect on success.



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

    If you have private fields and getters, make it a class.



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

    backtick type and variable names and put a period at the end.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 75 (patched)
<https://reviews.apache.org/r/63368/#comment276847>

    I'd kill these blank lines.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 197-240 (patched)
<https://reviews.apache.org/r/63368/#comment276855>

    I would still merge these two functions. Since it is for internal use only, it is fine to expect the user to understand when to expect a previous value (looks like there is one such instance in the current code).
    
    How about this?
    ```
    template<typename T>
    Result<T> writeJemallocSetting(const char* name, const T& value, bool requestPrevious)
    {
      if (!detectJemalloc()) {
        return Error(JEMALLOC_NOT_DETECTED_MESSAGE);
      }
    
      T previous;
      T* pprevious = requestPrevious ? &previous : nullptr;
      size_t size = sizeof(previous);
      size_t* pszie = requestPrevious ? &size : nullptr;
      
      int error = mallctl(
          name, pprevious, psize, const_cast<T*>(&value), sizeof(value));
    
      if (error) {
        return Error(strings::format(
            "Couldn't write value %s for option %s: %s",
            value, name, ::strerror(error)).get());
      }
    
      return requestPrevious ? previous : None();
    }
    ```
    
    And if you make `value` optional, you can have a single function plus maybe three syntactic sugar functions with descriptive names that call into the basic one.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 243-249 (patched)
<https://reviews.apache.org/r/63368/#comment276831>

    I think it is safe to use this function because it is only accessed from `MemoryProfiler::DiskArtifact::path()` which is in turn only accessed from `MemoryProfiler` methods, which are always serialized. You should call this in a comment or even employ `process::Once` now to make sure it is thread-safe and hence future-proof.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 264 (patched)
<https://reviews.apache.org/r/63368/#comment276819>

    Say "using std::string" in the beginning of the file and save hundreds of characters!



3rdparty/libprocess/src/memory_profiler.cpp
Lines 301 (patched)
<https://reviews.apache.org/r/63368/#comment276861>

    I'm not sure newlines in `Error` is good idea. You can easily do without.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 311 (patched)
<https://reviews.apache.org/r/63368/#comment276857>

    I'd avoid defaults in general. Given the ordering error below, I'd probably reorder the arguments and remove the default here.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 342 (patched)
<https://reviews.apache.org/r/63368/#comment276859>

    No periods in error messages, please!



3rdparty/libprocess/src/memory_profiler.cpp
Lines 381-382 (patched)
<https://reviews.apache.org/r/63368/#comment276866>

    Instead of trailing spaces, use leading spaces. This way it is easier to spot when a space is missing. Here and below.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 393-395 (patched)
<https://reviews.apache.org/r/63368/#comment276864>

    Looks like you would need a multiline message here. Use commas to separate lines. Ditto for other longer help messages you have below.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 437-470 (patched)
<https://reviews.apache.org/r/63368/#comment276867>

    Aint't it nice and tidy now?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 480 (patched)
<https://reviews.apache.org/r/63368/#comment276869>

    Who is `WARN()`? s/as WARN()/at `WARNING` level



3rdparty/libprocess/src/memory_profiler.cpp
Lines 523-524 (patched)
<https://reviews.apache.org/r/63368/#comment276832>

    Please blank comment line (`  //  `) between `TODO`s, `NOTE`s and alike.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 534 (patched)
<https://reviews.apache.org/r/63368/#comment276833>

    4 spaces indent is only after `(`. I know it's hard to follow the 2 vs. 4 indent rule, but please comply.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 544-547 (patched)
<https://reviews.apache.org/r/63368/#comment276835>

    I'm not sure I understand this part. This function is for internal use only, so why would we try to write something to the file and reuse the id? And if we do so, why is this not an error?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 578 (patched)
<https://reviews.apache.org/r/63368/#comment276870>

    Please create a default constant at the top of the file for this.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 592-598 (patched)
<https://reviews.apache.org/r/63368/#comment276872>

    Let's 
    1) Introduce constants for these values
    2) Mention them in the related help message
    3) Return `BadRequest` if duration is out of range



3rdparty/libprocess/src/memory_profiler.cpp
Lines 608 (patched)
<https://reviews.apache.org/r/63368/#comment276876>

    No need for this extra variable, see comments below.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 611 (patched)
<https://reviews.apache.org/r/63368/#comment276873>

    I doubt this is correct English. Suggestions: "... is already active", "has been already activated".



3rdparty/libprocess/src/memory_profiler.cpp
Lines 630 (patched)
<https://reviews.apache.org/r/63368/#comment276874>

    Use `profilingTimer->timeout().remaining()` directly.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 632 (patched)
<https://reviews.apache.org/r/63368/#comment276875>

    no need for this variable



3rdparty/libprocess/src/memory_profiler.cpp
Lines 633 (patched)
<https://reviews.apache.org/r/63368/#comment276878>

    `const` please.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 634 (patched)
<https://reviews.apache.org/r/63368/#comment276877>

    use `profilingTimer->timeout().remaining().secs()` directly.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 636 (patched)
<https://reviews.apache.org/r/63368/#comment276856>

    Swap the parameters ; )



3rdparty/libprocess/src/memory_profiler.cpp
Lines 650-653 (patched)
<https://reviews.apache.org/r/63368/#comment276879>

    Why do you need to check if you can read the key? Can this be omitted or maybe moved to the bottom of the function?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 664 (patched)
<https://reviews.apache.org/r/63368/#comment276880>

    No space between `[]` and `()`.
    `{` stays on the previous line.
    
    Reference: https://mesos.apache.org/documentation/latest/c++-style-guide/



3rdparty/libprocess/src/memory_profiler.cpp
Lines 680 (patched)
<https://reviews.apache.org/r/63368/#comment276881>

    I think the right criteria here is (was_active && not_active_now). If you want to fo with a single read of a jemalloc setting, then still use the value right after obtaining it.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 693 (patched)
<https://reviews.apache.org/r/63368/#comment276849>

    It looks like you don't use the previous value, so why not using `writeJemallocSetting()` instead?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 709 (patched)
<https://reviews.apache.org/r/63368/#comment276883>

    Please clarify which retries you mean. Maybe something like "If any error, e.g., writing the output file, happens after this point, it is not retried..."



3rdparty/libprocess/src/memory_profiler.cpp
Lines 722 (patched)
<https://reviews.apache.org/r/63368/#comment276886>

    const



3rdparty/libprocess/src/memory_profiler.cpp
Lines 732 (patched)
<https://reviews.apache.org/r/63368/#comment276885>

    No periods at the end : )



3rdparty/libprocess/src/memory_profiler.cpp
Lines 766 (patched)
<https://reviews.apache.org/r/63368/#comment276887>

    Let's tell the user what the available id is.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 803 (patched)
<https://reviews.apache.org/r/63368/#comment276888>

    `{` goes onto the previous line



3rdparty/libprocess/src/memory_profiler.cpp
Lines 804-806 (patched)
<https://reviews.apache.org/r/63368/#comment276890>

    Isn't it already covered with the check above?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 853-855 (patched)
<https://reviews.apache.org/r/63368/#comment276891>

    Ditto.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 879 (patched)
<https://reviews.apache.org/r/63368/#comment276818>

    `const string`



3rdparty/libprocess/src/memory_profiler.cpp
Lines 897-903 (patched)
<https://reviews.apache.org/r/63368/#comment276893>

    Period at the end.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 897-903 (patched)
<https://reviews.apache.org/r/63368/#comment276894>

    Maybe this instead?
    ```
      // State unrelated to jemalloc.
      JSON::Object state;
      {
        JSON::Object profilerState;
        profilerState.values["jemalloc_detected"] = detected;
        profilerState.values["profiling_active"] = heapProfilingActive;
    
        state.values["memory_profiler"] = std::move(profilerState);
      }
    ```



3rdparty/libprocess/src/memory_profiler.cpp
Lines 897 (patched)
<https://reviews.apache.org/r/63368/#comment276895>

    Period at the end.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 937 (patched)
<https://reviews.apache.org/r/63368/#comment276896>

    Same `{}` trick as above?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 957 (patched)
<https://reviews.apache.org/r/63368/#comment276897>

    Same `{}` trick as above?


- Alexander Rukletsov


On Feb. 6, 2018, 5:45 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2018, 5:45 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 3071b7ce2b82a4ce0ea62e4d2b3518a6f5114803 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt f002c157dc2ca64da66bc4e61f5095f2b533ae1f 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp ba9bc291bb6741e32b3a066fe90771311d21852a 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197998
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198052
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197897
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198030
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197945
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197879
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198018
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 2:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 2:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197869
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198057
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198043
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198065
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198004
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197875
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197917
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197986
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197933
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197905
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197982
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197977
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197856
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197957
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 2:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 2:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197865
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198014
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197922
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197889
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198034
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197929
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197949
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198026
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198008
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198047
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 2:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 2:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197861
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197970
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198409
-----------------------------------------------------------




3rdparty/libprocess/src/memory_profiler.cpp
Lines 305 (patched)
<https://reviews.apache.org/r/63368/#comment278541>

    This is a speculation, `result` should be informative enough. A likely reason is that `jeprof` is absent on the machine, in which case this line is confusing.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 312-322 (patched)
<https://reviews.apache.org/r/63368/#comment278542>

    Ideally we would respond with a simply formatted data, e.g., JSON, if a request comes from `curl`/`httpie` and alike, and a "proper" html if it comes from a browser.
    
    Our help endpoints try to detect `curl` and `httpie` and respond accordingly. Another approach would be to introduce a parameter to the endpoints, e.g., `format=json` that allows to control the format of the response.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 675 (patched)
<https://reviews.apache.org/r/63368/#comment278543>

    Can you please explain why you need default capture-by-reference here and below?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 689 (patched)
<https://reviews.apache.org/r/63368/#comment278539>

    s\in"\in "



3rdparty/libprocess/src/memory_profiler.cpp
Lines 689-692 (patched)
<https://reviews.apache.org/r/63368/#comment278540>

    Technically, you do not redirect to the result, but you redirect to the `stop` endpoint if the browser is used. `curl` users will have to request `stop` manually:
    ```
    alex@gru1: ~$ http 192.99.40.208:5555/memory-profiler/start?duration=1mins
    HTTP/1.1 200 OK
    Content-Length: 175
    Content-Type: text/html
    Date: Wed, 28 Feb 2018 21:32:42 GMT
    
    <html>  <head><meta http-equiv="refresh" content="60;url=./stop" /></head>  <body>Started new heap profiling run.<br/>You will be redirected to the result in60s.</body></html>
    ```


- Alexander Rukletsov


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197883
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197893
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Benno Evers <be...@mesosphere.com>.

> On April 6, 2018, 2:02 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/include/process/memory_profiler.hpp
> > Lines 104-113 (patched)
> > <https://reviews.apache.org/r/63368/diff/8/?file=1991764#file1991764line104>
> >
> >     Does this class have to be nested? How about making it `jemalloc::State` in the same file?

None of them technically *need* to be nested, but it feels a bit cleaner to not litter the `process`-namespace unnecessarily. It also seems more consistent to me to treat all three classes as similar as possible.


> On April 6, 2018, 2:02 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/include/process/memory_profiler.hpp
> > Lines 117-120 (patched)
> > <https://reviews.apache.org/r/63368/diff/8/?file=1991764#file1991764line117>
> >
> >     Can you please mention here that it is safe to cache `MemoryProfiler` because it is destructed on process termination? Or, if you prefer to capture a lambda, to warn that users must guarantee that it is safe to invoke the lambda at a future point of time?

I'm dropping this as I'm now explicitly passing a pointer, but the reason it was safe was actually another one: `ProfilingRun` was a member of `MemoryProfiler`, so it will be destroyed before the base class, no matter whether it's global or not.


> On April 6, 2018, 2:02 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/include/process/memory_profiler.hpp
> > Lines 120-121 (patched)
> > <https://reviews.apache.org/r/63368/diff/8/?file=1991764#file1991764line120>
> >
> >     s/class/struct/
> >     rm public:

Dropping this because it violates our style guide (https://google.github.io/styleguide/cppguide.html#Structs_vs._Classes)


> On April 6, 2018, 2:02 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/CMakeLists.txt
> > Lines 47 (patched)
> > <https://reviews.apache.org/r/63368/diff/8/?file=1991766#file1991766line47>
> >
> >     Please add *.hpp as well.

I think if we do this, it should be part of a separate patch series - right now, none of the public headers are included in the `CMakeLists.txt`.


> On April 6, 2018, 2:02 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 794 (patched)
> > <https://reviews.apache.org/r/63368/diff/8/?file=1991767#file1991767line794>
> >
> >     So if there is an error reading the jemalloc setting, the binary crashes?

No, only if the setting is read successfully and it is still active.


> On April 6, 2018, 2:02 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 797 (patched)
> > <https://reviews.apache.org/r/63368/diff/8/?file=1991767#file1991767line797>
> >
> >     If we come here via `/stop` endpoint, can the still running timer occasionally stop the next profiling run?

Urgh, I'm not sure how I did forget about this one again, especially since the original motivation for introducing `ProfilingRun` was to avoid exactly this race :D


> On April 6, 2018, 2:02 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/process.cpp
> > Lines 256 (patched)
> > <https://reviews.apache.org/r/63368/diff/8/?file=1991768#file1991768line256>
> >
> >     Let's make it `false` by default for now since it is an experimental feature, and turn it on after some time, ideally after some production testing together with feature graduation.

I'm not sure, keep in mind that the actual "feature" is hidden anyways behind the `--enable-jemalloc-allocator` configuration setting, which is off by default.

Without this, and assuming the user doesn't manually link against jemalloc, all endpoints will just return an error message saying that jemalloc couldn't be detected.


- Benno


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


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/Makefile.am cd2c3bc62df8de5b50ec2fa830b3e2634ba11e28 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp c36df991b6a2c120ab0562e8ff907f9fbf8630d1 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 9eb37465cd86f408d69f5f98fb76c4f4b93b9acd 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/9/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On April 6, 2018, 2:02 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/include/process/memory_profiler.hpp
> > Lines 104-113 (patched)
> > <https://reviews.apache.org/r/63368/diff/8/?file=1991764#file1991764line104>
> >
> >     Does this class have to be nested? How about making it `jemalloc::State` in the same file?
> 
> Benno Evers wrote:
>     None of them technically *need* to be nested, but it feels a bit cleaner to not litter the `process`-namespace unnecessarily. It also seems more consistent to me to treat all three classes as similar as possible.

Hence the suggestion to put it into the `jemalloc` namespace.


> On April 6, 2018, 2:02 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/CMakeLists.txt
> > Lines 47 (patched)
> > <https://reviews.apache.org/r/63368/diff/8/?file=1991766#file1991766line47>
> >
> >     Please add *.hpp as well.
> 
> Benno Evers wrote:
>     I think if we do this, it should be part of a separate patch series - right now, none of the public headers are included in the `CMakeLists.txt`.

I'm not sure it has to be public, but okay.


- Alexander


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


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/Makefile.am cd2c3bc62df8de5b50ec2fa830b3e2634ba11e28 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp c36df991b6a2c120ab0562e8ff907f9fbf8630d1 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 9eb37465cd86f408d69f5f98fb76c4f4b93b9acd 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/9/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review200633
-----------------------------------------------------------




3rdparty/libprocess/Makefile.am
Lines 208 (patched)
<https://reviews.apache.org/r/63368/#comment281411>

    Add *.hpp file as well please.



3rdparty/libprocess/Makefile.am
Lines 301 (patched)
<https://reviews.apache.org/r/63368/#comment281412>

    I don't see this file in this diff.



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

    also `try.hpp`, `nothing.hpp`



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

    Lbrace onto the next line please. Here and below.



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

    Does this class have to be nested? How about making it `jemalloc::State` in the same file?



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

    Can you please mention here that it is safe to cache `MemoryProfiler` because it is destructed on process termination? Or, if you prefer to capture a lambda, to warn that users must guarantee that it is safe to invoke the lambda at a future point of time?



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

    s/class/struct/
    rm public:



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

    Instead of capturing the reference, how about passing `MemoryProfiler*` into both c-tor and `extend()`? Or maybe even to pass an `onFinished` lambda into `ProfilingRun` at construction and let its user define the action?



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

    Why leading underscore?



3rdparty/libprocess/src/CMakeLists.txt
Lines 47 (patched)
<https://reviews.apache.org/r/63368/#comment281413>

    Please add *.hpp as well.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 41-45 (patched)
<https://reviews.apache.org/r/63368/#comment281428>

    Please mention the autostop and the collection time concept.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 49 (patched)
<https://reviews.apache.org/r/63368/#comment281426>

    either "is created" or "are accessed" says an ESL in me.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 53 (patched)
<https://reviews.apache.org/r/63368/#comment281427>

    Please backtick type, function, variable, etc names in comments.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 58-59 (patched)
<https://reviews.apache.org/r/63368/#comment281429>

    Why new line?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 59-63 (patched)
<https://reviews.apache.org/r/63368/#comment281430>

    Apparently, this does not work build on Mac
    ```
    alex@alexr.local: ~/Projects/mesos.build $ echo $CXX   
    ccache g++ -Qunused-arguments -fcolor-diagnostics -fvisibility-inlines-hidden -Wno-deprecated-declarations
    
    alex@alexr.local: ~/Projects/mesos.build $ g++ --version
    Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
    Apple LLVM version 9.0.0 (clang-900.0.39.2)
    ```
    Breaking build is not an option, if it is hard tofix. let's disable the code on Mac.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 96 (patched)
<https://reviews.apache.org/r/63368/#comment281432>

    Do you mean `--memory_profiling` flag of the binary or `--enable-jemalloc-allocator`? Can you please look through all comments and docs and groom the terminalogy and flag/parameter names?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 517-520 (patched)
<https://reviews.apache.org/r/63368/#comment281418>

    This is racy. The timer may elapse and try to invoke the callback, which uses a field `this->profiler` of a not yet initialized instance `this`.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 646 (patched)
<https://reviews.apache.org/r/63368/#comment281414>

    remove `principal`



3rdparty/libprocess/src/memory_profiler.cpp
Lines 690 (patched)
<https://reviews.apache.org/r/63368/#comment281434>

    Let's return `Conflict`. I doubt we should return 2** if we can't provide the user with the capability they request and will return `Bad Request` for `/stop`.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 698 (patched)
<https://reviews.apache.org/r/63368/#comment281435>

    space before After



3rdparty/libprocess/src/memory_profiler.cpp
Lines 703 (patched)
<https://reviews.apache.org/r/63368/#comment281436>

    Can you please explain here in the comment why you add `0.5`?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 773 (patched)
<https://reviews.apache.org/r/63368/#comment281437>

    ?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 794 (patched)
<https://reviews.apache.org/r/63368/#comment281438>

    So if there is an error reading the jemalloc setting, the binary crashes?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 797 (patched)
<https://reviews.apache.org/r/63368/#comment281440>

    If we come here via `/stop` endpoint, can the still running timer occasionally stop the next profiling run?



3rdparty/libprocess/src/process.cpp
Lines 256 (patched)
<https://reviews.apache.org/r/63368/#comment281425>

    Let's make it `false` by default for now since it is an experimental feature, and turn it on after some time, ideally after some production testing together with feature graduation.


- Alexander Rukletsov


On April 3, 2018, 4:29 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated April 3, 2018, 4:29 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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 9eb37465cd86f408d69f5f98fb76c4f4b93b9acd 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/8/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review201099
-----------------------------------------------------------




3rdparty/libprocess/src/memory_profiler.cpp
Lines 691-692 (original), 714-716 (patched)
<https://reviews.apache.org/r/63368/#comment282067>

    `return http::Conflict("...");`


- 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/Makefile.am cd2c3bc62df8de5b50ec2fa830b3e2634ba11e28 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp c36df991b6a2c120ab0562e8ff907f9fbf8630d1 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 9eb37465cd86f408d69f5f98fb76c4f4b93b9acd 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/9/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Benno Evers <be...@mesosphere.com>.

> On April 16, 2018, 10:32 a.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 286 (patched)
> > <https://reviews.apache.org/r/63368/diff/10/?file=1998458#file1998458line286>
> >
> >     Can we add a subfolder with current process pid to address scenarios where multiple libprocess processes are profiled on the same machine?

The `XXXXXX`-part already makes it unique per process.


> On April 16, 2018, 10:32 a.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 325-328 (patched)
> > <https://reviews.apache.org/r/63368/diff/10/?file=1998458#file1998458line325>
> >
> >     This can also happen if there are no useful data in the profile. Is it what you mean by "empty"?

Yes, that's what I meant. Reworded to "contains data".


> On April 16, 2018, 10:32 a.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 667-672 (patched)
> > <https://reviews.apache.org/r/63368/diff/10/?file=1998458#file1998458line667>
> >
> >     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.

Ok, added as a special case.


- Benno


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


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/11/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review201381
-----------------------------------------------------------


Ship it!




I will commit this now, fixing outstanding typos and style issues. I will also refactor `DiskArtifact` to be more aligned with the single version of the artifact and refactor some functions to minimize taking `Try<>`s as parameters. I think we can come up with even cleaner implementation.

- Alexander Rukletsov


On April 18, 2018, 4:15 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated April 18, 2018, 4:15 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7944
>     https://issues.apache.org/jira/browse/MESOS-7944
> 
> 
> 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 enables developers to access live
> statistics of current memory usage, and to create
> heap profiles that show where most memory is allocated.
> 
> 
> 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 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/12/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/
-----------------------------------------------------------

(Updated April 18, 2018, 4:15 a.m.)


Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Bugs: MESOS-7944
    https://issues.apache.org/jira/browse/MESOS-7944


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 enables developers to access live
statistics of current memory usage, and to create
heap profiles that show where most memory is allocated.


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 


Diff: https://reviews.apache.org/r/63368/diff/12/


Testing
-------


Thanks,

Benno Evers


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
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.


Changes
-------

Address comments.


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 (updated)
-----

  3rdparty/libprocess/Makefile.am c895d3ac7b9cc5ffd6c8b57ff24def866eb0213d 
  3rdparty/libprocess/include/Makefile.am cd2c3bc62df8de5b50ec2fa830b3e2634ba11e28 
  3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/process.hpp c36df991b6a2c120ab0562e8ff907f9fbf8630d1 
  3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
  3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp 9eb37465cd86f408d69f5f98fb76c4f4b93b9acd 


Diff: https://reviews.apache.org/r/63368/diff/9/

Changes: https://reviews.apache.org/r/63368/diff/8-9/


Testing
-------


Thanks,

Benno Evers


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/
-----------------------------------------------------------

(Updated April 3, 2018, 4:29 p.m.)


Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Changes
-------

Rebased onto latest master; worked around clang bug with this-capturing lambdas.


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 (updated)
-----

  3rdparty/libprocess/Makefile.am c895d3ac7b9cc5ffd6c8b57ff24def866eb0213d 
  3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
  3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
  3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp 9eb37465cd86f408d69f5f98fb76c4f4b93b9acd 


Diff: https://reviews.apache.org/r/63368/diff/8/

Changes: https://reviews.apache.org/r/63368/diff/7-8/


Testing
-------


Thanks,

Benno Evers


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/
-----------------------------------------------------------

(Updated March 21, 2018, 7:52 p.m.)


Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Changes
-------

Updated endpoints to return JSON.


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 (updated)
-----

  3rdparty/libprocess/Makefile.am c895d3ac7b9cc5ffd6c8b57ff24def866eb0213d 
  3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/process.hpp 95a82973644553e0a1c2bdef9121f17bf18b4fcc 
  3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
  3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 


Diff: https://reviews.apache.org/r/63368/diff/7/

Changes: https://reviews.apache.org/r/63368/diff/6-7/


Testing
-------


Thanks,

Benno Evers


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198022
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198038
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review198061
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 2:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 2:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197961
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 2:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 2:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197966
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197939
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197990
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197953
-----------------------------------------------------------



Bad review!

Error:
Circular dependency detected for review 63367. Please fix the 'depends_on' field.

- Mesos Reviewbot Windows


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/
-----------------------------------------------------------

(Updated Feb. 21, 2018, 10:41 a.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 (updated)
-----

  3rdparty/libprocess/Makefile.am c895d3ac7b9cc5ffd6c8b57ff24def866eb0213d 
  3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
  3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
  3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 


Diff: https://reviews.apache.org/r/63368/diff/6/

Changes: https://reviews.apache.org/r/63368/diff/5-6/


Testing
-------


Thanks,

Benno Evers


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Benno Evers <be...@mesosphere.com>.

> On Feb. 12, 2018, 1:13 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 706-708 (patched)
> > <https://reviews.apache.org/r/63368/diff/4/?file=1953408#file1953408line706>
> >
> >     Why this is not a simple delay? To indicate that a profiling session is running?

It would be possible to implement the same without having the `profilingTimer` (the only change being that the remaining time cannot be displayed), but binding that piece of state to a member variable seems more inuitive to me than having it implicit.


- Benno


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


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review197256
-----------------------------------------------------------




3rdparty/libprocess/src/memory_profiler.cpp
Lines 654 (patched)
<https://reviews.apache.org/r/63368/#comment277389>

    Do you need to check if the timer has been initialized?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 661 (patched)
<https://reviews.apache.org/r/63368/#comment277390>

    What if stop (for some reason) request came right after timer expiration? Do you still need to call `stopaAndGenerateRawProfile()`?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 706-708 (patched)
<https://reviews.apache.org/r/63368/#comment277391>

    Why this is not a simple delay? To indicate that a profiling session is running?


- Alexander Rukletsov


On Feb. 6, 2018, 5:45 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2018, 5:45 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 3071b7ce2b82a4ce0ea62e4d2b3518a6f5114803 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt f002c157dc2ca64da66bc4e61f5095f2b533ae1f 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp ba9bc291bb6741e32b3a066fe90771311d21852a 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/
-----------------------------------------------------------

(Updated Feb. 6, 2018, 5:45 p.m.)


Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Changes
-------

Moved changes to CMakeLists into this commit.


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 (updated)
-----

  3rdparty/libprocess/Makefile.am 3071b7ce2b82a4ce0ea62e4d2b3518a6f5114803 
  3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
  3rdparty/libprocess/src/CMakeLists.txt f002c157dc2ca64da66bc4e61f5095f2b533ae1f 
  3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp ba9bc291bb6741e32b3a066fe90771311d21852a 


Diff: https://reviews.apache.org/r/63368/diff/4/

Changes: https://reviews.apache.org/r/63368/diff/3-4/


Testing
-------


Thanks,

Benno Evers


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/
-----------------------------------------------------------

(Updated Feb. 1, 2018, 7:09 p.m.)


Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Changes
-------

Addressed comments.


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 (updated)
-----

  3rdparty/libprocess/Makefile.am 3071b7ce2b82a4ce0ea62e4d2b3518a6f5114803 
  3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
  3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp ba9bc291bb6741e32b3a066fe90771311d21852a 


Diff: https://reviews.apache.org/r/63368/diff/3/

Changes: https://reviews.apache.org/r/63368/diff/2-3/


Testing
-------


Thanks,

Benno Evers


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Benno Evers <be...@mesosphere.com>.

> On Jan. 26, 2018, 5:31 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 198-240 (patched)
> > <https://reviews.apache.org/r/63368/diff/2/?file=1945991#file1945991line198>
> >
> >     Can we combine these two with in a `Result<T> updateJemallocSetting(const char* name, const T& value)`?

Sadly, no. As the comment above `writeJemallocSetting()` is trying to say, we don't have something to pass for the second and third argument of `mallctl()`, and jemalloc will return an error if we pass in some dummy value.


- Benno


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


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>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review196340
-----------------------------------------------------------




3rdparty/libprocess/Makefile.am
Lines 213 (patched)
<https://reviews.apache.org/r/63368/#comment275910>

    Please tabs



3rdparty/libprocess/Makefile.am
Lines 303 (patched)
<https://reviews.apache.org/r/63368/#comment275911>

    Please tabs



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

    No need to use the FQN here.



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

    "How long"... what? Can you please be more verbose here?
    Also you mean request parameters, can you say this explicitly?



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

    Blank comment line in-between, please



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

    Please write a leading comment explaining what these functions return, i.e., last _finished_ state.



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

    No need for process prefix, here and below.



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

    What kind of statistics is this? Is this related to the last / active state? Or is it accumulated across all states since process launch? Can you please elaborate?



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

    Let's call this guy `stopAndGenerateRawProfile()` for clarity.



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

    ~~One shot one kill~~one line one declaration, please.
    
    Any reason to keep these fellas as `Try`s?
    
    Please also expand comments on these fields. For example, 
    ```
    // A timestamp of a last successful raw dump.
    Option<time_t> rawId;
    ```



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

    Let's make it `const`. You will likely need a static or free function that read the env var.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 52 (patched)
<https://reviews.apache.org/r/63368/#comment275959>

    Please backtick type, variable, class, and function names in comments.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 198-240 (patched)
<https://reviews.apache.org/r/63368/#comment275960>

    Can we combine these two with in a `Result<T> updateJemallocSetting(const char* name, const T& value)`?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 245 (patched)
<https://reviews.apache.org/r/63368/#comment275963>

    Here you convert `string` to `Path` in order to convert it back at a call site. I suggest to make `getRawProfilePath()` return `string.
    
    Also, there is no need in leading slashes.
    
    And since you already have a section for literals, maybe put file names there?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 261-271 (patched)
<https://reviews.apache.org/r/63368/#comment275966>

    It's not obvious that updating a setting triggers a dump. Can you please leave a note here?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 275 (patched)
<https://reviews.apache.org/r/63368/#comment275967>

    One shot one...



3rdparty/libprocess/src/memory_profiler.cpp
Lines 313 (patched)
<https://reviews.apache.org/r/63368/#comment275968>

    Why newline?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 326-330 (patched)
<https://reviews.apache.org/r/63368/#comment275971>

    Formatting.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 334 (patched)
<https://reviews.apache.org/r/63368/#comment275970>

    Let's make it a lambda in `start()`



3rdparty/libprocess/src/memory_profiler.cpp
Lines 347 (patched)
<https://reviews.apache.org/r/63368/#comment275969>

    Let's make it a lambda in `stop()`.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 355-357 (patched)
<https://reviews.apache.org/r/63368/#comment275972>

    For clarity, let's instead make it 
    ```
    Option<time_t> extractIdFromRequest(const process::http::Request& request)
    ```
    Then at the call site you can say:
    `extractIdFromRequest(r).getOrElse(fallback.getOrElse(0))`



3rdparty/libprocess/src/memory_profiler.cpp
Lines 361 (patched)
<https://reviews.apache.org/r/63368/#comment275974>

    `strtol`?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 372 (patched)
<https://reviews.apache.org/r/63368/#comment275975>

    Consider using an alias for `process::http` in this file.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 382 (patched)
<https://reviews.apache.org/r/63368/#comment275976>

    kill "end"



3rdparty/libprocess/src/memory_profiler.cpp
Lines 387 (patched)
<https://reviews.apache.org/r/63368/#comment275958>

    This looks like a general pattern we use in libprocess. Can we add an overload for `route()` and fix other places? Somehting like this:
    ```
    template <typename T>
    void route(
        const std::string& name,
        const Option<std::string>& realm,
        const Option<std::string>& help,
        Future<http::Response> (T::*method)(
            const http::Request&,
            const Option<http::authentication::Principal>&),
        const RouteOptions& options = RouteOptions())
    {
      if (realm.isSome()) {
        route(name, realm.get(), help, handler, options);
      } else {
        // Note that we use dynamic_cast here so a process can use
        // multiple inheritance if it sees so fit (e.g., to implement
        // multiple callback interfaces).
        HttpRequestHandler handler =
          lambda::bind(method, dynamic_cast<T*>(this), lambda::_1, None());
        route(name, help, handler, options);
      }
    }
    ```



3rdparty/libprocess/src/memory_profiler.cpp
Lines 395-398 (patched)
<https://reviews.apache.org/r/63368/#comment275957>

    I understand that you try to save some characters, but I'd argue and explicit `HELP` declaration is superior. Moreover, it is consistent to the rest of the codebase. Please define `static const std::string *_HELP();` functions



3rdparty/libprocess/src/memory_profiler.cpp
Lines 686-690 (patched)
<https://reviews.apache.org/r/63368/#comment275981>

    This looks to me that a situation when `rawId` is reset but the old file exists is possible. It is probably fine for now, but deserves a comment.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 714 (patched)
<https://reviews.apache.org/r/63368/#comment275982>

    Don't you need to check `graphId.isSome()`?



3rdparty/libprocess/src/memory_profiler.cpp
Lines 807 (patched)
<https://reviews.apache.org/r/63368/#comment275983>

    Do you check that the file actually exists before streaming it to the user? I don't see it neither here, nor in `generateGraph()`. You probably want to put such check into `generateGraph()` and reset `graphId` in case file is gone or can't be accessed.


- 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 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 Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review191026
-----------------------------------------------------------



Some suggestions from going over it together:

* Maybe add an optional duration to the profiling start request? I personally would want to enable it on a window basis so that I don't have to remember to follow up and stop to prevent an OOM.
* Could we look into the ability to get a profile that has the symbols resolved? (i.e. "--raw"?) That way, we could analyze the profile on a different machine or on a mac laptop.
* In the markdown documentation, in addition to the changes we made, it would be good to explain what the different profiles are (image vs text vs raw).

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


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63368/#review190864
-----------------------------------------------------------




3rdparty/libprocess/src/memory_profiler.cpp
Lines 345-346 (patched)
<https://reviews.apache.org/r/63368/#comment268427>

    Please add `memory_profiling` to Libprocess flags instead.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 360-361 (patched)
<https://reviews.apache.org/r/63368/#comment268428>

    Let's always a default for now. Feel free to leave a TODO to make it configurable.


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


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
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 Oct. 31, 2017, 1:53 a.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/include/process/memory_profiler.hpp
> > Lines 56-62 (patched)
> > <https://reviews.apache.org/r/63368/diff/1/?file=1870336#file1870336line56>
> >
> >     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.

Renamed to 'downloadXXX'.


- Benno


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


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 Benno Evers <be...@mesosphere.com>.

> On Oct. 31, 2017, 1:53 a.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/include/process/memory_profiler.hpp
> > Lines 26 (patched)
> > <https://reviews.apache.org/r/63368/diff/1/?file=1870336#file1870336line26>
> >
> >     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).

I've experimented a bit with this, but as far as I can see the benefits of introducing a `MemoryProfilerProcess` would be questionable in this particular case:

Right now this class is created once, globally, and only interface are the http endpoints. Introducing a separate `MemoryProfilerProcess` would mean triplicating the entire interface, for a functionality that is not expected to be used.

Furthermore, for some functions like `statistics()`, the natural return type would be `JSON::Object MemoryProfiler::statistics()` - but the HTTP endpoint ultimately needs to return a string, so we would have to parse a JSON struct just to immediately serialize it again, which seems needlessly convoluted.

So I would suggest following the precedence of the other libprocess-global processes (`Profiler`, `Logging`, etc.) and leaving it as it is.

As to the documentation, it was greatly expanded.


> On Oct. 31, 2017, 1:53 a.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/include/process/memory_profiler.hpp
> > Lines 49-54 (patched)
> > <https://reviews.apache.org/r/63368/diff/1/?file=1870336#file1870336line49>
> >
> >     Does this drop down a file? Is it possible to avoid that? Who will clean up this file?

Dropping this, since the function does not exist anymore in this form.

In general, dropping files is unavoidable as it is the only API provided by Mesos. For the initial version, they're not cleared at all (it's at most 3 files, and they're only added when profiling is actually used), later on we would probably want to add an `atexit()`-handler.


- Benno


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


On Feb. 21, 2018, 10:41 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2018, 10:41 a.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/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 42e7adf740b234ebf23d2dcb71440749c0ed87ec 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
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
> 
>