Merge 'Make inclusion of config.hh cheaper' from Nadav Har'El

This is an attempt (mostly suggested and implemented by AI, but with a few hours of human babysitting...), to somewhat reduce compilation time by picking one template, named_value<T>, which is used in more than a hundred source files through the config.hh header, and making it use external instantiation: The different methods of named_value<T> for various T are instantiated only once (in config.cc), and the individual translation units don't need to compile them a hundred times.

The resulting saving is a little underwhelming: The total object-file size goes down about 1% (from 346,200 before the patch to 343,488 after the patch), and previous experience shows that this object-file size is proportional to the compilation time, most of which involves code generation. But I haven't been able to measure speedup of the build itself.

1% is not nothing, but not a huge saving either. Though arguably, with 50 more of these patches, we can make the build twice faster :-)

Refs #1.

Closes scylladb/scylladb#28992

* github.com:scylladb/scylladb:
  config: move named_value<T> method bodies out-of-line
  config: suppress named_value<T> instantiation in every source file
This commit is contained in:
Botond Dénes
2026-04-15 14:30:14 +03:00
6 changed files with 269 additions and 75 deletions

View File

@@ -273,6 +273,34 @@ const config_type& config_type_for<std::vector<enum_option<db::consistency_level
return ct;
}
template <>
const config_type& config_type_for<enum_option<db::experimental_features_t>>() {
static config_type ct(
"experimental feature", printable_to_json<enum_option<db::experimental_features_t>>);
return ct;
}
template <>
const config_type& config_type_for<enum_option<db::replication_strategy_restriction_t>>() {
static config_type ct(
"replication strategy", printable_to_json<enum_option<db::replication_strategy_restriction_t>>);
return ct;
}
template <>
const config_type& config_type_for<enum_option<db::consistency_level_restriction_t>>() {
static config_type ct(
"consistency level", printable_to_json<enum_option<db::consistency_level_restriction_t>>);
return ct;
}
template <>
const config_type& config_type_for<db::error_injection_at_startup>() {
static config_type ct(
"error injection", printable_to_json<db::error_injection_at_startup>);
return ct;
}
template <>
const config_type& config_type_for<enum_option<db::tri_mode_restriction_t>>() {
static config_type ct(
@@ -1942,6 +1970,47 @@ std::unordered_map<sstring, db::tablets_mode_t::mode> db::tablets_mode_t::map()
template struct utils::config_file::named_value<seastar::log_level>;
// Explicit instantiation definitions for all named_value<T> specializations
// declared extern in config_file.hh and config.hh. This file is the only
// translation unit that includes config_file_impl.hh (which contains the
// full template bodies), so all the heavy boost / yaml-cpp machinery is
// compiled exactly once here instead of in every TU that includes config.hh.
// Primitive / standard types (extern-declared in utils/config_file.hh):
template struct utils::config_file::named_value<bool>;
template struct utils::config_file::named_value<uint16_t>;
template struct utils::config_file::named_value<uint32_t>;
template struct utils::config_file::named_value<uint64_t>;
template struct utils::config_file::named_value<int32_t>;
template struct utils::config_file::named_value<int64_t>;
template struct utils::config_file::named_value<float>;
template struct utils::config_file::named_value<double>;
template struct utils::config_file::named_value<sstring>;
template struct utils::config_file::named_value<std::string>;
template struct utils::config_file::named_value<utils::config_file::string_map>;
template struct utils::config_file::named_value<utils::config_file::string_list>;
// db-specific types (extern-declared in db/config.hh):
template struct utils::config_file::named_value<db::tri_mode_restriction>;
template struct utils::config_file::named_value<db::seed_provider_type>;
template struct utils::config_file::named_value<db::hints::host_filter>;
template struct utils::config_file::named_value<utils::UUID>;
template struct utils::config_file::named_value<db::error_injection_at_startup>;
template struct utils::config_file::named_value<compression_parameters>;
template struct utils::config_file::named_value<enum_option<db::experimental_features_t>>;
template struct utils::config_file::named_value<enum_option<db::replication_strategy_restriction_t>>;
template struct utils::config_file::named_value<enum_option<db::consistency_level_restriction_t>>;
template struct utils::config_file::named_value<enum_option<db::tablets_mode_t>>;
template struct utils::config_file::named_value<enum_option<netw::dict_training_loop::when>>;
template struct utils::config_file::named_value<netw::advanced_rpc_compressor::tracker::algo_config>;
template struct utils::config_file::named_value<std::vector<enum_option<db::experimental_features_t>>>;
template struct utils::config_file::named_value<std::vector<enum_option<db::replication_strategy_restriction_t>>>;
template struct utils::config_file::named_value<std::vector<enum_option<db::consistency_level_restriction_t>>>;
template struct utils::config_file::named_value<std::vector<db::error_injection_at_startup>>;
template struct utils::config_file::named_value<std::vector<std::unordered_map<sstring, sstring>>>;
template struct utils::config_file::named_value<std::unordered_map<sstring, seastar::log_level>>;
template struct utils::config_file::named_value<std::vector<db::object_storage_endpoint_param>>;
namespace utils {
sstring

View File

@@ -689,3 +689,42 @@ future<> update_relabel_config_from_file(const std::string& name);
std::vector<sstring> split_comma_separated_list(std::string_view comma_separated_list);
} // namespace utils
namespace utils {
// Declaration of the explicit specialization for seed_provider_type's
// add_command_line_option must appear before the extern template declaration
// below, to satisfy [temp.expl.spec]: a member specialization must be
// declared before any explicit instantiation (including extern template) of
// the enclosing class template.
template <>
void config_file::named_value<db::config::seed_provider_type>::add_command_line_option(
boost::program_options::options_description_easy_init&);
} // namespace utils
// Explicit instantiation declarations for named_value<T> specializations
// that use db-specific types. The definitions live in db/config.cc.
// Together with the declarations in utils/config_file.hh (for primitive
// types), this ensures the heavy template bodies from config_file_impl.hh
// (boost::program_options, boost::lexical_cast, boost::regex, yaml-cpp)
// are compiled only once.
extern template struct utils::config_file::named_value<db::tri_mode_restriction>;
extern template struct utils::config_file::named_value<db::seed_provider_type>;
extern template struct utils::config_file::named_value<db::hints::host_filter>;
extern template struct utils::config_file::named_value<utils::UUID>;
extern template struct utils::config_file::named_value<db::error_injection_at_startup>;
extern template struct utils::config_file::named_value<compression_parameters>;
extern template struct utils::config_file::named_value<enum_option<db::experimental_features_t>>;
extern template struct utils::config_file::named_value<enum_option<db::replication_strategy_restriction_t>>;
extern template struct utils::config_file::named_value<enum_option<db::consistency_level_restriction_t>>;
extern template struct utils::config_file::named_value<enum_option<db::tablets_mode_t>>;
extern template struct utils::config_file::named_value<enum_option<netw::dict_training_loop::when>>;
extern template struct utils::config_file::named_value<netw::advanced_rpc_compressor::tracker::algo_config>;
extern template struct utils::config_file::named_value<std::vector<enum_option<db::experimental_features_t>>>;
extern template struct utils::config_file::named_value<std::vector<enum_option<db::replication_strategy_restriction_t>>>;
extern template struct utils::config_file::named_value<std::vector<enum_option<db::consistency_level_restriction_t>>>;
extern template struct utils::config_file::named_value<std::vector<db::error_injection_at_startup>>;
extern template struct utils::config_file::named_value<std::vector<std::unordered_map<sstring, sstring>>>;
extern template struct utils::config_file::named_value<std::unordered_map<sstring, seastar::log_level>>;
extern template struct utils::config_file::named_value<std::vector<db::object_storage_endpoint_param>>;

View File

@@ -175,3 +175,5 @@ public:
};
}
} cfg;
template struct utils::config_file::named_value<encryption::encryption_config::string_string_map>;

