> On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote: > >
Thanks Ben for the cleanup and helping merge the code. Please check my answers inline. > On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote: > > src/tests/sorter_tests.cpp, lines 504-542 > > <https://reviews.apache.org/r/49843/diff/3/?file=1442032#file1442032line504> > > > > It looks like we can simplify these two functions down to the following > > single function? > > > > ``` > > // Returns a "ports" resource with the number of ranges > > // specified as: [1-2, 4-5, 7-8, 10-11, ...] > > static Resource makePortRanges(size_t numRanges) > > { > > ::mesos::Value::Ranges ranges; > > > > for (size_t i = 0; i < numRanges; ++i) { > > Value::Range* range = ranges.add_range(); > > range->set_begin((3 * i) + 1); > > range->set_end(range->begin() + 1); > > } > > > > Value value; > > value.set_type(Value::RANGES); > > value.mutable_ranges()->CopyFrom(ranges); > > > > Resource resource; > > resource.set_role("*"); > > resource.set_name("ports"); > > resource.set_type(Value::RANGES); > > resource.mutable_ranges()->CopyFrom(value.ranges()); > > > > return resource; > > } > > ``` > > > > Since we only care about the number of ranges, we don't need to specify > > the bounds, right? Yes, my orignial thinking was keep this same as https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L85-L122 in case we may need specify bounds in future, but since this is only for test and we do not need to care for bounds, so seems no need to have `makeRange` here. I was thinking we may also need to update https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L85-L122 as well to use a single function to create port ranges. > On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote: > > src/tests/sorter_tests.cpp, line 511 > > <https://reviews.apache.org/r/49843/diff/3/?file=1442032#file1442032line511> > > > > Is this an optimization? Yes, but I saw that you removed this part when merge the code, do we need to add it back? ``` // Reserve space to expand the field to at least the given size. This only // resizes the pointer array; it doesn't allocate any objects. If the // array is grown, it will always be at least doubled in size. void Reserve(int new_size); ``` > On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote: > > src/tests/sorter_tests.cpp, line 536 > > <https://reviews.apache.org/r/49843/diff/3/?file=1442032#file1442032line536> > > > > We tend to use 'unsigned int' rather than just 'unsigned'. >From http://en.cppreference.com/w/cpp/types/size_t , it seems it is >recommended to use `size_t` with following reason: ``` std::size_t is commonly used for array indexing and loop counting. Programs that use other types, such as unsigned int, for array indexing may fail on, e.g. 64-bit systems when the index exceeds UINT_MAX or if it relies on 32-bit modular arithmetic. ``` > On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote: > > src/tests/sorter_tests.cpp, line 577 > > <https://reviews.apache.org/r/49843/diff/3/?file=1442032#file1442032line577> > > > > Seems we should move this down to at the start of the loop to avoid > > timing the resource parsing? Yes. > On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote: > > src/tests/sorter_tests.cpp, line 595 > > <https://reviews.apache.org/r/49843/diff/3/?file=1442032#file1442032line595> > > > > No need for the trailing semi-colon after disk? I will do a clean up for this in mesos test cause many `Resources::parse` including a trailing semi-colon. > On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote: > > src/tests/sorter_tests.cpp, lines 600-603 > > <https://reviews.apache.org/r/49843/diff/3/?file=1442032#file1442032line600> > > > > Oh.. this isn't allocating resources on all of the agents! The > > following will do it: > > > > ``` > > watch.start(); > > size_t clientIndex = 0; > > foreach (const SlaveID& slaveId, agents) { > > const string& client = clients[clientIndex++ % clients.size()]; > > sorter.allocated(client, slaveId, allocated); > > } > > watch.stop(); > > ``` > > > > Note that this makes things slower, as expected. Sorry I missed that part, I was only thinking to make all clients get resources and then check the time of `sorter`, yes, we should also make the time of adding agents accurate. > On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote: > > src/tests/sorter_tests.cpp, lines 515-516 > > <https://reviews.apache.org/r/49843/diff/3/?file=1442032#file1442032line515> > > > > Why unsigned when these are actually uint64_t? I should check this when get it from https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L85-L122 - Guangya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49843/#review142264 ----------------------------------------------------------- On 七月 12, 2016, 9:09 a.m., Guangya Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49843/ > ----------------------------------------------------------- > > (Updated 七月 12, 2016, 9:09 a.m.) > > > Review request for mesos, Benjamin Mahler and Klaus Ma. > > > Bugs: MESOS-5701 > https://issues.apache.org/jira/browse/MESOS-5701 > > > Repository: mesos > > > Description > ------- > > Added benchmark test for sorter. > > > Diffs > ----- > > src/tests/sorter_tests.cpp 20e42419934e81b97676569876cd63ee0a550da3 > > Diff: https://reviews.apache.org/r/49843/diff/ > > > Testing > ------- > > make > make check > > ./bin/mesos-tests.sh --benchmark > --gtest_filter="AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/*" > [==========] Running 36 tests from 1 test case. > [----------] Global test environment set-up. > [----------] 36 tests from AgentAndClientCount/Sorter_BENCHMARK_Test > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/0 > Using 1000 agents and 1 clients > Added 1 clients in 1047us > Added 1000 agents in 30147us > Sorted 1 clients in 87us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/0 (33 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/1 > Using 1000 agents and 50 clients > Added 50 clients in 884us > Added 1000 agents in 30129us > Sorted 50 clients in 1284us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/1 (37 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/2 > Using 1000 agents and 100 clients > Added 100 clients in 1676us > Added 1000 agents in 25157us > Sorted 100 clients in 3814us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/2 (41 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/3 > Using 1000 agents and 200 clients > Added 200 clients in 3930us > Added 1000 agents in 25743us > Sorted 200 clients in 7780us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/3 (58 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/4 > Using 1000 agents and 500 clients > Added 500 clients in 9219us > Added 1000 agents in 29179us > Sorted 500 clients in 16210us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/4 (112 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/5 > Using 1000 agents and 1000 clients > Added 1000 clients in 22029us > Added 1000 agents in 28043us > Sorted 1000 clients in 36030us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/5 (192 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/6 > Using 5000 agents and 1 clients > Added 1 clients in 43us > Added 5000 agents in 134324us > Sorted 1 clients in 42us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/6 (136 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/7 > Using 5000 agents and 50 clients > Added 50 clients in 845us > Added 5000 agents in 137171us > Sorted 50 clients in 1274us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/7 (144 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/8 > Using 5000 agents and 100 clients > Added 100 clients in 1698us > Added 5000 agents in 137456us > Sorted 100 clients in 2582us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/8 (152 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/9 > Using 5000 agents and 200 clients > Added 200 clients in 3460us > Added 5000 agents in 129285us > Sorted 200 clients in 5587us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/9 (159 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/10 > Using 5000 agents and 500 clients > Added 500 clients in 8871us > Added 5000 agents in 133412us > Sorted 500 clients in 16717us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/10 (212 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/11 > Using 5000 agents and 1000 clients > Added 1000 clients in 22705us > Added 5000 agents in 143366us > Sorted 1000 clients in 36587us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/11 (313 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/12 > Using 10000 agents and 1 clients > Added 1 clients in 34us > Added 10000 agents in 284603us > Sorted 1 clients in 43us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/12 (286 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/13 > Using 10000 agents and 50 clients > Added 50 clients in 857us > Added 10000 agents in 270253us > Sorted 50 clients in 1192us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/13 (279 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/14 > Using 10000 agents and 100 clients > Added 100 clients in 1786us > Added 10000 agents in 282553us > Sorted 100 clients in 2660us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/14 (297 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/15 > Using 10000 agents and 200 clients > Added 200 clients in 3447us > Added 10000 agents in 270059us > Sorted 200 clients in 6696us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/15 (302 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/16 > Using 10000 agents and 500 clients > Added 500 clients in 10724us > Added 10000 agents in 271856us > Sorted 500 clients in 16945us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/16 (353 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/17 > Using 10000 agents and 1000 clients > Added 1000 clients in 21394us > Added 10000 agents in 274222us > Sorted 1000 clients in 38006us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/17 (437 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/18 > Using 20000 agents and 1 clients > Added 1 clients in 44us > Added 20000 agents in 545506us > Sorted 1 clients in 50us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/18 (549 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/19 > Using 20000 agents and 50 clients > Added 50 clients in 823us > Added 20000 agents in 558720us > Sorted 50 clients in 1270us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/19 (568 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/20 > Using 20000 agents and 100 clients > Added 100 clients in 1682us > Added 20000 agents in 603786us > Sorted 100 clients in 2689us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/20 (623 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/21 > Using 20000 agents and 200 clients > Added 200 clients in 3563us > Added 20000 agents in 639230us > Sorted 200 clients in 7246us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/21 (681 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/22 > Using 20000 agents and 500 clients > Added 500 clients in 9577us > Added 20000 agents in 603457us > Sorted 500 clients in 16546us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/22 (691 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/23 > Using 20000 agents and 1000 clients > Added 1000 clients in 18104us > Added 20000 agents in 517677us > Sorted 1000 clients in 31077us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/23 (677 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/24 > Using 30000 agents and 1 clients > Added 1 clients in 47us > Added 30000 agents in 856802us > Sorted 1 clients in 46us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/24 (862 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/25 > Using 30000 agents and 50 clients > Added 50 clients in 1665us > Added 30000 agents in 883548us > Sorted 50 clients in 1259us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/25 (895 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/26 > Using 30000 agents and 100 clients > Added 100 clients in 1619us > Added 30000 agents in 834926us > Sorted 100 clients in 2630us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/26 (855 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/27 > Using 30000 agents and 200 clients > Added 200 clients in 3288us > Added 30000 agents in 798885us > Sorted 200 clients in 7605us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/27 (834 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/28 > Using 30000 agents and 500 clients > Added 500 clients in 8797us > Added 30000 agents in 798340us > Sorted 500 clients in 17014us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/28 (882 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/29 > Using 30000 agents and 1000 clients > Added 1000 clients in 20749us > Added 30000 agents in 817770us > Sorted 1000 clients in 35576us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/29 (976 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/30 > Using 50000 agents and 1 clients > Added 1 clients in 47us > Added 50000 agents in 1.502226secs > Sorted 1 clients in 56us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/30 (1512 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/31 > Using 50000 agents and 50 clients > Added 50 clients in 1150us > Added 50000 agents in 1.367002secs > Sorted 50 clients in 1820us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/31 (1385 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/32 > Using 50000 agents and 100 clients > Added 100 clients in 1812us > Added 50000 agents in 1.374993secs > Sorted 100 clients in 2552us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/32 (1395 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/33 > Using 50000 agents and 200 clients > Added 200 clients in 3197us > Added 50000 agents in 1.319963secs > Sorted 200 clients in 5672us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/33 (1356 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/34 > Using 50000 agents and 500 clients > Added 500 clients in 8942us > Added 50000 agents in 1.368761secs > Sorted 500 clients in 14604us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/34 (1459 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35 > Using 50000 agents and 1000 clients > Added 1000 clients in 19147us > Added 50000 agents in 1.401624secs > Sorted 1000 clients in 35276us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35 (1579 ms) > [----------] 36 tests from AgentAndClientCount/Sorter_BENCHMARK_Test (21323 > ms total) > > [----------] Global test environment tear-down > [==========] 36 tests from 1 test case ran. (21345 ms total) > [ PASSED ] 36 tests. > > > Thanks, > > Guangya Liu > >
