> On April 8, 2014, 12:03 p.m., Ben Mahler wrote: > > Please add 'benh' to the reviewers on this one as well. > > > > As discussed, would it be beneficial to also time individual operations on > > a full Registry in the benchmarks? That way, we could more closely measure > > the penalty we're taking from the copying in a single batch. > > > > How long does copying take for 50,000 slaves? More importantly, I'm curious > > how much benefit this code change is providing via the benchmarks (i.e. how > > many copies are avoided per batch).
Um... so I stand corrected about the performance improvement. All these copies seem to have been elided. For this line: Registry registry = variable.get().get(); // Option<Variable<Registry> > variable; Copying a registry with 50,000 slaves costs about 95ms on my testing machine, with or without this patch. In my tests returning a copy actually outperformed returning a const ref. In 100 such tests, returning copies each costs 3ms less on avg, and collectively 0.3secs less. Maybe there are optimizations that made it better than returning const refs. In terms of the total time for the registrar to apply a single operation (including storage time) one-by-one, returning copy is also faster (6.45min vs 6.73mins for 100 operations/InMemoryStorage with no batching). So... I like the explicitness of returning const ref and that it allows the caller to decide whether to induce a copy but we seem to not be doing it for performance reasons here :) - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19764/#review39813 ----------------------------------------------------------- On April 11, 2014, 10:06 a.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19764/ > ----------------------------------------------------------- > > (Updated April 11, 2014, 10:06 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. > > > Bugs: MESOS-1155 > https://issues.apache.org/jira/browse/MESOS-1155 > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/state/protobuf.hpp 802617b48586d8fc682d5efda3e2b86205e988b6 > src/state/state.hpp bebfe02dcdcbaec8742d173e2233817288722bd8 > > Diff: https://reviews.apache.org/r/19764/diff/ > > > Testing > ------- > > mesos-tests.sh –verbose –gtest_filter=“RegistrarTest*” > –gtest_also_run_disabled_tests > On my test machine it costs about 40-60ms to copy a registry of 20,000 slaves. > > > Thanks, > > Jiang Yan Xu > >
