> On Feb. 4, 2019, 7:22 p.m., Benjamin Mahler wrote: > > src/tests/resources_tests.cpp > > Lines 3883-3887 (original), 3883-3891 (patched) > > <https://reviews.apache.org/r/69885/diff/1/?file=2123828#file2123828line3883> > > > > I love the spirit of this change, but this seems pretty bizzare? This > > function builds a vector of resources, only to return a particular index? > > > > How about using `SetUpTestSuite` to build the resource parameters and > > then indexing into that? > > > > > > https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#sharing-resources-between-tests-in-the-same-test-suite
That's definitely less weird, yes. I still keep the `parameters` function which allows some validation. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69885/#review212519 ----------------------------------------------------------- On Feb. 4, 2019, 9:29 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69885/ > ----------------------------------------------------------- > > (Updated Feb. 4, 2019, 9:29 p.m.) > > > Review request for mesos, Benjamin Mahler and Klaus Ma. > > > Bugs: MESOS-8835 > https://issues.apache.org/jira/browse/MESOS-8835 > > > Repository: mesos > > > Description > ------- > > This patch moves computation of some resource benchmark test parameters > from test instantiation time to test execution time. This prevents us > from having to perform the expensive calculation of test parameters even > when not executing the benchmark. > > As a result the startup time of the Mesos tests binary is improved, > while the total wall time required to run these particular benchmarks is > degraded accordingly. > > > Diffs > ----- > > src/tests/resources_tests.cpp f762d17376cc5c29e8556ef5aa2b981e8fe19985 > > > Diff: https://reviews.apache.org/r/69885/diff/2/ > > > Testing > ------- > > Benchmarked `./src/mesos-tests --gtest_list_tests` with clang-9.0.0, > lld-2.27. Overall execution time is improved, especially for not optimized > builds. > > ``` > Benchmark #1: Before patch, debug > Time (mean ± ?): 2.706 s ± 0.018 s [User: 2.472 s, System: 0.168 s] > Range (min … max): 2.690 s … 2.732 s 10 runs > Benchmark #2: After patch, debug > Time (mean ± ?): 683.7 ms ± 18.1 ms [User: 474.2 ms, System: 152.9 > ms] > Range (min … max): 673.4 ms … 734.2 ms 10 runs > ``` > > ``` > Benchmark #3: Before patch, optimized > Time (mean ± ?): 783.0 ms ± 15.0 ms [User: 537.4 ms, System: 144.9 > ms] > Range (min … max): 772.2 ms … 815.5 ms 10 runs > Benchmark #4: After patch, optimized > Time (mean ± ?): 572.5 ms ± 6.7 ms [User: 343.3 ms, System: 138.4 > ms] > Range (min … max): 562.2 ms … 588.7 ms 10 runs > ``` > > Remaining time is due to the long list of filters `mesos-tests` uses. > > > Thanks, > > Benjamin Bannier > >