> On July 20, 2018, 2:29 p.m., Michael Park wrote: > > Looks good! > > > > It might be worth considering using the `OStreamWrapper` stuff for the > > `ostream` API. > > I know writing to `StringBuffer` is faster than writing to > > `OStreamWrapper`, but > > I think just writing to `OStreamWrapper` would be faster than writing to > > `StringBuffer` > > then copying that into a `std::string` then writing that to `ostream` in > > the end anyway. > > Benjamin Mahler wrote: > > I think just writing to OStreamWrapper would be faster than writing to > StringBuffer > > That had seemed doubtful to me based on their documentation: > http://rapidjson.org/md_doc_stream.html#iostreamWrapper > > I would be interesting to get the numbers! However, we don't write to > ostream in the end any longer: > https://reviews.apache.org/r/67992/ > > I could remove the `<<` operator but I figured I would just leave it > untouched for now.
Yeah, you left out the second half of my sentence but I assume it wasn't intentional. I didn't notice the patch to replace `ostringstream`. Sounds good to me! > On July 20, 2018, 2:29 p.m., Michael Park wrote: > > 3rdparty/stout/include/stout/jsonify.hpp > > Line 137 (original), 95 (patched) > > <https://reviews.apache.org/r/67988/diff/1/?file=2061761#file2061761line158> > > > > Looks like `GetString` returns a `const char*`. We should provide the > > length here: `{buffer.GetString(), buffer.GetSize()}`. > > Benjamin Mahler wrote: > Ah nice catch! This should avoid walking the string right? I'll re-run > the numbers. Yep, exactly. - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67988/#review206290 ----------------------------------------------------------- On July 19, 2018, 8:38 p.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67988/ > ----------------------------------------------------------- > > (Updated July 19, 2018, 8:38 p.m.) > > > Review request for mesos, Alexander Rukletsov, Benno Evers, and Michael Park. > > > Bugs: MESOS-9092 > https://issues.apache.org/jira/browse/MESOS-9092 > > > Repository: mesos > > > Description > ------- > > This reduces the time needed for the client to finish receiving a > master's /state response by 50% in the `StateQuery` benchmark: > > min q1 q3 max > baseline 6.52 6.76 7.33 8.26 > rapidjson w/ SIMD 3.48 3.54 4.12 4.4 > rapidjson 3.29 3.32 3.65 3.85 > > SIMD is left disabled for now since it showed slightly slower > results. > > > Diffs > ----- > > 3rdparty/stout/include/stout/jsonify.hpp > 2314980e185ee61cc2ea54f1b4d2a8b35e58121c > > > Diff: https://reviews.apache.org/r/67988/diff/2/ > > > Testing > ------- > > Tested at the end of this chain, since this is split across > stout/libprocess/mesos. > > > Thanks, > > Benjamin Mahler > >
