Re: Review Request 56732: Remove unnecessary perf version checks.

2017-04-28 Thread Jiang Yan Xu


> On April 28, 2017, 4:06 p.m., Jiang Yan Xu wrote:
> > src/linux/perf.cpp
> > Line 322 (original), 313 (patched)
> > 
> >
> > Now without the `process::collect`.
> > 
> > 1. Remove from includes:
> > ```
> > #include 
> > ```
> > 
> > 2. We can just inline the lambda (the way we usually do this)
> > 
> > ```
> >   return output
> > .then([start, duration](const string output)
> > -> Future> {
> >   Try> result =
> > perf::parse(output);
> > 
> >   if (result.isError()) {
> > return Failure("Failed to parse perf sample: " + 
> > result.error());
> >   }
> > 
> >   foreachvalue (mesos::PerfStatistics& statistics, result.get()) {
> > statistics.set_timestamp(start.secs());
> > statistics.set_duration(duration.secs());
> >   }
> > 
> >   return result.get();
> > });
> > ```

Apparently we have to keep `#include ` for `process:await`.


- Jiang Yan


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


On Feb. 15, 2017, 3:22 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56732/
> ---
> 
> (Updated Feb. 15, 2017, 3:22 p.m.)
> 
> 
> Review request for mesos, Mesos Reviewbot and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7049
> https://issues.apache.org/jira/browse/MESOS-7049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We can remove the version check from the perf subprocess
> wrapper because we longer uses the version to figure out
> how to parse the stat output and the callers explicitly
> check perf::supported().
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp 9c4330b6086abb18f03660fe89a6fb01d8ed 
>   src/linux/perf.cpp 3141e5ee9eee78e974625791f362dc345c682ebb 
>   src/tests/containerizer/perf_tests.cpp 
> d536ecc5cb24787bc3487efb146573cd4f3ded43 
> 
> 
> Diff: https://reviews.apache.org/r/56732/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 56732: Remove unnecessary perf version checks.

2017-04-28 Thread Jiang Yan Xu

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


Fix it, then Ship it!




Committing with the minor changes if no objection.


src/linux/perf.cpp
Line 322 (original), 313 (patched)


Now without the `process::collect`.

1. Remove from includes:
```
#include 
```

2. We can just inline the lambda (the way we usually do this)

```
  return output
.then([start, duration](const string output)
-> Future> {
  Try> result =
perf::parse(output);

  if (result.isError()) {
return Failure("Failed to parse perf sample: " + result.error());
  }

  foreachvalue (mesos::PerfStatistics& statistics, result.get()) {
statistics.set_timestamp(start.secs());
statistics.set_duration(duration.secs());
  }

  return result.get();
});
```


- Jiang Yan Xu


On Feb. 15, 2017, 3:22 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56732/
> ---
> 
> (Updated Feb. 15, 2017, 3:22 p.m.)
> 
> 
> Review request for mesos, Mesos Reviewbot and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7049
> https://issues.apache.org/jira/browse/MESOS-7049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We can remove the version check from the perf subprocess
> wrapper because we longer uses the version to figure out
> how to parse the stat output and the callers explicitly
> check perf::supported().
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp 9c4330b6086abb18f03660fe89a6fb01d8ed 
>   src/linux/perf.cpp 3141e5ee9eee78e974625791f362dc345c682ebb 
>   src/tests/containerizer/perf_tests.cpp 
> d536ecc5cb24787bc3487efb146573cd4f3ded43 
> 
> 
> Diff: https://reviews.apache.org/r/56732/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 56732: Remove unnecessary perf version checks.

2017-03-07 Thread haosdent huang

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


Ship it!




Ship It!

- haosdent huang


On Feb. 15, 2017, 11:22 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56732/
> ---
> 
> (Updated Feb. 15, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos, Mesos Reviewbot and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7049
> https://issues.apache.org/jira/browse/MESOS-7049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We can remove the version check from the perf subprocess
> wrapper because we longer uses the version to figure out
> how to parse the stat output and the callers explicitly
> check perf::supported().
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp 9c4330b6086abb18f03660fe89a6fb01d8ed 
>   src/linux/perf.cpp 3141e5ee9eee78e974625791f362dc345c682ebb 
>   src/tests/containerizer/perf_tests.cpp 
> d536ecc5cb24787bc3487efb146573cd4f3ded43 
> 
> 
> Diff: https://reviews.apache.org/r/56732/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>