----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53929/#review156494 -----------------------------------------------------------
I pushed the change since this is cleaner and it looks like you ran the benchmark. Any reason you didn't include the results? Do we have benchmark coverage of the quota code paths? If not, did you write a small custom benchmark just to confirm this is indeed faster? - Benjamin Mahler On Nov. 19, 2016, 2:44 p.m., Guangya Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53929/ > ----------------------------------------------------------- > > (Updated Nov. 19, 2016, 2:44 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > In allocator `getQuotaRoleAllocatedResources`, it is trying to omits > dynamic reservation, persistent volume info and additionally strip > `role` in it. But it is traversing all `Resource` object from the > `Resources` object, strip the `role` and then using `+=` to add each > `Resource` object back. > > The `+=` for `Resource` object will invoke `validate` which will impact > the performance. This fix is calling `flatten()` directly to strip the > `role` for the all of the `Resource` objects and this will not invoke > `validate`. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.cpp > c8f9492ee1b69e125a1e841116d22a578a9b524e > > Diff: https://reviews.apache.org/r/53929/diff/ > > > Testing > ------- > > make > make check > > ``` > ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" > ./bin/mesos-tests.sh > --gtest_filter="*HierarchicalAllocator_BENCHMARK_Test.*/0" --benchmark > ``` > > > Thanks, > > Guangya Liu > >
