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


Fix it, then Ship it!




Looks pretty good! The only thing that feels a little off is the size 
parameterization. It currently represents the number of ranges, but I imagine 
other benchmarks might not want to use that as the "size" (vs the overall range 
size or number of items). I think it's fine for now but probably warrants some 
explanation (e.g. we choose to parameterize on number of subranges becacuse 
it's a dominant factor in the performance of operations). What do you think?


src/tests/resources_tests.cpp
Lines 3981 (patched)
<https://reviews.apache.org/r/67590/#comment288424>

    Oh, I guess I misunderstood this value, it's not the number of items but 
rather the number of sub-ranges? Seems like this needs to at least be 
documented? I guess this means 10,000 represents far more than 10,000 ports in 
the overlapping benchmark? Probably warrants writing it down?



src/tests/resources_tests.cpp
Lines 3992-3993 (patched)
<https://reviews.apache.org/r/67590/#comment288425>

    Can you show the end as well?
    
    ```
    ports1 = [1-6, 11-16, 21-26, ..., 1001-1006] (100 sub-ranges of [1-1006]).
    ```
    
    Looks like the 100 size case is already pretty representative of port 
ranges, and 1000 would be a range of ~ [1-10000], that makes 10,000 beyond what 
we would be interested in benchmarking since it maps to ~ [1-100000]?



src/tests/resources_tests.cpp
Lines 4026 (patched)
<https://reviews.apache.org/r/67590/#comment288426>

    "size" seems ambiguous here, it represents the number of subranges but not 
the number of items or overall range bounds, so we should probably remove any 
ambiguity in the wording


- Benjamin Mahler


On June 28, 2018, 7:34 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67590/
> -----------------------------------------------------------
> 
> (Updated June 28, 2018, 7:34 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8989
>     https://issues.apache.org/jira/browse/MESOS-8989
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> Also removed the current range benchmark which could produce
> misleading results for subtraction (See MESOS-8989).
> 
> 
> Diffs
> -----
> 
>   src/tests/resources_tests.cpp 0095b945ebfd9be52162547f3999826c8cc33f87 
> 
> 
> Diff: https://reviews.apache.org/r/67590/diff/2/
> 
> 
> Testing
> -------
> 
> Build with `-O2` optimization, ran on a multicore machine with peak frequency 
> at 2.2GHz:
> 
> Took 23.31231ms to perform 1000 'a += b' operations on ports:[1-6, 11-16, 
> 21-26... and ports:[3-8, 13-18, 23-28... with size 100
> Took 85.494021ms to perform 1000 'a -= b' operations on ports:[1-6, 11-16, 
> 21-26... and ports:[3-8, 13-18, 23-28... with size 100
> Took 45.536174ms to perform 1000 'a + b' operations on ports:[1-6, 11-16, 
> 21-26... and ports:[3-8, 13-18, 23-28... with size 100
> Took 108.754885ms to perform 1000 'a - b' operations on ports:[1-6, 11-16, 
> 21-26... and ports:[3-8, 13-18, 23-28... with size 100
> 
> Took 309.716888ms to perform 1000 'a += b' operations on ports:[1-6, 11-16, 
> 21-26... and ports:[3-8, 13-18, 23-28... with size 1000
> Took 917.667222ms to perform 1000 'a -= b' operations on ports:[1-6, 11-16, 
> 21-26... and ports:[3-8, 13-18, 23-28... with size 1000
> Took 502.702844ms to perform 1000 'a + b' operations on ports:[1-6, 11-16, 
> 21-26... and ports:[3-8, 13-18, 23-28... with size 1000
> Took 1.178407509secs to perform 1000 'a - b' operations on ports:[1-6, 11-16, 
> 21-26... and ports:[3-8, 13-18, 23-28... with size 1000
> 
> Took 3.144658342secs to perform 1000 'a += b' operations on ports:[1-6, 
> 11-16, 21-26... and ports:[3-8, 13-18, 23-28... with size 10000
> Took 9.086052071secs to perform 1000 'a -= b' operations on ports:[1-6, 
> 11-16, 21-26... and ports:[3-8, 13-18, 23-28... with size 10000
> Took 4.518318425secs to perform 1000 'a + b' operations on ports:[1-6, 11-16, 
> 21-26... and ports:[3-8, 13-18, 23-28... with size 10000
> Took 10.604580315secs to perform 1000 'a - b' operations on ports:[1-6, 
> 11-16, 21-26... and ports:[3-8, 13-18, 23-28... with size 10000
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Reply via email to