----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50017/#review142257 -----------------------------------------------------------
include/mesos/resources.hpp (lines 135 - 147) <https://reviews.apache.org/r/50017/#comment207803> This suggests a change of semantics to `Resources`. Currently: `Resources` always refers to a valid collection resources. This function would never return Some(Error) unless we change the semantics. If we want to remove the unnecessary validation, we can do this by making '`operator += (const Resources& that)`' avoid validating '`that`' since it's already valid. Currently it just calls '`operator += (const Resource& r)`' which is unaware of '`r`' already being valid since it comes from a '`Resources`'. However, we have to keep the validation in '`operator += (const Resource& r)`', since '`r`' is just a protobuf coming from an arbitrary caller, it may be invalid and we need to validate. Make sense? - Benjamin Mahler On July 14, 2016, 3:59 p.m., Guangya Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50017/ > ----------------------------------------------------------- > > (Updated July 14, 2016, 3:59 p.m.) > > > Review request for mesos, Benjamin Mahler and Klaus Ma. > > > Repository: mesos > > > Description > ------- > > The "validation" API was called in a huge number of times, but this > was only needed when parsing resources and we do not need to do > other validation when doing resources operations, such as add etc. > > > Diffs > ----- > > include/mesos/resources.hpp a557e97c65194d4aad879fb88d8edefd1c95b8d8 > include/mesos/v1/resources.hpp a5ba8fec4c9c3643646308f75a4b28cefe0b3df3 > src/common/resources.cpp f6ff92b591c15bc8e93fd85e1896349c3a7bb968 > src/v1/resources.cpp 8c3f2d1c1529915a59d47fe37bb3fc7a3267079a > > Diff: https://reviews.apache.org/r/50017/diff/ > > > Testing > ------- > > The `qcachegrind` result here: > https://docs.google.com/document/d/1oilen04e8trIOgYbj-jxAd7esTRDaySll8y2BQ6p5nU/edit?usp=sharing > > After the fix, the performance of `adding` resources increased 30+% when > adding 50000 agents to the cluster. > > Before fix: > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/30 > Using 50000 agents and 1 clients > Added 1 clients in 47us > **Added 50000 agents in 1.312497secs** > Sorted 1 clients in 43us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/30 (1321 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/31 > Using 50000 agents and 50 clients > Added 50 clients in 948us > **Added 50000 agents in 1.325987secs** > Sorted 50 clients in 1165us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/31 (1340 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/32 > Using 50000 agents and 100 clients > Added 100 clients in 1697us > **Added 50000 agents in 1.409478secs** > Sorted 100 clients in 2876us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/32 (1432 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/33 > Using 50000 agents and 200 clients > Added 200 clients in 4553us > **Added 50000 agents in 1.371473secs** > Sorted 200 clients in 5371us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/33 (1412 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/34 > Using 50000 agents and 500 clients > Added 500 clients in 8836us > **Added 50000 agents in 1.304245secs** > Sorted 500 clients in 14697us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/34 (1387 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35 > Using 50000 agents and 1000 clients > Added 1000 clients in 19508us > **Added 50000 agents in 1.270555secs** > Sorted 1000 clients in 32575us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35 (1433 ms) > > > After the fix: > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/30 > Using 50000 agents and 1 clients > Added 1 clients in 42us > **Added 50000 agents in 891266us** > Sorted 1 clients in 59us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/30 (902 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/31 > Using 50000 agents and 50 clients > Added 50 clients in 933us > **Added 50000 agents in 885006us** > Sorted 50 clients in 1220us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/31 (899 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/32 > Using 50000 agents and 100 clients > Added 100 clients in 1879us > **Added 50000 agents in 903112us** > Sorted 100 clients in 2800us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/32 (922 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/33 > Using 50000 agents and 200 clients > Added 200 clients in 3893us > **Added 50000 agents in 881240us** > Sorted 200 clients in 5802us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/33 (912 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/34 > Using 50000 agents and 500 clients > Added 500 clients in 10712us > **Added 50000 agents in 877442us** > Sorted 500 clients in 17887us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/34 (949 ms) > [ RUN ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35 > Using 50000 agents and 1000 clients > Added 1000 clients in 21472us > **Added 50000 agents in 916653us** > Sorted 1000 clients in 37369us > [ OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35 (1057 ms) > > > Thanks, > > Guangya Liu > >
