> On 七月 19, 2016, 3:13 a.m., Benjamin Mahler wrote: > > src/tests/sorter_tests.cpp, lines 510-514 > > <https://reviews.apache.org/r/50172/diff/1/?file=1447034#file1447034line510> > > > > Based on this comment, the formula is wrong? > > > > 10 / 2 + 1 = 6 but the max is 5?
here will be (10 - 1)/2 + 1 = 5 > On 七月 19, 2016, 3:13 a.m., Benjamin Mahler wrote: > > src/tests/sorter_tests.cpp, lines 534-544 > > <https://reviews.apache.org/r/50172/diff/1/?file=1447034#file1447034line534> > > > > It doesn't seem like you need to special case step == 1, is this case > > possible? I'm pretty confused by this code, I would expect the following > > behavior from fragment: > > > > ``` > > [1-10], 1 -> [1-10] > > [1-10], 2 -> [1-4,6-10] > > [1-10], 3 -> [1-3,5-6,8-10] > > [1-10], 4 -> [1-2,4-5,7-8,10-10] // Multiple choices are possible > > [1-10], 5 -> [1-1,3-3,5-5,7-7,9-10] // Multiple choices are possible > > ``` > > > > Or something very simple to implement and still easy to understand, > > like picking off single ranges from the front always, this one has no > > ambiguity like the above: > > > > ``` > > [1-10], 1 -> [1-10] > > [1-10], 2 -> [1-1,3-10] > > [1-10], 3 -> [1-1,3-3,5-10] > > [1-10], 4 -> [1-1,3-3,5-5,7-10] > > [1-10], 5 -> [1-1,3-3,5-5,7-7,9-10] > > ``` > > > > But yours does the following from what I can tell: > > > > ``` > > [1-10], 1 -> [1-2] > > [1-10], 2 -> [1-2,6-7] > > [1-10], 3 -> [1-2,4-5,7-8] > > [1-10], 4 -> [1-2,3-4,5-6,7-8] -> [1-8] // Wrong? > > [1-10], 5 -> [1-2,3-4,5-6,7-8,9-10] -> [1-10] // Wrong? > > ``` > > > > This looks wrong..? I'd say let's just implement the second suggestion > > I made here as it's simple, and it is easy to understand. Hi Ben, with above logic, [1-10], 5 -> [1-1,3-3,5-5,7-7,9-9]. I lost the case when step is 2, basically, the step should start from 3 to make sure there is no overlap, so what about the following logic: [1-10], 1 -> [1-2] [1-10], 2 -> [1-2,5-6] [1-10], 3 -> [1-2,4-5,7-8] [1-10], 4 -> [1-1,3-3,5-5,7-7] [1-10], 5 -> [1-1,3-3,5-5,7-7,9-9] ``` if (step < 3) { start = bounds.begin() + (i * 2); end = start; } else { start = bounds.begin() + (i * step); end = start + 1; } ``` - Guangya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50172/#review142684 ----------------------------------------------------------- On 七月 19, 2016, 3:35 a.m., Guangya Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50172/ > ----------------------------------------------------------- > > (Updated 七月 19, 2016, 3:35 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > This patch added two new functions to sorter benchmark test to > generate a `ports` resource which falls into a specifed ports > range bounds. > > 1) fragment: Returns a "ports" resource with the number of ranges. > 2) makeRange: Returns the "ports" range bounds. > > > Diffs > ----- > > src/tests/sorter_tests.cpp b0e5ef8a55bbcca3553a221bd5691a9c801a04f7 > > Diff: https://reviews.apache.org/r/50172/diff/ > > > Testing > ------- > > ``` > ./bin/mesos-tests.sh --benchmark > --gtest_filter="AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/1" > [==========] Running 1 test from 1 test case. > [----------] Global test environment set-up. > [----------] 1 test from AgentAndClientCount/Sorter_BENCHMARK_Test > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/1 > Using 1000 agents and 50 clients > Added 50 clients in 872us > Added 1000 agents in 26457us > Added allocations for 1000 agents in 79697us > Full sort of 50 clients took 1321us > No-op sort of 50 clients took 28us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/1 (115 ms) > [----------] 1 test from AgentAndClientCount/Sorter_BENCHMARK_Test (115 ms > total) > > [----------] Global test environment tear-down > [==========] 1 test from 1 test case ran. (133 ms total) > [ PASSED ] 1 test. > ``` > > > Thanks, > > Guangya Liu > >