View File

@@ -32,3 +32,5 @@ public:
};
}
extern template struct utils::config_file::named_value<encryption::encryption_config::string_string_map>;

View File

@@ -168,96 +168,40 @@ public:
config_source _source = config_source::None;
value_status _value_status;
struct the_value_type final : any_value {
the_value_type(T value) : value(std::move(value)) {}
the_value_type(T value);
utils::updateable_value_source<T> value;
virtual std::unique_ptr<any_value> clone() const override {
return std::make_unique<the_value_type>(value());
}
virtual void update_from(const any_value* source) override {
auto typed_source = static_cast<const the_value_type*>(source);
value.set(typed_source->value());
}
std::unique_ptr<any_value> clone() const override;
void update_from(const any_value* source) override;
};
liveness _liveness;
std::vector<T> _allowed_values;
protected:
updateable_value_source<T>& the_value() {
any_value* av = _cf->_per_shard_values[_cf->s_shard_id][_per_shard_values_offset].get();
return static_cast<the_value_type*>(av)->value;
}
const updateable_value_source<T>& the_value() const {
return const_cast<named_value*>(this)->the_value();
}
virtual const void* current_value() const override {
return &the_value().get();
}
updateable_value_source<T>& the_value();
const updateable_value_source<T>& the_value() const;
const void* current_value() const override;
public:
typedef T type;
typedef named_value<T> MyType;
named_value(config_file* file, std::string_view name, std::string_view alias, liveness liveness_, value_status vs, const T& t = T(), std::string_view desc = {},
std::initializer_list<T> allowed_values = {})
: config_src(file, name, alias, &config_type_for<T>(), desc)
, _value_status(vs)
, _liveness(liveness_)
, _allowed_values(std::move(allowed_values)) {
file->add(*this, std::make_unique<the_value_type>(std::move(t)));
}
std::initializer_list<T> allowed_values = {});
named_value(config_file* file, std::string_view name, liveness liveness_, value_status vs, const T& t = T(), std::string_view desc = {},
std::initializer_list<T> allowed_values = {})
: named_value(file, name, {}, liveness_, vs, t, desc, std::move(allowed_values)) {
}
std::initializer_list<T> allowed_values = {});
named_value(config_file* file, std::string_view name, std::string_view alias, value_status vs, const T& t = T(), std::string_view desc = {},
std::initializer_list<T> allowed_values = {})
: named_value(file, name, alias, liveness::MustRestart, vs, t, desc, allowed_values) {
}
std::initializer_list<T> allowed_values = {});
named_value(config_file* file, std::string_view name, value_status vs, const T& t = T(), std::string_view desc = {},
std::initializer_list<T> allowed_values = {})
: named_value(file, name, {}, liveness::MustRestart, vs, t, desc, allowed_values) {
}
value_status status() const noexcept override {
return _value_status;
}
config_source source() const noexcept override {
return _source;
}
bool is_set() const {
return _source > config_source::None;
}
MyType & operator()(const T& t, config_source src = config_source::Internal) {
if (!_allowed_values.empty() && std::find(_allowed_values.begin(), _allowed_values.end(), t) == _allowed_values.end()) {
throw std::invalid_argument(fmt::format("Invalid value for {}: got {} which is not inside the set of allowed values {}", name(), t, _allowed_values));
}
the_value().set(t);
if (src > config_source::None) {
_source = src;
}
return *this;
}
MyType & operator()(T&& t, config_source src = config_source::Internal) {
if (!_allowed_values.empty() && std::find(_allowed_values.begin(), _allowed_values.end(), t) == _allowed_values.end()) {
throw std::invalid_argument(fmt::format("Invalid value for {}: got {} which is not inside the set of allowed values {}", name(), t, _allowed_values));
}
the_value().set(std::move(t));
if (src > config_source::None) {
_source = src;
}
return *this;
}
void set(T&& t, config_source src = config_source::None) {
operator()(std::move(t), src);
}
const T& operator()() const {
return the_value().get();
}
std::initializer_list<T> allowed_values = {});
value_status status() const noexcept override;
config_source source() const noexcept override;
bool is_set() const;
MyType & operator()(const T& t, config_source src = config_source::Internal);
MyType & operator()(T&& t, config_source src = config_source::Internal);
void set(T&& t, config_source src = config_source::None);
const T& operator()() const;
operator updateable_value<T>() const & {
return updateable_value<T>(the_value());
}
operator updateable_value<T>() const &;
observer<T> observe(std::function<void (const T&)> callback) const {
return the_value().observe(std::move(callback));
}
observer<T> observe(std::function<void (const T&)> callback) const;
void add_command_line_option(bpo::options_description_easy_init&) override;
void set_value(const YAML::Node&, config_source) override;
@@ -336,5 +280,24 @@ const config_file::named_value<T>& operator||(const config_file::named_value<T>&
extern template struct config_file::named_value<seastar::log_level>;
// Explicit instantiation declarations for the most common named_value<T>
// specializations. The definitions are in db/config.cc (which is the only
// TU that includes config_file_impl.hh and therefore has the full template
// bodies). This avoids re-compiling the heavy boost::program_options /
// boost::lexical_cast / yaml-cpp machinery in every TU that includes
// config.hh.
extern template struct config_file::named_value<bool>;
extern template struct config_file::named_value<uint16_t>;
extern template struct config_file::named_value<uint32_t>;
extern template struct config_file::named_value<uint64_t>;
extern template struct config_file::named_value<int32_t>;
extern template struct config_file::named_value<int64_t>;
extern template struct config_file::named_value<float>;
extern template struct config_file::named_value<double>;
extern template struct config_file::named_value<sstring>;
extern template struct config_file::named_value<std::string>;
extern template struct config_file::named_value<config_file::string_map>;
extern template struct config_file::named_value<config_file::string_list>;
}

