> On Oct. 24, 2019, 7:21 a.m., Andrei Sekretenko wrote: > > src/tests/hierarchical_allocator_tests.cpp > > Lines 8414 (patched) > > <https://reviews.apache.org/r/71639/diff/2/?file=2170111#file2170111line8414> > > > > Wouldn't the order of parameters look more intuitive if it were > > (roleCount, reservationCount, portRangeCount)? > > > > After all, reservationCount is a count of reservations per role, and > > portRangeCount is per-reservation, right?
sounds good. > On Oct. 24, 2019, 7:21 a.m., Andrei Sekretenko wrote: > > src/tests/hierarchical_allocator_tests.cpp > > Lines 8440 (patched) > > <https://reviews.apache.org/r/71639/diff/2/?file=2170111#file2170111line8440> > > > > Why not just use UUIDs? wanted to control the length of labels and etc. > On Oct. 24, 2019, 7:21 a.m., Andrei Sekretenko wrote: > > src/tests/hierarchical_allocator_tests.cpp > > Lines 8464 (patched) > > <https://reviews.apache.org/r/71639/diff/2/?file=2170111#file2170111line8464> > > > > All the roles will be the same, right? > > > > We should either generate different child roles, or use a single role, > > rename `rolesCount` into `frameworksCount` and reword the comments. > > > > (If I'm not missing something, under current architecture of the > > allocator performance of updateAllocation() doesn't repend on number of > > roles, but we should probably make this benchmark multi-role in case this > > changes in the future). Thanks for catching this. I meant to use `std::generate`. Fixed. > On Oct. 24, 2019, 7:21 a.m., Andrei Sekretenko wrote: > > src/tests/hierarchical_allocator_tests.cpp > > Lines 8528-8550 (patched) > > <https://reviews.apache.org/r/71639/diff/2/?file=2170111#file2170111line8528> > > > > Looks very similar to the loop body - can't we reuse part of that code? > > or maybe just use the last reservation of the last added framework for the > > UNRESERVE/RESERVE loop? Good point. > On Oct. 24, 2019, 7:21 a.m., Andrei Sekretenko wrote: > > src/tests/hierarchical_allocator_tests.cpp > > Lines 8606 (patched) > > <https://reviews.apache.org/r/71639/diff/2/?file=2170111#file2170111line8606> > > > > s/ranges per port resources/port range resources per reservation/ ? I think it should be "ranges per port resource", ranges are combined into one port resource. - Meng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71639/#review218386 ----------------------------------------------------------- On Oct. 28, 2019, 8:43 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71639/ > ----------------------------------------------------------- > > (Updated Oct. 28, 2019, 8:43 p.m.) > > > Review request for mesos, Andrei Sekretenko and Benjamin Mahler. > > > Bugs: MESOS-10016 > https://issues.apache.org/jira/browse/MESOS-10016 > > > Repository: mesos > > > Description > ------- > > This benchmark evaluates the performance of > `allocator->UpdateAllocation()` where the agent has various > sizes of resource reservations. > > > Diffs > ----- > > src/tests/hierarchical_allocator_tests.cpp > 38fd19cee6409e4daa5bb0ab523e8e464cdcc9a5 > > > Diff: https://reviews.apache.org/r/71639/diff/3/ > > > Testing > ------- > > On the master branch optimized build: > > ``` > [ RUN ] > ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/0 > Agent resources size: 50 (50 roles, 1 reservations per role, 1 ranges per > port resource) > 20 RESERVE operations took 1.968569746secs, each takes 98.428487ms > 20 UNRESERVE operations took 1.978470742secs, each takes 98.923537ms > [ OK ] > ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/0 > (3996 ms) > [ RUN ] > ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/1 > Agent resources size: 100 (100 roles, 1 reservations per role, 1 ranges per > port resource) > 20 RESERVE operations took 10.094084426secs, each takes 504.704221ms > 20 UNRESERVE operations took 10.293614069secs, each takes 514.680703ms > [ OK ] > ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/1 > (20584 ms) > [ RUN ] > ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/2 > Agent resources size: 200 (200 roles, 1 reservations per role, 1 ranges per > port resource) > 20 RESERVE operations took 1.16018705933333mins, each takes 3.480561178secs > 20 UNRESERVE operations took 1.17209781845mins, each takes 3.516293455secs > [ OK ] > ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/2 > (141363 ms) > ``` > > > Thanks, > > Meng Zhu > >
