> On Aug. 13, 2019, 7:58 p.m., Benjamin Mahler wrote: > > src/master/allocator/mesos/hierarchical.hpp > > Lines 231 (patched) > > <https://reviews.apache.org/r/71269/diff/2/?file=2160787#file2160787line231> > > > > This can be `hashmap<std::string, Role>` which would avoid the > > potential leak of forgetting `delete`, and avoid the extra memory > > allocation. > > Meng Zhu wrote: > hmm, would prefer keep the map just as a simple lookup table. Feels > somewhat convoluted if also using it to manage the life cycle. > The simple destructors should be enough to guard against any leaks. > Also, the "get" call would need to return a coy of `Role` instead of a > pointer, not quite what we want. > > Benjamin Mahler wrote: > > hmm, would prefer keep the map just as a simple lookup table. Feels > somewhat convoluted if also using it to manage the life cycle. > > The map is already managing lifecycle? No `Role*` should exist outside > the map, right? If we remove a `Role*` from the map, it needs to be > `delete`ed right? > > > Also, the "get" call would need to return a coy of Role instead of a > pointer, not quite what we want. > > Why would you need to copy? You can just return a pointer to the value > from the map, that's no problem?
OK, didn't think deep about it. Updated the code to use the map for life cycle management. - Meng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71269/#review217192 ----------------------------------------------------------- On Aug. 15, 2019, 8:36 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71269/ > ----------------------------------------------------------- > > (Updated Aug. 15, 2019, 8:36 p.m.) > > > Review request for mesos, Andrei Sekretenko and Benjamin Mahler. > > > Bugs: MESOS-9917 > https://issues.apache.org/jira/browse/MESOS-9917 > > > Repository: mesos > > > Description > ------- > > The role concept in Mesos fits into a tree structure naturally. > However, the role state in the allocator are currenstored > in a hashmap. This is less efficient and harder to use and reason. > > This patch introduced a `RoleTree` structure in the allocator > and organizes all the roles in to a tree. This should simplify > the code logic and opens further refactor and optimization > opportunities. > > In addition, the master code also lacks a proper tree structure > for tracking roles. We should leverage the same role tree code > here to simplify that as well. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.hpp > 8be8dcee04e8fc5b97f730b2f058d14c81678788 > src/master/allocator/mesos/hierarchical.cpp > 580d35a3b71c1f7e851fa0504c6b9f037c05c378 > > > Diff: https://reviews.apache.org/r/71269/diff/4/ > > > Testing > ------- > > make check > > Benchmaring using optimized build shows no noticable performance change. > > Benchmark: `BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota` > > ## Before: > > Added 30 agents in 1.252621ms > Added 30 frameworks in 7.747202ms > Benchmark setup: 30 agents, 30 roles, 30 frameworks, with drf sorter > Made 36 allocations in 12.791427ms > > Added 300 agents in 9.765295ms > Added 300 frameworks in 273.978325ms > Benchmark setup: 300 agents, 300 roles, 300 frameworks, with drf sorter > Made 350 allocations in 424.603736ms > > Added 3000 agents in 79.646516ms > Added 3000 frameworks in 19.17449514secs > Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with drf sorter > Made 3500 allocations in 31.687207066secs > > ## After: > > Added 30 agents in 1.376619ms > Added 30 frameworks in 7.647487ms > Benchmark setup: 30 agents, 30 roles, 30 frameworks, with drf sorter > Made 36 allocations in 11.638116ms > > Added 300 agents in 9.506101ms > Added 300 frameworks in 263.008198ms > Benchmark setup: 300 agents, 300 roles, 300 frameworks, with drf sorter > Made 350 allocations in 418.962396ms > > Added 3000 agents in 81.553527ms > Added 3000 frameworks in 19.201708305secs > Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with drf sorter > Made 3500 allocations in 31.583417136secs > > > Thanks, > > Meng Zhu > >
