> On Aug. 8, 2017, 4:35 p.m., Benno Evers wrote: > > src/examples/balloon_framework.cpp > > Line 520 (original), 524 (patched) > > <https://reviews.apache.org/r/61111/diff/6/?file=1790499#file1790499line525> > > > > No real issue, but I find it curious that our style guide basically > > forces us to make redundant string temporaries here, by insisting on > > `constexpr char[]` over `const string`-literals. Seems like a bug in the > > style guide?
One main issue when using static `std::string`s would be that `std::string` has a non-trivial destructor which makes it hard to reason about behavior on library unloading or shutdown in general. The Google style guide (on which the Mesos style guide is based) gives some more detail, https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables While we probably do not do enough of a job of preventing calling likely expensive (copy) constructors, I believe that the performance impact in this particular case is small (code is called once in `main`). I also suspect that we already incur a couple orders of magnitude more string (copy) constructors through other temporary objects created. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61111/#review182390 ----------------------------------------------------------- On Aug. 7, 2017, 2:50 p.m., Armand Grillet wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61111/ > ----------------------------------------------------------- > > (Updated Aug. 7, 2017, 2:50 p.m.) > > > Review request for mesos, Alexander Rukletsov and Till Toenshoff. > > > Bugs: MESOS-7814 > https://issues.apache.org/jira/browse/MESOS-7814 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/examples/balloon_framework.cpp dfd049b860345adc33c3e774dcdff0320da107f6 > src/examples/disk_full_framework.cpp > 215b6d29cf5f52973f2b211e9fc72ca6ec2afa7a > src/examples/docker_no_executor_framework.cpp > 4a58f11fc8892f23ade1d8c872ab9b4fc580d478 > src/examples/dynamic_reservation_framework.cpp > f3b1c8c4d2684e827fc10776fef4f2287e315b85 > src/examples/load_generator_framework.cpp > abb70f42a3a755afaa1d56b1491058b90958f030 > src/examples/long_lived_framework.cpp > 2a79dfd3f81e5254922895af45c2b72d80ce5f49 > src/examples/no_executor_framework.cpp > 7d841c6f364e1b671ec829aa9bff3b1f8ecf55ef > src/examples/persistent_volume_framework.cpp > 17140294a1eaeeeb92e9e14fc8638182b3a0682e > src/examples/test_framework.cpp 9dbc18b039ee13cc1ec9454bd41cb3cfe30d63b4 > src/examples/test_http_framework.cpp > a0cd84ab6dfdd8d6ed3403c6e06fabae6d5b46c9 > > > Diff: https://reviews.apache.org/r/61111/diff/6/ > > > Testing > ------- > > $ make check > > > Thanks, > > Armand Grillet > >
