labath added inline comments.
================ Comment at: source/Core/Log.cpp:78 + char *text; + vasprintf(&text, format, args); + message << text; ---------------- zturner wrote: > dancol wrote: > > I usually implement printf-into-std::string by using `vsnprintf` to figure > > out how many characters we generate, using `std::string::resize` to create > > a buffer of that many characters (unfortunately, zero-filling them), then > > `vsnprintf` directly into that buffer. This way, you only need one > > allocation. > > > > The currant approach involves at least three allocations: first, the string > > generated by `vasprintf`. Second, the internal `stringstream` buffer. > > Third, the copy of the buffer that `std::stringstream::str` generates. > > > > It's more expensive that it needs to be. > To be fair, we should really be deleting these methods long term, and using > `formatv`. This way you often end up with 0 allocations (for short > messages), and on top of that, only one underlying format call (as opposed to > wasting time calling `vasprintf` twice here). > The currant approach involves at least three allocations: first, the string > generated by vasprintf. Second, the internal stringstream buffer. Third, the > copy of the buffer that std::stringstream::str generates. `str()` returns a reference to the underlying buffer (this is an llvm stream, not std::stringstream), so there is no copy there. Since this is not really performance critical, I'd prefer to keep the code simpler (plus, I think the extra vsnprintf call balances the extra malloc, roughly speaking). And, same as Zachary, I hope this code will go away eventually. https://reviews.llvm.org/D27459 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
