From 3133ecdd6599469672f1d7cbffdd92606f2b132c Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Tue, 10 Dec 2024 15:01:54 +0800 Subject: [PATCH] util/backtrace: Optimize formatter to reduce memory allocation overhead This commit addresses a critical memory allocation issue in backtrace formatting by directly specializing fmt::formatter for backtrace types, eliminating dependency on iostream-based formatting. Problem: When Seastar applications experience high memory pressure, logging backtrace information could fail due to additional memory allocation required by iostream formatting. This resulted in errors like: ``` ERROR 2024-12-10 01:59:16,905 [shard 0:main] seastar_memory - seastar/src/core/memory.cc:2126 @void seastar::memory::maybe_dump_memory_diagnostics(size_t, bool): failed to log message: fmt='Failed to allocate {} bytes at {}': std::__ios_failure (error iostream:1, basic_ios::clear: iostream error)" ``` Solution: - Implement direct fmt::formatter specialization for backtrace - Remove reliance on operator<< for string representation - Reduce memory allocation pressure during out-of-memory scenarios - Improve reliability of backtrace logging under extreme memory constraints Compatibility Improvements: - Ensure consistent `fmt::formatter` availability across fmt library versions - Deprecate two `simple_backtrace` constructors previously added in 095a07b955, as they are no longer needed with fmt::join - Maintain robust formatting approach independent of library version Impact: This change enhances system resilience by enabling backtrace logging even under significant memory pressure, providing more reliable post-mortem debugging information. Signed-off-by: Kefu Chai --- include/seastar/util/backtrace.hh | 36 +++++++++++------ src/util/backtrace.cc | 65 ++++++++++++++++++++++--------- 2 files changed, 71 insertions(+), 30 deletions(-) diff --git a/include/seastar/util/backtrace.hh b/include/seastar/util/backtrace.hh index 5a4dbad54e6..ec4b9597348 100644 --- a/include/seastar/util/backtrace.hh +++ b/include/seastar/util/backtrace.hh @@ -84,18 +84,20 @@ public: using vector_type = boost::container::static_vector; private: vector_type _frames; - size_t _hash; - char _delimeter; + size_t _hash = 0; + char _delimeter = ' '; private: size_t calculate_hash() const noexcept; public: - simple_backtrace(vector_type f, char delimeter = ' ') noexcept : _frames(std::move(f)), _hash(calculate_hash()), _delimeter(delimeter) {} - simple_backtrace(char delimeter = ' ') noexcept : simple_backtrace({}, delimeter) {} + simple_backtrace(vector_type f) noexcept : _frames(std::move(f)), _hash(calculate_hash()) {} + simple_backtrace() noexcept = default; + [[deprecated]] simple_backtrace(vector_type f, char delimeter) : _frames(std::move(f)), _hash(calculate_hash()), _delimeter(delimeter) {} + [[deprecated]] simple_backtrace(char delimeter) : _delimeter(delimeter) {} size_t hash() const noexcept { return _hash; } char delimeter() const noexcept { return _delimeter; } - friend std::ostream& operator<<(std::ostream& out, const simple_backtrace&); + friend fmt::formatter; bool operator==(const simple_backtrace& o) const noexcept { return _hash == o._hash && _frames == o._frames; @@ -116,7 +118,7 @@ public: : _task_type(&ti) { } - friend std::ostream& operator<<(std::ostream& out, const task_entry&); + friend fmt::formatter; bool operator==(const task_entry& o) const noexcept { return *_task_type == *o._task_type; @@ -151,7 +153,7 @@ public: size_t hash() const noexcept { return _hash; } char delimeter() const noexcept { return _main.delimeter(); } - friend std::ostream& operator<<(std::ostream& out, const tasktrace&); + friend fmt::formatter; bool operator==(const tasktrace& o) const noexcept; @@ -182,10 +184,22 @@ struct hash { } -#if FMT_VERSION >= 90000 -template <> struct fmt::formatter : fmt::ostream_formatter {}; -template <> struct fmt::formatter : fmt::ostream_formatter {}; -#endif +template <> struct fmt::formatter { + constexpr auto parse(format_parse_context& ctx) { return ctx.begin(); } + auto format(const seastar::frame&, fmt::format_context& ctx) const -> decltype(ctx.out()); +}; +template <> struct fmt::formatter { + constexpr auto parse(format_parse_context& ctx) { return ctx.begin(); } + auto format(const seastar::simple_backtrace&, fmt::format_context& ctx) const -> decltype(ctx.out()); +}; +template <> struct fmt::formatter { + constexpr auto parse(format_parse_context& ctx) { return ctx.begin(); } + auto format(const seastar::tasktrace&, fmt::format_context& ctx) const -> decltype(ctx.out()); +}; +template <> struct fmt::formatter { + constexpr auto parse(format_parse_context& ctx) { return ctx.begin(); } + auto format(const seastar::task_entry&, fmt::format_context& ctx) const -> decltype(ctx.out()); +}; namespace seastar { diff --git a/src/util/backtrace.cc b/src/util/backtrace.cc index cadf841e983..9b3a0bc9441 100644 --- a/src/util/backtrace.cc +++ b/src/util/backtrace.cc @@ -33,6 +33,7 @@ module; #include #include #include +#include #ifdef SEASTAR_MODULE module seastar; @@ -111,37 +112,23 @@ size_t simple_backtrace::calculate_hash() const noexcept { } std::ostream& operator<<(std::ostream& out, const frame& f) { - if (!f.so->name.empty()) { - out << f.so->name << "+"; - } - out << format("0x{:x}", f.addr); + fmt::print(out, "{}", f); return out; } std::ostream& operator<<(std::ostream& out, const simple_backtrace& b) { - char delim[2] = {'\0', '\0'}; - for (auto f : b._frames) { - out << delim << f; - delim[0] = b.delimeter(); - } + fmt::print(out, "{}", b); return out; } std::ostream& operator<<(std::ostream& out, const tasktrace& b) { - out << b._main; - for (auto&& e : b._prev) { - out << "\n --------"; - std::visit(make_visitor([&] (const shared_backtrace& sb) { - out << '\n' << sb; - }, [&] (const task_entry& f) { - out << "\n " << f; - }), e); - } + fmt::print(out, "{}", b); return out; } std::ostream& operator<<(std::ostream& out, const task_entry& e) { - return out << seastar::pretty_type_name(*e._task_type); + fmt::print(out, "{}", e); + return out; } tasktrace current_tasktrace() noexcept { @@ -195,3 +182,43 @@ bool tasktrace::operator==(const tasktrace& o) const noexcept { tasktrace::~tasktrace() {} } // namespace seastar + +namespace fmt { + +auto formatter::format(const seastar::frame& f, format_context& ctx) const + -> decltype(ctx.out()) { + auto out = ctx.out(); + if (!f.so->name.empty()) { + out = fmt::format_to(out, "{}+", f.so->name); + } + return fmt::format_to(out, "0x{:x}", f.addr); +} + +auto formatter::format(const seastar::simple_backtrace& b, format_context& ctx) const + -> decltype(ctx.out()) { + return fmt::format_to(ctx.out(), "{}", fmt::join(b._frames, " ")); +} + +auto formatter::format(const seastar::tasktrace& b, format_context& ctx) const + -> decltype(ctx.out()) { + auto out = ctx.out(); + out = fmt::format_to(out, "{}", b._main); + for (auto&& e : b._prev) { + out = fmt::format_to(out, "\n --------"); + out = std::visit(seastar::make_visitor( + [&] (const seastar::shared_backtrace& sb) { + return fmt::format_to(out, "\n{}", sb); + }, + [&] (const seastar::task_entry& f) { + return fmt::format_to(out, "\n {}", f); + }), e); + } + return out; +} + +auto formatter::format(const seastar::task_entry& e, format_context& ctx) const + -> decltype(ctx.out()) { + return fmt::format_to(ctx.out(), "{}", seastar::pretty_type_name(*e._task_type)); +} + +}