From 007211e7a27a512cb343060e8b363c9f66ef67af Mon Sep 17 00:00:00 2001 From: piegames Date: Sun, 18 Aug 2024 14:43:12 +0200 Subject: [PATCH] libutil: Optimize feature checks Instead of doing a linear search on an std::set, we use a bitset enum. Change-Id: Ide537f6cffdd16d06e59aaeb2e4ac0acb6493421 --- src/libutil/config-impl.hh | 12 +++---- src/libutil/config.cc | 50 ++++++++++++++++------------ src/libutil/config.hh | 5 ++- src/libutil/deprecated-features.cc | 6 ++-- src/libutil/deprecated-features.hh | 18 ++++++++-- src/libutil/experimental-features.cc | 6 ++-- src/libutil/experimental-features.hh | 16 ++++++++- src/nix/main.cc | 9 +++-- 8 files changed, 77 insertions(+), 45 deletions(-) diff --git a/src/libutil/config-impl.hh b/src/libutil/config-impl.hh index bc88b5504..024018e00 100644 --- a/src/libutil/config-impl.hh +++ b/src/libutil/config-impl.hh @@ -30,11 +30,11 @@ template<> struct BaseSetting::trait { static constexpr bool appendable = true; }; -template<> struct BaseSetting>::trait +template<> struct BaseSetting::trait { static constexpr bool appendable = true; }; -template<> struct BaseSetting>::trait +template<> struct BaseSetting::trait { static constexpr bool appendable = true; }; @@ -54,8 +54,8 @@ bool BaseSetting::isAppendable() template<> void BaseSetting::appendOrSet(Strings newValue, bool append); template<> void BaseSetting::appendOrSet(StringSet newValue, bool append); template<> void BaseSetting::appendOrSet(StringMap newValue, bool append); -template<> void BaseSetting>::appendOrSet(std::set newValue, bool append); -template<> void BaseSetting>::appendOrSet(std::set newValue, bool append); +template<> void BaseSetting::appendOrSet(ExperimentalFeatures newValue, bool append); +template<> void BaseSetting::appendOrSet(DeprecatedFeatures newValue, bool append); template void BaseSetting::appendOrSet(T newValue, bool append) @@ -120,8 +120,8 @@ DECLARE_CONFIG_SERIALISER(bool) DECLARE_CONFIG_SERIALISER(Strings) DECLARE_CONFIG_SERIALISER(StringSet) DECLARE_CONFIG_SERIALISER(StringMap) -DECLARE_CONFIG_SERIALISER(std::set) -DECLARE_CONFIG_SERIALISER(std::set) +DECLARE_CONFIG_SERIALISER(ExperimentalFeatures) +DECLARE_CONFIG_SERIALISER(DeprecatedFeatures) template T BaseSetting::parse(const std::string & str) const diff --git a/src/libutil/config.cc b/src/libutil/config.cc index 3371e0bb3..3c4f4798b 100644 --- a/src/libutil/config.cc +++ b/src/libutil/config.cc @@ -330,55 +330,61 @@ template<> std::string BaseSetting::to_string() const return concatStringsSep(" ", value); } -template<> std::set BaseSetting>::parse(const std::string & str) const +template<> ExperimentalFeatures BaseSetting::parse(const std::string & str) const { - std::set res; + ExperimentalFeatures res{}; for (auto & s : tokenizeString(str)) { if (auto thisXpFeature = parseExperimentalFeature(s); thisXpFeature) - res.insert(thisXpFeature.value()); + res = res | thisXpFeature.value(); else warn("unknown experimental feature '%s'", s); } return res; } -template<> void BaseSetting>::appendOrSet(std::set newValue, bool append) +template<> void BaseSetting::appendOrSet(ExperimentalFeatures newValue, bool append) { - if (!append) value.clear(); - value.insert(std::make_move_iterator(newValue.begin()), std::make_move_iterator(newValue.end())); + if (append) + value = value | newValue; + else + value = newValue; } -template<> std::string BaseSetting>::to_string() const +template<> std::string BaseSetting::to_string() const { StringSet stringifiedXpFeatures; - for (const auto & feature : value) - stringifiedXpFeatures.insert(std::string(showExperimentalFeature(feature))); + for (size_t tag = 0; tag < sizeof(ExperimentalFeatures) * CHAR_BIT; tag++) + if ((value & ExperimentalFeature(tag)) != ExperimentalFeatures{}) + stringifiedXpFeatures.insert(std::string(showExperimentalFeature(ExperimentalFeature(tag)))); return concatStringsSep(" ", stringifiedXpFeatures); } -template<> std::set BaseSetting>::parse(const std::string & str) const +template<> DeprecatedFeatures BaseSetting::parse(const std::string & str) const { - std::set res; + DeprecatedFeatures res{}; for (auto & s : tokenizeString(str)) { if (auto thisDpFeature = parseDeprecatedFeature(s); thisDpFeature) - res.insert(thisDpFeature.value()); + res = res | thisDpFeature.value(); else warn("unknown deprecated feature '%s'", s); } return res; } -template<> void BaseSetting>::appendOrSet(std::set newValue, bool append) +template<> void BaseSetting::appendOrSet(DeprecatedFeatures newValue, bool append) { - if (!append) value.clear(); - value.insert(std::make_move_iterator(newValue.begin()), std::make_move_iterator(newValue.end())); + if (append) + value = value | newValue; + else + value = newValue; } -template<> std::string BaseSetting>::to_string() const +template<> std::string BaseSetting::to_string() const { StringSet stringifiedDpFeatures; - for (const auto & feature : value) - stringifiedDpFeatures.insert(std::string(showDeprecatedFeature(feature))); + for (size_t tag = 0; tag < sizeof(DeprecatedFeatures) * CHAR_BIT; tag++) + if ((value & DeprecatedFeature(tag)) != DeprecatedFeatures{}) + stringifiedDpFeatures.insert(std::string(showDeprecatedFeature(DeprecatedFeature(tag)))); return concatStringsSep(" ", stringifiedDpFeatures); } @@ -417,8 +423,8 @@ template class BaseSetting; template class BaseSetting; template class BaseSetting; template class BaseSetting; -template class BaseSetting>; -template class BaseSetting>; +template class BaseSetting; +template class BaseSetting; static Path parsePath(const AbstractSetting & s, const std::string & str) { @@ -562,7 +568,7 @@ static GlobalConfig::Register rSettings(&experimentalFeatureSettings); bool FeatureSettings::isEnabled(const ExperimentalFeature & feature) const { auto & f = experimentalFeatures.get(); - return std::find(f.begin(), f.end(), feature) != f.end(); + return (f & feature) != ExperimentalFeatures{}; } void FeatureSettings::require(const ExperimentalFeature & feature) const @@ -584,7 +590,7 @@ void FeatureSettings::require(const std::optional & feature bool FeatureSettings::isEnabled(const DeprecatedFeature & feature) const { auto & f = deprecatedFeatures.get(); - return std::find(f.begin(), f.end(), feature) != f.end(); + return (f & feature) != DeprecatedFeatures{}; } void FeatureSettings::require(const DeprecatedFeature & feature) const diff --git a/src/libutil/config.hh b/src/libutil/config.hh index 36e42fe63..d1812f47e 100644 --- a/src/libutil/config.hh +++ b/src/libutil/config.hh @@ -444,7 +444,7 @@ extern GlobalConfig globalConfig; struct FeatureSettings : Config { - Setting> experimentalFeatures{ + Setting experimentalFeatures{ this, {}, "experimental-features", R"( Experimental features that are enabled. @@ -484,8 +484,7 @@ struct FeatureSettings : Config { * disabled, and so the function does nothing in that case. */ void require(const std::optional &) const; - - Setting> deprecatedFeatures{ + Setting deprecatedFeatures{ this, {}, "deprecated-features", R"( Deprecated features that are allowed. diff --git a/src/libutil/deprecated-features.cc b/src/libutil/deprecated-features.cc index 11b6c42bd..877d69da0 100644 --- a/src/libutil/deprecated-features.cc +++ b/src/libutil/deprecated-features.cc @@ -77,12 +77,12 @@ nlohmann::json documentDeprecatedFeatures() return (nlohmann::json) res; } -std::set parseDeprecatedFeatures(const std::set & rawFeatures) +DeprecatedFeatures parseDeprecatedFeatures(const std::set & rawFeatures) { - std::set res; + DeprecatedFeatures res{}; for (auto & rawFeature : rawFeatures) if (auto feature = parseDeprecatedFeature(rawFeature)) - res.insert(*feature); + res = res | *feature; return res; } diff --git a/src/libutil/deprecated-features.hh b/src/libutil/deprecated-features.hh index c00a5d7bd..3776e6c29 100644 --- a/src/libutil/deprecated-features.hh +++ b/src/libutil/deprecated-features.hh @@ -18,9 +18,23 @@ namespace nix { */ enum struct DeprecatedFeature { - UrlLiterals + UrlLiterals, }; +enum struct DeprecatedFeatures {}; + +inline DeprecatedFeatures operator| (DeprecatedFeatures a, DeprecatedFeatures b) { + return static_cast(static_cast(a) | static_cast(b)); +} + +inline DeprecatedFeatures operator| (DeprecatedFeatures a, DeprecatedFeature b) { + return a | static_cast(1 << static_cast(b)); +} + +inline DeprecatedFeatures operator& (DeprecatedFeatures a, DeprecatedFeature b) { + return static_cast(static_cast(a) & (1 << static_cast(b))); +} + /** * Just because writing `DeprecatedFeature::UrlLiterals` is way too long */ @@ -50,7 +64,7 @@ std::ostream & operator<<( * Parse a set of strings to the corresponding set of deprecated * features, ignoring (but warning for) any unknown feature. */ -std::set parseDeprecatedFeatures(const std::set &); +DeprecatedFeatures parseDeprecatedFeatures(const std::set &); /** * A deprecated feature used for some diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index 15a18c770..35982c28c 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -293,12 +293,12 @@ nlohmann::json documentExperimentalFeatures() return (nlohmann::json) res; } -std::set parseFeatures(const std::set & rawFeatures) +ExperimentalFeatures parseFeatures(const std::set & rawFeatures) { - std::set res; + ExperimentalFeatures res {}; for (auto & rawFeature : rawFeatures) if (auto feature = parseExperimentalFeature(rawFeature)) - res.insert(*feature); + res = res | *feature; return res; } diff --git a/src/libutil/experimental-features.hh b/src/libutil/experimental-features.hh index 121318d23..c45f05d78 100644 --- a/src/libutil/experimental-features.hh +++ b/src/libutil/experimental-features.hh @@ -33,6 +33,20 @@ enum struct ExperimentalFeature ReplAutomation, }; +enum struct ExperimentalFeatures {}; + +inline ExperimentalFeatures operator| (ExperimentalFeatures a, ExperimentalFeatures b) { + return static_cast(static_cast(a) | static_cast(b)); +} + +inline ExperimentalFeatures operator| (ExperimentalFeatures a, ExperimentalFeature b) { + return a | static_cast(1 << static_cast(b)); +} + +inline ExperimentalFeatures operator& (ExperimentalFeatures a, ExperimentalFeature b) { + return static_cast(static_cast(a) & (1 << static_cast(b))); +} + /** * Just because writing `ExperimentalFeature::CaDerivations` is way too long */ @@ -62,7 +76,7 @@ std::ostream & operator<<( * Parse a set of strings to the corresponding set of experimental * features, ignoring (but warning for) any unknown feature. */ -std::set parseFeatures(const std::set &); +ExperimentalFeatures parseFeatures(const std::set &); /** * An experimental feature was required for some (experimental) diff --git a/src/nix/main.cc b/src/nix/main.cc index 5356a0d04..e84e4f310 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -378,11 +378,10 @@ void mainWrapped(int argc, char * * argv) } if (argc == 2 && std::string(argv[1]) == "__dump-language") { - experimentalFeatureSettings.experimentalFeatures = { - Xp::Flakes, - Xp::FetchClosure, - Xp::DynamicDerivations, - }; + experimentalFeatureSettings.experimentalFeatures = ExperimentalFeatures{} + | Xp::Flakes + | Xp::FetchClosure + | Xp::DynamicDerivations; evalSettings.pureEval = false; EvalState state({}, openStore("dummy://")); auto res = nlohmann::json::object();