View File

@@ -186,6 +186,125 @@ sstring hyphenate(const std::string_view&);
}
template<typename T>
utils::config_file::named_value<T>::the_value_type::the_value_type(T value)
: value(std::move(value)) {}
template<typename T>
std::unique_ptr<utils::config_file::any_value>
utils::config_file::named_value<T>::the_value_type::clone() const {
return std::make_unique<the_value_type>(value());
}
template<typename T>
void utils::config_file::named_value<T>::the_value_type::update_from(const any_value* source) {
auto typed_source = static_cast<const the_value_type*>(source);
value.set(typed_source->value());
}
template<typename T>
utils::updateable_value_source<T>& utils::config_file::named_value<T>::the_value() {
any_value* av = _cf->_per_shard_values[_cf->s_shard_id][_per_shard_values_offset].get();
return static_cast<the_value_type*>(av)->value;
}
template<typename T>
const utils::updateable_value_source<T>& utils::config_file::named_value<T>::the_value() const {
return const_cast<named_value*>(this)->the_value();
}
template<typename T>
const void* utils::config_file::named_value<T>::current_value() const {
return &the_value().get();
}
template<typename T>
utils::config_file::named_value<T>::named_value(config_file* file, std::string_view name, std::string_view alias, liveness liveness_, value_status vs, const T& t, std::string_view desc,
std::initializer_list<T> allowed_values)
: config_src(file, name, alias, &config_type_for<T>(), desc)
, _value_status(vs)
, _liveness(liveness_)
, _allowed_values(std::move(allowed_values)) {
file->add(*this, std::make_unique<the_value_type>(std::move(t)));
}
template<typename T>
utils::config_file::named_value<T>::named_value(config_file* file, std::string_view name, liveness liveness_, value_status vs, const T& t, std::string_view desc,
std::initializer_list<T> allowed_values)
: named_value(file, name, {}, liveness_, vs, t, desc, std::move(allowed_values)) {
}
template<typename T>
utils::config_file::named_value<T>::named_value(config_file* file, std::string_view name, std::string_view alias, value_status vs, const T& t, std::string_view desc,
std::initializer_list<T> allowed_values)
: named_value(file, name, alias, liveness::MustRestart, vs, t, desc, allowed_values) {
}
template<typename T>
utils::config_file::named_value<T>::named_value(config_file* file, std::string_view name, value_status vs, const T& t, std::string_view desc,
std::initializer_list<T> allowed_values)
: named_value(file, name, {}, liveness::MustRestart, vs, t, desc, allowed_values) {
}
template<typename T>
utils::config_file::value_status utils::config_file::named_value<T>::status() const noexcept {
return _value_status;
}
template<typename T>
utils::config_file::config_source utils::config_file::named_value<T>::source() const noexcept {
return _source;
}
template<typename T>
bool utils::config_file::named_value<T>::is_set() const {
return _source > config_source::None;
}
template<typename T>
utils::config_file::named_value<T>& utils::config_file::named_value<T>::operator()(const T& t, config_source src) {
if (!_allowed_values.empty() && std::find(_allowed_values.begin(), _allowed_values.end(), t) == _allowed_values.end()) {
throw std::invalid_argument(fmt::format("Invalid value for {}: got {} which is not inside the set of allowed values {}", name(), t, _allowed_values));
}
the_value().set(t);
if (src > config_source::None) {
_source = src;
}
return *this;
}
template<typename T>
utils::config_file::named_value<T>& utils::config_file::named_value<T>::operator()(T&& t, config_source src) {
if (!_allowed_values.empty() && std::find(_allowed_values.begin(), _allowed_values.end(), t) == _allowed_values.end()) {
throw std::invalid_argument(fmt::format("Invalid value for {}: got {} which is not inside the set of allowed values {}", name(), t, _allowed_values));
}
the_value().set(std::move(t));
if (src > config_source::None) {
_source = src;
}
return *this;
}
template<typename T>
void utils::config_file::named_value<T>::set(T&& t, config_source src) {
operator()(std::move(t), src);
}
template<typename T>
const T& utils::config_file::named_value<T>::operator()() const {
return the_value().get();
}
template<typename T>
utils::config_file::named_value<T>::operator utils::updateable_value<T>() const & {
return updateable_value<T>(the_value());
}
template<typename T>
utils::observer<T> utils::config_file::named_value<T>::observe(std::function<void (const T&)> callback) const {
return the_value().observe(std::move(callback));
}
template<typename T>
void utils::config_file::named_value<T>::add_command_line_option(boost::program_options::options_description_easy_init& init) {
const auto hyphenated_name = hyphenate(name());