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

Ship it!


:D


3rdparty/libprocess/src/reap.cpp
<https://reviews.apache.org/r/25947/#comment94509>

    Why did you use auto here? size_t is very straightforward.
    
    Maybe call this s/size/count/ to better match your constants?



3rdparty/libprocess/src/reap.cpp
<https://reviews.apache.org/r/25947/#comment94511>

    Signed vs unsigned comparison? Looks like LOW_PID_COUNT and HIGH_PID_COUNT 
should be size_t.



3rdparty/libprocess/src/reap.cpp
<https://reviews.apache.org/r/25947/#comment94515>

    You can leverage the Duration operators instead of needing Duration::create 
+ error handling, this cleans up the code quite a lot:
    
    ```
    } else {
      // Linear interpolation.
      double percent = ((double) (size - LOW_PID_COUNT)) / (HIGH_PID_COUNT - 
LOW_PID_COUNT);
    
      return LOW_INTERVAL + (HIGH_INTERVAL - LOW_INTERVAL) * percent;
    }
    ```
    
    Adjust the math as you think is clearest. It might be nice to have the 
linear interpolation logic in one place, instead of defining the "gain" 
constant up above.


- Ben Mahler


On Sept. 24, 2014, 12:26 a.m., Ian Downes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25947/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2014, 12:26 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, Craig Hansen-Sturm, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-1199
>     https://issues.apache.org/jira/browse/MESOS-1199
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> If pid count <= 50 then use 100 ms (<= 0.5% core usage), if count >= 500 use 
> 1000 ms (<= 1% core usage at 500 pids), else interpolate.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/reap.cpp b350ee15bf232493be01506037524b7590c0d2f5 
> 
> Diff: https://reviews.apache.org/r/25947/diff/
> 
> 
> Testing
> -------
> 
> Wrote test program which looped, calling kill(0, pid) and waitpid(pid, 
> &status, WNOHANG) on N children (still running) each loop. These are the 
> system calls used by the reaper. Computed load as wall time / (user + system 
> time). Tested on Linux and OSX.
> 
> `sudo ./bin/mesos-tests.sh` time reduced 20% from 6:40 to 5:20 (not running 
> Docker tests).
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Reply via email to