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

Ship it!


Will get this committed now, thanks Paul!


src/linux/perf.cpp (line 167)
<https://reviews.apache.org/r/37423/#comment150885>

    This seems a little brittle, because the caller may have already put 'perf' 
as the first argument, no? I'll add a note as to why we need this hack.



src/linux/perf.cpp (line 183)
<https://reviews.apache.org/r/37423/#comment150886>

    newline for readability?



src/linux/perf.cpp (line 344)
<https://reviews.apache.org/r/37423/#comment150887>

    PerfSampler doesn't exist..? We can just remove this comment altogether IMO.



src/linux/perf.cpp (lines 356 - 370)
<https://reviews.apache.org/r/37423/#comment150888>

    Recall that with .then your lambda doesn't need to take a Future since it 
only composes when the future succeeds, normally calling .get() on a Future 
would need to be guarded to ensure the future is ready.
    
    Ideally we could compose with Try here:
    
    ```
    auto stamp = [start, duration] (
        const hashmap<string, mesos::PerfStatistics>& statistics) {
      foreachvalue (mesos::PerfStatistics& s, statistics) {
        s.set_timestamp(start.secs());
        s.set_duration(duration.secs());
      }
      return stats;
    }
    
    return future
      .then(perf::parse)
      .then(stamp);
    ```
    
    But we can't compose like this currently, so I'll just pull out a single 
lambda for parsing / stamping.


- Ben Mahler


On Aug. 18, 2015, 6:10 p.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37423/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2015, 6:10 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3185
>     https://issues.apache.org/jira/browse/MESOS-3185
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Split out common functions for running "perf" into a common perf class with 
> wrapper functions to allow for reuse between sample, valid and version 
> operations.
> 
> 
> Diffs
> -----
> 
>   src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 
> 
> Diff: https://reviews.apache.org/r/37423/diff/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Reply via email to