You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jiang Yan Xu <ya...@jxu.me> on 2017/04/28 23:06:22 UTC
Re: Review Request 56732: Remove unnecessary perf version checks.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/56732/#comment246394>
Now without the `process::collect`.
1. Remove from includes:
```
#include <process/collect.hpp>
```
2. We can just inline the lambda (the way we usually do this)
```
return output
.then([start, duration](const string output)
-> Future<hashmap<string, mesos::PerfStatistics>> {
Try<hashmap<string, mesos::PerfStatistics>> 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 9c4330b6086abb18f036222260fe89a6fb01d8ed
> 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.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On April 28, 2017, 4:06 p.m., Jiang Yan Xu wrote:
> > src/linux/perf.cpp
> > Line 322 (original), 313 (patched)
> > <https://reviews.apache.org/r/56732/diff/1/?file=1635867#file1635867line322>
> >
> > Now without the `process::collect`.
> >
> > 1. Remove from includes:
> > ```
> > #include <process/collect.hpp>
> > ```
> >
> > 2. We can just inline the lambda (the way we usually do this)
> >
> > ```
> > return output
> > .then([start, duration](const string output)
> > -> Future<hashmap<string, mesos::PerfStatistics>> {
> > Try<hashmap<string, mesos::PerfStatistics>> 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 <process/collect.hpp>` 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 9c4330b6086abb18f036222260fe89a6fb01d8ed
> 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
>
>