Skip to content

Commit

Permalink
Merge 'treewide: use fmtlib's fmt::format_string in format() ' from K…
Browse files Browse the repository at this point in the history
…efu Chai

this series enables `seastar::format()` to use compile-time format check.

using `fmt::format_string` would allow us to perform compile-time
format checking. so in this change, we switch to `fmt::format_string`
when fmtlib's version is greater or equal to 8.0, which introduced
the compile-time checking.

also, because the `fmt::format_string` can only be instantiated with
a compile-time constexpr, the fmt string specified in test is changed
to a constexpr accordingly.

please note, this is a breaking change which might require caller to use
`seastar::format()` if the tree is compiled with libstdc++ 13 or a standard
library with `std::formt()` implemented.

Closes #1727

* github.com:scylladb/seastar:
  print: use fmtlib's fmt::format_string in format()
  rpc: do not use seastar::format() in rpc logger
  treewide: s/format/seastar::format/
  • Loading branch information
avikivity committed Sep 3, 2024
2 parents 2d64e05 + b6015b4 commit e3249a8
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 16 deletions.
4 changes: 2 additions & 2 deletions apps/io_tester/io_tester.cc
Original file line number Diff line number Diff line change
Expand Up @@ -966,7 +966,7 @@ struct convert<options> {
} else if (st == "steady") {
op.sleep_fn = timer_sleep<std::chrono::steady_clock>;
} else {
throw std::runtime_error(format("Unknown sleep_type {}", st));
throw std::runtime_error(seastar::format("Unknown sleep_type {}", st));
}
}
if (node["pause_distribution"]) {
Expand All @@ -976,7 +976,7 @@ struct convert<options> {
} else if (pd == "poisson") {
op.pause_fn = make_poisson_pause;
} else {
throw std::runtime_error(format("Unknown pause_distribution {}", pd));
throw std::runtime_error(seastar::format("Unknown pause_distribution {}", pd));
}
}
return true;
Expand Down
4 changes: 2 additions & 2 deletions include/seastar/core/print.hh
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,9 @@ log(A&&... a) {
*/
template <typename... A>
sstring
format(const char* fmt, A&&... a) {
format(fmt::format_string<A...> fmt, A&&... a) {
fmt::memory_buffer out;
fmt::format_to(fmt::appender(out), fmt::runtime(fmt), std::forward<A>(a)...);
fmt::format_to(fmt::appender(out), fmt, std::forward<A>(a)...);
return sstring{out.data(), out.size()};
}

Expand Down
4 changes: 3 additions & 1 deletion include/seastar/rpc/rpc.hh
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,9 @@ class logger {
// Ignore less severe levels in order not to spam user's log with messages during transition,
// i.e. when the user still only defines a level-less logger.
} else if (_logger && level <= log_level::info) {
_logger(format(fmt, std::forward<Args>(args)...));
fmt::memory_buffer out;
fmt::format_to(fmt::appender(out), fmt::runtime(fmt), std::forward<Args>(args)...);
_logger(sstring{out.data(), out.size()});
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/core/semaphore.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,23 +85,23 @@ static_assert(std::is_nothrow_move_constructible_v<named_semaphore>);

named_semaphore_timed_out::named_semaphore_timed_out(std::string_view msg) noexcept : _msg() {
try {
_msg = format("Semaphore timed out: {}", msg);
_msg = seastar::format("Semaphore timed out: {}", msg);
} catch (...) {
// ignore, empty _msg will generate a static message in what().
}
}

broken_named_semaphore::broken_named_semaphore(std::string_view msg) noexcept : _msg() {
try {
_msg = format("Semaphore broken: {}", msg);
_msg = seastar::format("Semaphore broken: {}", msg);
} catch (...) {
// ignore, empty _msg will generate a static message in what().
}
}

named_semaphore_aborted::named_semaphore_aborted(std::string_view msg) noexcept : _msg() {
try {
_msg = format("Semaphore aborted: {}", msg);
_msg = seastar::format("Semaphore aborted: {}", msg);
} catch (...) {
// ignore, empty _msg will generate a static message in what().
}
Expand Down
2 changes: 1 addition & 1 deletion src/json/formatter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ static bool needs_escaping(const string_view& str) {

static sstring string_view_to_json(const string_view& str) {
if (!needs_escaping(str)) {
return format("\"{}\"", str);
return seastar::format("\"{}\"", str);
}

ostringstream oss;
Expand Down
14 changes: 7 additions & 7 deletions src/net/net.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ qp::qp(bool register_copy_stats,
//
// Tx
sm::make_gauge(_queue_name + "_tx_packet_queue_last_bunch", _stats.tx.good.last_bunch,
sm::description(format("Holds a number of packets sent in the bunch. "
sm::description(seastar::format("Holds a number of packets sent in the bunch. "
"A high value in conjunction with a high value of a {} indicates an efficient Tx packets bulking.", _queue_name + "_tx_packet_queue"))),
// Rx
sm::make_gauge(_queue_name + "_rx_packet_queue_last_bunch", _stats.rx.good.last_bunch,
Expand All @@ -165,10 +165,10 @@ qp::qp(bool register_copy_stats,
//
// Tx
sm::make_counter(_queue_name + "_tx_frags", _stats.tx.good.nr_frags,
sm::description(format("Counts a number of sent fragments. Divide this value by a {} to get an average number of fragments in a Tx packet.", _queue_name + "_tx_packets"))),
sm::description(seastar::format("Counts a number of sent fragments. Divide this value by a {} to get an average number of fragments in a Tx packet.", _queue_name + "_tx_packets"))),
// Rx
sm::make_counter(_queue_name + "_rx_frags", _stats.rx.good.nr_frags,
sm::description(format("Counts a number of received fragments. Divide this value by a {} to get an average number of fragments in an Rx packet.", _queue_name + "_rx_packets"))),
sm::description(seastar::format("Counts a number of received fragments. Divide this value by a {} to get an average number of fragments in an Rx packet.", _queue_name + "_rx_packets"))),
});

if (register_copy_stats) {
Expand All @@ -178,20 +178,20 @@ qp::qp(bool register_copy_stats,
//
// Tx
sm::make_counter(_queue_name + "_tx_copy_bytes", _stats.tx.good.copy_bytes,
sm::description(format("Counts a number of sent bytes that were handled in a non-zero-copy way. Divide this value by a {} to get a portion of data sent using a non-zero-copy flow.", _queue_name + "_tx_bytes"))),
sm::description(seastar::format("Counts a number of sent bytes that were handled in a non-zero-copy way. Divide this value by a {} to get a portion of data sent using a non-zero-copy flow.", _queue_name + "_tx_bytes"))),
// Rx
sm::make_counter(_queue_name + "_rx_copy_bytes", _stats.rx.good.copy_bytes,
sm::description(format("Counts a number of received bytes that were handled in a non-zero-copy way. Divide this value by an {} to get a portion of received data handled using a non-zero-copy flow.", _queue_name + "_rx_bytes"))),
sm::description(seastar::format("Counts a number of received bytes that were handled in a non-zero-copy way. Divide this value by an {} to get a portion of received data handled using a non-zero-copy flow.", _queue_name + "_rx_bytes"))),

//
// Non-zero-copy data fragments rate: DERIVE:0:u
//
// Tx
sm::make_counter(_queue_name + "_tx_copy_frags", _stats.tx.good.copy_frags,
sm::description(format("Counts a number of sent fragments that were handled in a non-zero-copy way. Divide this value by a {} to get a portion of fragments sent using a non-zero-copy flow.", _queue_name + "_tx_frags"))),
sm::description(seastar::format("Counts a number of sent fragments that were handled in a non-zero-copy way. Divide this value by a {} to get a portion of fragments sent using a non-zero-copy flow.", _queue_name + "_tx_frags"))),
// Rx
sm::make_counter(_queue_name + "_rx_copy_frags", _stats.rx.good.copy_frags,
sm::description(format("Counts a number of received fragments that were handled in a non-zero-copy way. Divide this value by a {} to get a portion of received fragments handled using a non-zero-copy flow.", _queue_name + "_rx_frags"))),
sm::description(seastar::format("Counts a number of received fragments that were handled in a non-zero-copy way. Divide this value by a {} to get a portion of received fragments handled using a non-zero-copy flow.", _queue_name + "_rx_frags"))),

});
}
Expand Down

0 comments on commit e3249a8

Please sign in to comment.