wasphin commented on PR #3534: URL: https://github.com/apache/brpc/pull/3534#issuecomment-5630140176
Thanks for the cleanup. However, I think we need more benchmark results before making this change. Replacing an existing implementation with std::to_string simply because the standard library provides it is not, by itself, a sufficient reason to change. We should demonstrate that the replacement is at least comparable in performance and does not introduce regressions on the toolchains we support. In particular, the implementation of std::to_string varies across libstdc++ versions. Modern versions have optimized integer conversion, while older versions may implement it through vsnprintf, which can be significantly slower than the current butil implementation. Could you please provide benchmarks covering representative integer types/values and, ideally, both older and newer supported GCC/libstdc++ versions? This would give us enough evidence to evaluate the trade-off rather than making the switch solely for API modernization. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
