You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Niklas Nielsen <ni...@qni.dk> on 2014/10/08 19:32:48 UTC
Re: Review Request 26150: Libprocess Benchmark
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26150/#review55824
-----------------------------------------------------------
A high-level comment: It is a bit hard to understand what's going on here. The test body is pretty dense and not well commented - mind spending a few cycles breaking it up and make it more consumable?
Also, we need to do a style scan :-)
3rdparty/libprocess/src/tests/benchmarks.cpp
<https://reviews.apache.org/r/26150/#comment96201>
Let's include the usual license blob.
3rdparty/libprocess/src/tests/benchmarks.cpp
<https://reviews.apache.org/r/26150/#comment96203>
We try to use camel case, so _numIter, _maxOutstanding and so on.
Here and below.
3rdparty/libprocess/src/tests/benchmarks.cpp
<https://reviews.apache.org/r/26150/#comment96202>
Out of curiosity - wouldn't plain '0' do?
3rdparty/libprocess/src/tests/benchmarks.cpp
<https://reviews.apache.org/r/26150/#comment96204>
Bring '{' down on new line
3rdparty/libprocess/src/tests/benchmarks.cpp
<https://reviews.apache.org/r/26150/#comment96205>
We try to spell out variable names. How about "message"?
3rdparty/libprocess/src/tests/benchmarks.cpp
<https://reviews.apache.org/r/26150/#comment96206>
s/max_outstanding/maxOutstanding/
3rdparty/libprocess/src/tests/benchmarks.cpp
<https://reviews.apache.org/r/26150/#comment96208>
Instead of doing char* and strlen. Can we use a std::string instead?
3rdparty/libprocess/src/tests/benchmarks.cpp
<https://reviews.apache.org/r/26150/#comment96210>
For wrapping boolean expressions, see http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Boolean_Expressions
3rdparty/libprocess/src/tests/benchmarks.cpp
<https://reviews.apache.org/r/26150/#comment96214>
Would you mind adding a comment on what the test is doing here?
3rdparty/libprocess/src/tests/benchmarks.cpp
<https://reviews.apache.org/r/26150/#comment96211>
+1 - can we do this the old way for now?
3rdparty/libprocess/src/tests/benchmarks.cpp
<https://reviews.apache.org/r/26150/#comment96212>
Same here
3rdparty/libprocess/src/tests/benchmarks.cpp
<https://reviews.apache.org/r/26150/#comment96213>
I am not sure we have graced range based loops yet. Do you have any references to whether it is supported by our set of graced compilers?
- Niklas Nielsen
On Sept. 29, 2014, 2:38 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26150/
> -----------------------------------------------------------
>
> (Updated Sept. 29, 2014, 2:38 p.m.)
>
>
> Review request for mesos and Niklas Nielsen.
>
>
> Bugs: MESOS-1840
> https://issues.apache.org/jira/browse/MESOS-1840
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> A benchmark for libprocess.
> It forks num_proc times and launched num_threads "libprocess" processes in each child. They are aware of the "master"'s (parent) address and all play ping pong with it.
> This benchmark measures throughput in terms of the number of RPCs handled per second using persistent (linked) connections.
>
> A new test file (benchmarks) is introduced because we want to fork before libprocess is initialized. This allows us to prevent short-circuiting of message passing between processes under the same ProcessManager. This way we force the execution path of the underlying event management system.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am 616618e
> 3rdparty/libprocess/src/tests/benchmarks.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26150/diff/
>
>
> Testing
> -------
>
> make check in 3rdparty/libprocess
> support/mesos-style.py
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 26150: Libprocess Benchmark
Posted by Michael Park <mc...@gmail.com>.
> On Oct. 8, 2014, 5:32 p.m., Niklas Nielsen wrote:
> > 3rdparty/libprocess/src/tests/benchmarks.cpp, line 200
> > <https://reviews.apache.org/r/26150/diff/1/?file=708531#file708531line200>
> >
> > I am not sure we have graced range based loops yet. Do you have any references to whether it is supported by our set of graced compilers?
It's not :( Not supported by `gcc-4.4`.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26150/#review55824
-----------------------------------------------------------
On Sept. 29, 2014, 9:38 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26150/
> -----------------------------------------------------------
>
> (Updated Sept. 29, 2014, 9:38 p.m.)
>
>
> Review request for mesos and Niklas Nielsen.
>
>
> Bugs: MESOS-1840
> https://issues.apache.org/jira/browse/MESOS-1840
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> A benchmark for libprocess.
> It forks num_proc times and launched num_threads "libprocess" processes in each child. They are aware of the "master"'s (parent) address and all play ping pong with it.
> This benchmark measures throughput in terms of the number of RPCs handled per second using persistent (linked) connections.
>
> A new test file (benchmarks) is introduced because we want to fork before libprocess is initialized. This allows us to prevent short-circuiting of message passing between processes under the same ProcessManager. This way we force the execution path of the underlying event management system.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am 616618e
> 3rdparty/libprocess/src/tests/benchmarks.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26150/diff/
>
>
> Testing
> -------
>
> make check in 3rdparty/libprocess
> support/mesos-style.py
>
>
> Thanks,
>
> Joris Van Remoortere
>
>