----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71639/#review218386 -----------------------------------------------------------
Many thanks for this benchmark! Even in its WIP version it already helped to understand what is wrong with `updateAllocation()`. src/tests/hierarchical_allocator_tests.cpp Lines 8414 (patched) <https://reviews.apache.org/r/71639/#comment306089> 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? src/tests/hierarchical_allocator_tests.cpp Lines 8440 (patched) <https://reviews.apache.org/r/71639/#comment306090> Why not just use UUIDs? src/tests/hierarchical_allocator_tests.cpp Lines 8455 (patched) <https://reviews.apache.org/r/71639/#comment306093> const ? src/tests/hierarchical_allocator_tests.cpp Lines 8464 (patched) <https://reviews.apache.org/r/71639/#comment306091> 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). src/tests/hierarchical_allocator_tests.cpp Lines 8473 (patched) <https://reviews.apache.org/r/71639/#comment306092> emplace(framework.id(), Resources()) ? src/tests/hierarchical_allocator_tests.cpp Lines 8528-8550 (patched) <https://reviews.apache.org/r/71639/#comment306094> 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? src/tests/hierarchical_allocator_tests.cpp Lines 8606 (patched) <https://reviews.apache.org/r/71639/#comment306096> s/ranges per port resources/port range resources per reservation/ ? src/tests/hierarchical_allocator_tests.cpp Lines 8608-8611 (patched) <https://reviews.apache.org/r/71639/#comment306095> I would suggest `RESERVE operations` and `UNRESERVE operations` - we have terms for them, don't we? - Andrei Sekretenko On Oct. 24, 2019, 2:56 a.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71639/ > ----------------------------------------------------------- > > (Updated Oct. 24, 2019, 2:56 a.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/2/ > > > 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 resources) > 20 reservation operations took 442.402455ms, each takes 22.120122ms > 20 Unreservation operations took 464.085487ms, each takes 23.204274ms > > [ RUN ] > ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/1 > Agent resources size: 100 (100 roles, 1 reservations per role, 1 ranges per > port resources) > 20 reservation operations took 1.321309035secs, each takes 66.065451ms > 20 Unreservation operations took 1.311244246secs, each takes 65.562212ms > > [ RUN ] > ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/2 > Agent resources size: 200 (200 roles, 1 reservations per role, 1 ranges per > port resources) > 20 reservation operations took 4.129166934secs, each takes 206.458346ms > 20 Unreservation operations took 4.041358384secs, each takes 202.067919ms > > [ RUN ] > ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/3 > Agent resources size: 400 (400 roles, 1 reservations per role, 1 ranges per > port resources) > 20 reservation operations took 15.011075671secs, each takes 750.553783ms > 20 Unreservation operations took 15.41327483secs, each takes 770.663741ms > > [ RUN ] > ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/4 > Agent resources size: 600 (600 roles, 1 reservations per role, 1 ranges per > port resources) > 20 reservation operations took 31.159754828secs, each takes 1.557987741secs > 20 Unreservation operations took 30.69454178secs, each takes 1.534727089secs > ``` > > > Thanks, > > Meng Zhu > >
