> On 十月 7, 2016, 7:31 p.m., Benjamin Mahler wrote: > > Thanks looks pretty good! Just a few suggestions for simplifying things a > > bit below. > > > > Also, 42 seconds for these seems too long for a benchmark test, can you > > reduce it to be no more than a few seconds to run these?
Sure, I will make the `totalOperations` as a paramter and control the time. > On 十月 7, 2016, 7:31 p.m., Benjamin Mahler wrote: > > src/tests/resources_tests.cpp, lines 2959-2963 > > <https://reviews.apache.org/r/50551/diff/4/?file=1525234#file1525234line2959> > > > > How about calling this `Parameter` like the one above and nesting both > > of them within the test class? i.e. > > `Resources_Contains_BENCHMARK_Test::Parameter` Bwn, just a question here: I'm not clear for how we can do this as the `Resources_Contains_BENCHMARK_Test` actually depend on `ContainsParameter` as following: ``` class Resources_Contains_BENCHMARK_Test : public ::testing::Test, public ::testing::WithParamInterface<ContainsParameter> { ``` So how can we nest the `ContainsParameter` in `Resources_Contains_BENCHMARK_Test`? Can you please show more detail here? > On 十月 7, 2016, 7:31 p.m., Benjamin Mahler wrote: > > src/tests/resources_tests.cpp, lines 2971-2972 > > <https://reviews.apache.org/r/50551/diff/4/?file=1525234#file1525234line2971> > > > > Showing a list of the parameters up here in the comment would be > > helpful. will update this in next patch. > On 十月 7, 2016, 7:31 p.m., Benjamin Mahler wrote: > > src/tests/resources_tests.cpp, lines 3042-3053 > > <https://reviews.apache.org/r/50551/diff/4/?file=1525234#file1525234line3042> > > > > It looks like we should just perform this portion > > (`superset.contains(subset)`) of the benchmark, rather than also doing > > `subset.contains(superset)` and `subset.contains(subset)`. > > > > If we want to test other contains checks we can simply add parameters. I'm using those three is because I want to cover three cases: 1) superset.contains(subset): this is a `TRUE` case for contain 2) subset.contains(superset): This is a `FALSE` case for contain 3) subset.contains(subset): This is a `TRUE` case for some boundry test cast. I will use a parameter to control the elapse time here, what about keeping those three cases if we can reduce the elapse time to be in a few seconds? - Guangya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50551/#review151852 ----------------------------------------------------------- On 十月 6, 2016, 9:30 a.m., Guangya Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50551/ > ----------------------------------------------------------- > > (Updated 十月 6, 2016, 9:30 a.m.) > > > Review request for mesos, Benjamin Mahler, Klaus Ma, and Jiang Yan Xu. > > > Bugs: MESOS-5700 > https://issues.apache.org/jira/browse/MESOS-5700 > > > Repository: mesos > > > Description > ------- > > Added benchmark test for `Resources::contains`. > > > Diffs > ----- > > src/tests/resources_tests.cpp 3e493007d6d1d8194d07035aaa1cde28dedf2b5a > > Diff: https://reviews.apache.org/r/50551/diff/ > > > Testing > ------- > > make > make check > > ``` > ./bin/mesos-tests.sh --benchmark > --gtest_filter="*Resources_Contains_BENCHMARK_Test.Contains/*" > [==========] Running 3 tests from 1 test case. > [----------] Global test environment set-up. > [----------] 3 tests from ResourcesContains/Resources_Contains_BENCHMARK_Test > [ RUN ] ResourcesContains/Resources_Contains_BENCHMARK_Test.Contains/0 > Took 8629us to perform 5000 'subset.contains(superset)' operations on subset > resources cpus(*):1; mem(*):128 contains superset resources cpus(*):1; > gpus(*):1; mem(*):128; disk(*):256 > Took 20378us to perform 5000 'superset.contains(subset)' operations on > superset resources cpus(*):1; gpus(*):1; mem(*):128; disk(*):256 contains > subset resources cpus(*):1; mem(*):128 > Took 9526us to perform 5000 'subset.contains(subset)' operations on subset > resources cpus(*):1; mem(*):128 contains subset resources cpus(*):1; > mem(*):128 > [ OK ] ResourcesContains/Resources_Contains_BENCHMARK_Test.Contains/0 > (39 ms) > [ RUN ] ResourcesContains/Resources_Contains_BENCHMARK_Test.Contains/1 > Took 1.165801secs to perform 5000 'subset.contains(superset)' operations on > subset resources ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1... contains > superset resources ports(*):[1-1000] > Took 895058us to perform 5000 'superset.contains(subset)' operations on > superset resources ports(*):[1-1000] contains subset resources ports(*):[1-2, > 4-5, 7-8, 10-11, 13-14, 16-17, 1... > Took 19.768667secs to perform 5000 'subset.contains(subset)' operations on > subset resources ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1... contains > subset resources ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1... > [ OK ] ResourcesContains/Resources_Contains_BENCHMARK_Test.Contains/1 > (21831 ms) > [ RUN ] ResourcesContains/Resources_Contains_BENCHMARK_Test.Contains/2 > Took 326768us to perform 5000 'subset.contains(superset)' operations on > subset resources cpus(*):1; mem(*):128; ports(*):[1-2, 4-5, 7-8,... contains > superset resources cpus(*):1; gpus(*):1; mem(*):128; disk(*):256; ... > Took 1.003977secs to perform 5000 'superset.contains(subset)' operations on > superset resources cpus(*):1; gpus(*):1; mem(*):128; disk(*):256; ... > contains subset resources cpus(*):1; mem(*):128; ports(*):[1-2, 4-5, 7-8,... > Took 19.641502secs to perform 5000 'subset.contains(subset)' operations on > subset resources cpus(*):1; mem(*):128; ports(*):[1-2, 4-5, 7-8,... contains > subset resources cpus(*):1; mem(*):128; ports(*):[1-2, 4-5, 7-8,... > [ OK ] ResourcesContains/Resources_Contains_BENCHMARK_Test.Contains/2 > (20974 ms) > [----------] 3 tests from ResourcesContains/Resources_Contains_BENCHMARK_Test > (42844 ms total) > > [----------] Global test environment tear-down > [==========] 3 tests from 1 test case ran. (42853 ms total) > [ PASSED ] 3 tests. > ``` > > > Thanks, > > Guangya Liu > >
