You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jay Guo <gu...@gmail.com> on 2017/04/19 14:21:54 UTC
Review Request 58533: Added a benchmark test for hierarchical sorter.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58533/
-----------------------------------------------------------
Review request for mesos, Benjamin Mahler and Neil Conway.
Bugs: MESOS-7395
https://issues.apache.org/jira/browse/MESOS-7395
Repository: mesos
Description
-------
This test is very similar to Sorter_BENCHMARK_Test.FullSort, except
that hierarchical role is built instead of flat one.
Diffs
-----
src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a
Diff: https://reviews.apache.org/r/58533/diff/1/
Testing
-------
`./bin/mesos-tests.sh --benchmark --gtest_filter="AgentAndClientCount/HSorter_BENCHMARK_Test.FullSort*"`
Thanks,
Jay Guo
Re: Review Request 58533: Added a benchmark test for hierarchical
sorter.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58533/#review172453
-----------------------------------------------------------
Bad review!
Reviews applied: [58533, 57516, 57254, 58112, 58110, 57564, 57528, 57527, 57788, 57167, 57166, 56805, 57165, 57164]
Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing list.
- Mesos Reviewbot
On April 20, 2017, 3:54 a.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58533/
> -----------------------------------------------------------
>
> (Updated April 20, 2017, 3:54 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Neil Conway.
>
>
> Bugs: MESOS-7395
> https://issues.apache.org/jira/browse/MESOS-7395
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test is very similar to Sorter_BENCHMARK_Test.FullSort, except
> that hierarchical role is built instead of flat one.
>
>
> Diffs
> -----
>
> src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a
>
>
> Diff: https://reviews.apache.org/r/58533/diff/2/
>
>
> Testing
> -------
>
> `./bin/mesos-tests.sh --benchmark --gtest_filter="AgentAndClientCount/HSorter_BENCHMARK_Test.FullSort*"`
>
>
> Thanks,
>
> Jay Guo
>
>
Re: Review Request 58533: Added a benchmark test for hierarchical
sorter.
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58533/#review173074
-----------------------------------------------------------
Ship it!
Ship It!
- Neil Conway
On April 20, 2017, 3:54 a.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58533/
> -----------------------------------------------------------
>
> (Updated April 20, 2017, 3:54 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Neil Conway.
>
>
> Bugs: MESOS-7395
> https://issues.apache.org/jira/browse/MESOS-7395
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test is very similar to Sorter_BENCHMARK_Test.FullSort, except
> that hierarchical role is built instead of flat one.
>
>
> Diffs
> -----
>
> src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a
>
>
> Diff: https://reviews.apache.org/r/58533/diff/2/
>
>
> Testing
> -------
>
> `./bin/mesos-tests.sh --benchmark --gtest_filter="AgentAndClientCount/HSorter_BENCHMARK_Test.FullSort*"`
>
>
> Thanks,
>
> Jay Guo
>
>
Re: Review Request 58533: Added a benchmark test for hierarchical
sorter.
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58533/#review173080
-----------------------------------------------------------
Note that a made a few cosmetic fixes; see https://github.com/apache/mesos/commit/f5d77f05b008c063ed6330ed80d08d625827b31a
- Neil Conway
On April 20, 2017, 3:54 a.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58533/
> -----------------------------------------------------------
>
> (Updated April 20, 2017, 3:54 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Neil Conway.
>
>
> Bugs: MESOS-7395
> https://issues.apache.org/jira/browse/MESOS-7395
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test is very similar to Sorter_BENCHMARK_Test.FullSort, except
> that hierarchical role is built instead of flat one.
>
>
> Diffs
> -----
>
> src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a
>
>
> Diff: https://reviews.apache.org/r/58533/diff/2/
>
>
> Testing
> -------
>
> `./bin/mesos-tests.sh --benchmark --gtest_filter="AgentAndClientCount/HSorter_BENCHMARK_Test.FullSort*"`
>
>
> Thanks,
>
> Jay Guo
>
>
Re: Review Request 58533: Added a benchmark test for hierarchical
sorter.
Posted by Jay Guo <gu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58533/
-----------------------------------------------------------
(Updated April 20, 2017, 11:54 a.m.)
Review request for mesos, Benjamin Mahler and Neil Conway.
Changes
-------
address comments from neilc
Bugs: MESOS-7395
https://issues.apache.org/jira/browse/MESOS-7395
Repository: mesos
Description
-------
This test is very similar to Sorter_BENCHMARK_Test.FullSort, except
that hierarchical role is built instead of flat one.
Diffs (updated)
-----
src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a
Diff: https://reviews.apache.org/r/58533/diff/2/
Changes: https://reviews.apache.org/r/58533/diff/1-2/
Testing
-------
`./bin/mesos-tests.sh --benchmark --gtest_filter="AgentAndClientCount/HSorter_BENCHMARK_Test.FullSort*"`
Thanks,
Jay Guo
Re: Review Request 58533: Added a benchmark test for hierarchical
sorter.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58533/#review172347
-----------------------------------------------------------
Bad review!
Reviews applied: [58533, 57516, 57254, 58112, 58110, 57564, 57528, 57527, 57788, 57167, 57166, 56805, 57165, 57164]
Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing list.
- Mesos Reviewbot
On April 19, 2017, 2:21 p.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58533/
> -----------------------------------------------------------
>
> (Updated April 19, 2017, 2:21 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Neil Conway.
>
>
> Bugs: MESOS-7395
> https://issues.apache.org/jira/browse/MESOS-7395
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test is very similar to Sorter_BENCHMARK_Test.FullSort, except
> that hierarchical role is built instead of flat one.
>
>
> Diffs
> -----
>
> src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a
>
>
> Diff: https://reviews.apache.org/r/58533/diff/1/
>
>
> Testing
> -------
>
> `./bin/mesos-tests.sh --benchmark --gtest_filter="AgentAndClientCount/HSorter_BENCHMARK_Test.FullSort*"`
>
>
> Thanks,
>
> Jay Guo
>
>
Re: Review Request 58533: Added a benchmark test for hierarchical
sorter.
Posted by Jay Guo <gu...@gmail.com>.
> On April 20, 2017, 3:04 a.m., Neil Conway wrote:
> > src/tests/sorter_tests.cpp
> > Lines 1570 (patched)
> > <https://reviews.apache.org/r/58533/diff/1/?file=1694263#file1694263line1570>
> >
> > I wonder if an iterative solution would be clearer.
I benchmarked both iterative and recursive solutions, and recursive one seems to be much more efficient.
- Jay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58533/#review172377
-----------------------------------------------------------
On April 20, 2017, 11:54 a.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58533/
> -----------------------------------------------------------
>
> (Updated April 20, 2017, 11:54 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Neil Conway.
>
>
> Bugs: MESOS-7395
> https://issues.apache.org/jira/browse/MESOS-7395
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test is very similar to Sorter_BENCHMARK_Test.FullSort, except
> that hierarchical role is built instead of flat one.
>
>
> Diffs
> -----
>
> src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a
>
>
> Diff: https://reviews.apache.org/r/58533/diff/2/
>
>
> Testing
> -------
>
> `./bin/mesos-tests.sh --benchmark --gtest_filter="AgentAndClientCount/HSorter_BENCHMARK_Test.FullSort*"`
>
>
> Thanks,
>
> Jay Guo
>
>
Re: Review Request 58533: Added a benchmark test for hierarchical
sorter.
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58533/#review172377
-----------------------------------------------------------
src/tests/sorter_tests.cpp
Lines 1547 (patched)
<https://reviews.apache.org/r/58533/#comment245489>
Should end in a period. Can we also clarify the exact number of clients we expect in each configuration?
src/tests/sorter_tests.cpp
Lines 1570 (patched)
<https://reviews.apache.org/r/58533/#comment245488>
I wonder if an iterative solution would be clearer.
src/tests/sorter_tests.cpp
Lines 1571 (patched)
<https://reviews.apache.org/r/58533/#comment245481>
Why are we passing `depth` and `breadth` by const ref here? I'd prefer just `size_t` (no const for by-value parameters).
- Neil Conway
On April 19, 2017, 2:21 p.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58533/
> -----------------------------------------------------------
>
> (Updated April 19, 2017, 2:21 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Neil Conway.
>
>
> Bugs: MESOS-7395
> https://issues.apache.org/jira/browse/MESOS-7395
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test is very similar to Sorter_BENCHMARK_Test.FullSort, except
> that hierarchical role is built instead of flat one.
>
>
> Diffs
> -----
>
> src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a
>
>
> Diff: https://reviews.apache.org/r/58533/diff/1/
>
>
> Testing
> -------
>
> `./bin/mesos-tests.sh --benchmark --gtest_filter="AgentAndClientCount/HSorter_BENCHMARK_Test.FullSort*"`
>
>
> Thanks,
>
> Jay Guo
>
>