From e4463b11af804cff5fbd2fa96d6b7b35b857034a Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Tue, 31 Dec 2024 13:48:36 +0800 Subject: [PATCH] treewide: replace boost::algorithm::join() with fmt::join() Replace usages of `boost::algorithm::join()` with `fmt::join()` to improve performance and reduce dependency on Boost. `fmt::join()` allows direct formatting of ranges and tuples with custom separators without creating intermediate strings. When formatting comma-separated values into another string, fmt::join() avoids the overhead of temporary string creation that `boost::algorithm::join()` requires. This change also helps streamline our dependencies by leveraging the existing fmt library instead of Boost.Algorithm. To avoid the ambiguity, some caller sites were updated to call `seastar::format()` explicitly. See also - boost::algorithm::join(): https://www.boost.org/doc/libs/1_87_0/doc/html/string_algo/reference.html#doxygen.join_8hpp - fmt::join(): https://fmt.dev/11.0/api/#ranges-api Signed-off-by: Kefu Chai Closes scylladb/scylladb#22082 --- auth/default_authorizer.cc | 1 - auth/resource.cc | 3 +-- auth/service.cc | 6 ++---- auth/standard_role_manager.cc | 3 +-- cql3/statements/authorization_statement.cc | 3 --- cql3/util.cc | 2 -- db/view/view.cc | 5 ++--- index/secondary_index.cc | 1 - main.cc | 7 +++---- service/qos/service_level_controller.cc | 5 ++--- service/raft/raft_group0_client.cc | 3 +-- service/storage_service.cc | 1 - test/boost/cql_query_test.cc | 1 - test/boost/user_types_test.cc | 1 - test/lib/data_model.cc | 4 +--- test/lib/mutation_source_test.cc | 1 - test/lib/random_schema.cc | 11 +++++------ test/perf/perf_fast_forward.cc | 1 - tools/lua_sstable_consumer.cc | 1 - tools/scylla-nodetool.cc | 1 - tools/scylla-sstable.cc | 1 - tools/scylla-types.cc | 4 +--- 22 files changed, 19 insertions(+), 47 deletions(-) diff --git a/auth/default_authorizer.cc b/auth/default_authorizer.cc index 260896e989..13fa66abfc 100644 --- a/auth/default_authorizer.cc +++ b/auth/default_authorizer.cc @@ -16,7 +16,6 @@ extern "C" { #include } -#include #include #include #include diff --git a/auth/resource.cc b/auth/resource.cc index 0e486bd661..6e6d26707c 100644 --- a/auth/resource.cc +++ b/auth/resource.cc @@ -15,7 +15,6 @@ #include #include -#include #include #include @@ -148,7 +147,7 @@ resource::resource(functions_resource_t, std::string_view keyspace, std::string_ } sstring resource::name() const { - return boost::algorithm::join(_parts, "/"); + return fmt::to_string(fmt::join(_parts, "/")); } std::optional resource::parent() const { diff --git a/auth/service.cc b/auth/service.cc index 94174b0f9e..18c6f798b8 100644 --- a/auth/service.cc +++ b/auth/service.cc @@ -14,7 +14,6 @@ #include "auth/service.hh" #include -#include #include #include @@ -875,7 +874,6 @@ future<> migrate_to_auth_v2(db::system_keyspace& sys_ks, ::service::raft_group0_ for (const auto& col : schema->all_columns()) { col_names.push_back(col.name_as_cql_string()); } - auto col_names_str = boost::algorithm::join(col_names, ", "); sstring val_binders_str = "?"; for (size_t i = 1; i < col_names.size(); ++i) { val_binders_str += ", ?"; @@ -891,10 +889,10 @@ future<> migrate_to_auth_v2(db::system_keyspace& sys_ks, ::service::raft_group0_ } } auto muts = co_await qp.get_mutations_internal( - format("INSERT INTO {}.{} ({}) VALUES ({})", + seastar::format("INSERT INTO {}.{} ({}) VALUES ({})", db::system_keyspace::NAME, cf_name, - col_names_str, + fmt::join(col_names, ", "), val_binders_str), internal_distributed_query_state(), ts, diff --git a/auth/standard_role_manager.cc b/auth/standard_role_manager.cc index 8a3d4df430..867a481ff9 100644 --- a/auth/standard_role_manager.cc +++ b/auth/standard_role_manager.cc @@ -12,7 +12,6 @@ #include #include -#include #include #include #include @@ -330,7 +329,7 @@ standard_role_manager::alter(std::string_view role_name, const role_config_updat assignments.push_back(sstring("can_login = ") + (*u.can_login ? "true" : "false")); } - return boost::algorithm::join(assignments, ", "); + return fmt::to_string(fmt::join(assignments, ", ")); }; return require_record(_qp, role_name).then([this, role_name, &u, &mc](record) { diff --git a/cql3/statements/authorization_statement.cc b/cql3/statements/authorization_statement.cc index a8fccdc8a9..ea3ca4712f 100644 --- a/cql3/statements/authorization_statement.cc +++ b/cql3/statements/authorization_statement.cc @@ -13,9 +13,6 @@ #include "auth/resource.hh" #include "cql3/query_processor.hh" #include "exceptions/exceptions.hh" -#include -#include -#include #include "db/cql_type_parser.hh" #include "auth/common.hh" diff --git a/cql3/util.cc b/cql3/util.cc index c1105c1720..190b928d68 100644 --- a/cql3/util.cc +++ b/cql3/util.cc @@ -8,8 +8,6 @@ #include "util.hh" #include "cql3/expr/expr-utils.hh" -#include - #ifdef DEBUG #include diff --git a/db/view/view.cc b/db/view/view.cc index 57ef4958d1..dd68531e64 100644 --- a/db/view/view.cc +++ b/db/view/view.cc @@ -16,7 +16,6 @@ #include #include -#include #include #include @@ -2656,7 +2655,7 @@ future<> view_builder::migrate_to_v2(locator::token_metadata_ptr tmptr, db::syst co_await utils::get_local_injector().inject("view_builder_pause_in_migrate_v2", utils::wait_for_message(5min)); auto col_names = schema->all_columns() | std::views::transform([] (const auto& col) {return col.name_as_cql_string(); }) | std::ranges::to>(); - auto col_names_str = boost::algorithm::join(col_names, ", "); + auto col_names_str = fmt::to_string(fmt::join(col_names, ", ")); sstring val_binders_str = "?"; for (size_t i = 1; i < col_names.size(); ++i) { val_binders_str += ", ?"; @@ -2698,7 +2697,7 @@ future<> view_builder::migrate_to_v2(locator::token_metadata_ptr tmptr, db::syst auto row_ts = row.get_as("ts"); auto muts = co_await qp.get_mutations_internal( - format("INSERT INTO {}.{} ({}) VALUES ({})", + seastar::format("INSERT INTO {}.{} ({}) VALUES ({})", db::system_keyspace::NAME, db::system_keyspace::VIEW_BUILD_STATUS_V2, col_names_str, diff --git a/index/secondary_index.cc b/index/secondary_index.cc index c96e013413..a5494ee068 100644 --- a/index/secondary_index.cc +++ b/index/secondary_index.cc @@ -13,7 +13,6 @@ #include "cql3/statements/index_target.hh" #include -#include #include #include "exceptions/exceptions.hh" diff --git a/main.cc b/main.cc index cd4da43c52..8b73496bfc 100644 --- a/main.cc +++ b/main.cc @@ -118,8 +118,6 @@ #include "utils/shared_dict.hh" #include "message/dictionary_service.hh" -#include - seastar::metrics::metric_groups app_metrics; using namespace std::chrono_literals; @@ -533,8 +531,9 @@ std::string format_parsed_options(const std::vector& opts) { return opt.string_key; } - return (opt.string_key.empty() ? "(positional) " : fmt::format("{}: ", opt.string_key)) + - boost::algorithm::join(opt.value, " "); + return fmt::format("{}{}", + opt.string_key.empty() ? "(positional) " : fmt::format("{}: ", opt.string_key), + fmt::join(opt.value, " ")); }), ", ") ); } diff --git a/service/qos/service_level_controller.cc b/service/qos/service_level_controller.cc index 3399c8ab4a..f5ba500ba0 100644 --- a/service/qos/service_level_controller.cc +++ b/service/qos/service_level_controller.cc @@ -8,7 +8,6 @@ #include "cql3/util.hh" #include "utils/assert.hh" -#include #include #include @@ -824,7 +823,7 @@ future<> service_level_controller::migrate_to_v2(size_t nodes_count, db::system_ auto col_names = schema->all_columns() | std::views::transform([] (const auto& col) {return col.name_as_cql_string(); }) | std::ranges::to>(); - auto col_names_str = boost::algorithm::join(col_names, ", "); + auto col_names_str = fmt::to_string(fmt::join(col_names, ", ")); sstring val_binders_str = "?"; for (size_t i = 1; i < col_names.size(); ++i) { val_binders_str += ", ?"; @@ -844,7 +843,7 @@ future<> service_level_controller::migrate_to_v2(size_t nodes_count, db::system_ } auto muts = co_await qp.get_mutations_internal( - format("INSERT INTO {}.{} ({}) VALUES ({})", + seastar::format("INSERT INTO {}.{} ({}) VALUES ({})", db::system_keyspace::NAME, db::system_keyspace::SERVICE_LEVELS_V2, col_names_str, diff --git a/service/raft/raft_group0_client.cc b/service/raft/raft_group0_client.cc index c52284b43f..9fc59bcf57 100644 --- a/service/raft/raft_group0_client.cc +++ b/service/raft/raft_group0_client.cc @@ -12,7 +12,6 @@ #include #include "raft_group0_client.hh" #include "raft_group_registry.hh" -#include #include "frozen_schema.hh" #include "schema_mutations.hh" @@ -589,7 +588,7 @@ future<> group0_batch::commit(::service::raft_group0_client& group0_client, seas if (!_guard) { on_internal_error(logger, "group0_batch: trying to announce without guard"); } - auto description = boost::algorithm::join(_descriptions, "; "); + auto description = fmt::to_string(fmt::join(_descriptions, "; ")); // common case, don't bother with generators as we would have only 1-2 mutations, // when producer expects substantial number or size of mutations it should use generator if (_generators.size() == 0) { diff --git a/service/storage_service.cc b/service/storage_service.cc index fed207321a..bb9e8ea906 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -114,7 +114,6 @@ #include #include -#include #include #include diff --git a/test/boost/cql_query_test.cc b/test/boost/cql_query_test.cc index 95a7308081..ff4a7878ce 100644 --- a/test/boost/cql_query_test.cc +++ b/test/boost/cql_query_test.cc @@ -7,7 +7,6 @@ */ -#include #include #include #include diff --git a/test/boost/user_types_test.cc b/test/boost/user_types_test.cc index 3236ada1c3..9246fbcd83 100644 --- a/test/boost/user_types_test.cc +++ b/test/boost/user_types_test.cc @@ -20,7 +20,6 @@ #include "db/config.hh" #include -#include BOOST_AUTO_TEST_SUITE(user_types_test) diff --git a/test/lib/data_model.cc b/test/lib/data_model.cc index 4afab72ea0..38a30286af 100644 --- a/test/lib/data_model.cc +++ b/test/lib/data_model.cc @@ -11,8 +11,6 @@ #include "utils/assert.hh" #include "test/lib/data_model.hh" -#include - #include "schema/schema_builder.hh" #include "concrete_types.hh" @@ -356,7 +354,7 @@ void table_description::rename_clustering_column(const sstring& from, const sstr table_description::table table_description::build() const { auto s = build_schema(); - return { boost::algorithm::join(_change_log, "\n"), s, build_mutations(s) }; + return { fmt::to_string(fmt::join(_change_log, "\n")), s, build_mutations(s) }; } } diff --git a/test/lib/mutation_source_test.cc b/test/lib/mutation_source_test.cc index 4a29661e67..37cd7c07a9 100644 --- a/test/lib/mutation_source_test.cc +++ b/test/lib/mutation_source_test.cc @@ -27,7 +27,6 @@ #include "test/lib/key_utils.hh" #include "test/lib/log.hh" #include "test/lib/reader_concurrency_semaphore.hh" -#include #include "types/user.hh" #include "types/map.hh" #include "types/list.hh" diff --git a/test/lib/random_schema.cc b/test/lib/random_schema.cc index 9b731b0abd..58537ac35a 100644 --- a/test/lib/random_schema.cc +++ b/test/lib/random_schema.cc @@ -8,7 +8,6 @@ #include -#include #include #include @@ -1016,22 +1015,22 @@ sstring random_schema::cql() const { std::move(cols.begin(), cols.end(), std::back_inserter(col_specs)); } - sstring primary_key; + std::string primary_key; auto partition_column_names = column_names(_schema, column_kind::partition_key); auto clustering_key_names = column_names(_schema, column_kind::clustering_key); if (!clustering_key_names.empty()) { - primary_key = format("({}), {}", boost::algorithm::join(partition_column_names, ", "), boost::algorithm::join(clustering_key_names, ", ")); + primary_key = fmt::format("({}), {}", fmt::join(partition_column_names, ", "), fmt::join(clustering_key_names, ", ")); } else { - primary_key = format("{}", boost::algorithm::join(partition_column_names, ", ")); + primary_key = fmt::format("{}", fmt::join(partition_column_names, ", ")); } // FIXME include the clustering column orderings - return format( + return seastar::format( "{}\nCREATE TABLE {}.{} (\n\t{}\n\tPRIMARY KEY ({}))", udts_str, _schema->ks_name(), _schema->cf_name(), - boost::algorithm::join(col_specs, ",\n\t"), + fmt::join(col_specs, ",\n\t"), primary_key); } diff --git a/test/perf/perf_fast_forward.cc b/test/perf/perf_fast_forward.cc index f99bba2fbc..d7aa1cb4f2 100644 --- a/test/perf/perf_fast_forward.cc +++ b/test/perf/perf_fast_forward.cc @@ -11,7 +11,6 @@ #include "utils/assert.hh" #include #include -#include #include #include #include diff --git a/tools/lua_sstable_consumer.cc b/tools/lua_sstable_consumer.cc index 2be97e117d..e1e7c570d1 100644 --- a/tools/lua_sstable_consumer.cc +++ b/tools/lua_sstable_consumer.cc @@ -7,7 +7,6 @@ */ #include "utils/assert.hh" -#include #include #include #include diff --git a/tools/scylla-nodetool.cc b/tools/scylla-nodetool.cc index 6b7133ba6a..930e640078 100644 --- a/tools/scylla-nodetool.cc +++ b/tools/scylla-nodetool.cc @@ -17,7 +17,6 @@ #include #include #include -#include #include #include #include diff --git a/tools/scylla-sstable.cc b/tools/scylla-sstable.cc index 31f33c3c01..0595cc7b54 100644 --- a/tools/scylla-sstable.cc +++ b/tools/scylla-sstable.cc @@ -7,7 +7,6 @@ */ #include -#include #include #include #include diff --git a/tools/scylla-types.cc b/tools/scylla-types.cc index 394f900c3d..d187a92608 100644 --- a/tools/scylla-types.cc +++ b/tools/scylla-types.cc @@ -6,8 +6,6 @@ * SPDX-License-Identifier: LicenseRef-ScyllaDB-Source-Available-1.0 */ -#include -#include #include #include @@ -96,7 +94,7 @@ sstring to_printable_string(const compound_type& type, bytes_view for (size_t i = 0; i != values.size(); ++i) { printable_values.emplace_back(types.at(i)->to_string(values.at(i))); } - return format("({})", boost::algorithm::join(printable_values, ", ")); + return seastar::format("({})", fmt::join(printable_values, ", ")); } struct printing_visitor {