-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68731/#review208774
-----------------------------------------------------------



I am honestly surprised by the improvement this patch brings. Did not expect 
the hashmap has such high overhead, given the keys are short strings. But 
numbers are numbers :)


src/master/allocator/sorter/sorter.hpp
Lines 163-164 (patched)
<https://reviews.apache.org/r/68731/#comment292956>

    Why we are enforcing alphabetically sorted? Seems to me unnecessary, 
especially we do not force it.



src/master/allocator/sorter/sorter.hpp
Lines 176 (patched)
<https://reviews.apache.org/r/68731/#comment292955>

    If the value is zero, we still return `true`? That is quite 
counter-intuitive.
    
    Also, consider using `find_if` for simplicity. Ditt below.



src/master/allocator/sorter/sorter.hpp
Lines 194 (patched)
<https://reviews.apache.org/r/68731/#comment292951>

    I think it is a good idea to print the whole vector as well.


- Meng Zhu


On Sept. 18, 2018, 2:20 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68731/
> -----------------------------------------------------------
> 
> (Updated Sept. 18, 2018, 2:20 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Meng Zhu.
> 
> 
> Bugs: MESOS-9239
>     https://issues.apache.org/jira/browse/MESOS-9239
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This type replaces the use of hashmaps keyed by resource names in
> favor of storing vectors of `pair<string,Value::Scalar>`, in order
> to avoid the performance penalty of using hashmaps.
> 
> Running *HierarchicalAllocator_BENCHMARK_Test.DeclineOffers/21 shows
> the following improvement:
> 
> Using 10000 agents and 1000 frameworks
> Added 1000 frameworks in 42.49ms -> 42.85ms (no change)
> Added 10000 agents in 7.69secs -> 4.89secs (normalized: 1 -> 0.64)
> round 0 allocate() took 5.42secs -> 3.53secs (nomralized: 1 -> 0.65)
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 5a4fa5e2dca61168923261230b1f5c245354cbb7 
>   src/master/allocator/sorter/random/sorter.hpp 
> 7f6c0de70e3ae03d7362fb9e140b93435e530499 
>   src/master/allocator/sorter/sorter.hpp 
> 432ccfe24ed2854df9cc186a8691009cbdb763c7 
> 
> 
> Diff: https://reviews.apache.org/r/68731/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Reply via email to