Re: Review Request 58533: Added a benchmark test for hierarchical sorter.

2017-04-26 Thread Neil Conway 

---
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.

2017-04-26 Thread Neil Conway 

---
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.

2017-04-19 Thread Mesos Reviewbot 

---
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.

2017-04-19 Thread Jay Guo 


> On April 20, 2017, 3:04 a.m., Neil Conway wrote:
> > src/tests/sorter_tests.cpp
> > Lines 1570 (patched)
> > 
> >
> > 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.

2017-04-19 Thread Jay Guo 

---
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.

2017-04-19 Thread Neil Conway 

---
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)


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)


I wonder if an iterative solution would be clearer.



src/tests/sorter_tests.cpp
Lines 1571 (patched)


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
> 
>

Re: Review Request 58533: Added a benchmark test for hierarchical sorter.

2017-04-19 Thread Mesos Reviewbot 

---
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
> 
>

Review Request 58533: Added a benchmark test for hierarchical sorter.

2017-04-19 Thread Jay Guo 

---
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