From 1b96a704d38b38804d317a7dac3663630ac599e7 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 14 Oct 2025 16:49:59 +0200 Subject: [PATCH] Add lazy evaluation for experimental feature reasons Wrap fmt() calls in lambdas to defer string formatting until the feature check fails. This avoids unnecessary string formatting in the common case where the feature is enabled. Addresses performance concern raised by xokdvium in PR review. --- src/libstore/derivations.cc | 7 +++++-- src/libstore/derived-path.cc | 6 +++--- src/libstore/downstream-placeholder.cc | 3 ++- src/libutil/include/nix/util/configuration.hh | 13 +++++++++++++ 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index b5d8d1a1c..fa8bc58ac 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -426,7 +426,9 @@ Derivation parseDerivation( if (*versionS == "xp-dyn-drv"sv) { // Only version we have so far version = DerivationATermVersion::DynamicDerivations; - xpSettings.require(Xp::DynamicDerivations, fmt("derivation '%s', ATerm format version 'xp-dyn-drv'", name)); + xpSettings.require(Xp::DynamicDerivations, [&] { + return fmt("derivation '%s', ATerm format version 'xp-dyn-drv'", name); + }); } else { throw FormatError("Unknown derivation ATerm format version '%s'", *versionS); } @@ -1454,7 +1456,8 @@ Derivation Derivation::fromJSON(const nlohmann::json & _json, const Experimental node.value = getStringSet(valueAt(json, "outputs")); auto drvs = getObject(valueAt(json, "dynamicOutputs")); for (auto & [outputId, childNode] : drvs) { - xpSettings.require(Xp::DynamicDerivations, fmt("dynamic output '%s' in JSON", outputId)); + xpSettings.require( + Xp::DynamicDerivations, [&] { return fmt("dynamic output '%s' in JSON", outputId); }); node.childMap[outputId] = doInput(childNode); } return node; diff --git a/src/libstore/derived-path.cc b/src/libstore/derived-path.cc index 34e591666..8d606cb41 100644 --- a/src/libstore/derived-path.cc +++ b/src/libstore/derived-path.cc @@ -86,9 +86,9 @@ void drvRequireExperiment(const SingleDerivedPath & drv, const ExperimentalFeatu // plain drv path; no experimental features required. }, [&](const SingleDerivedPath::Built & b) { - xpSettings.require( - Xp::DynamicDerivations, - fmt("building output '%s' of '%s'", b.output, b.drvPath->getBaseStorePath().to_string())); + xpSettings.require(Xp::DynamicDerivations, [&] { + return fmt("building output '%s' of '%s'", b.output, b.drvPath->getBaseStorePath().to_string()); + }); }, }, drv.raw()); diff --git a/src/libstore/downstream-placeholder.cc b/src/libstore/downstream-placeholder.cc index 30044501b..780717a62 100644 --- a/src/libstore/downstream-placeholder.cc +++ b/src/libstore/downstream-placeholder.cc @@ -24,7 +24,8 @@ DownstreamPlaceholder DownstreamPlaceholder::unknownDerivation( OutputNameView outputName, const ExperimentalFeatureSettings & xpSettings) { - xpSettings.require(Xp::DynamicDerivations, fmt("placeholder for unknown derivation output '%s'", outputName)); + xpSettings.require( + Xp::DynamicDerivations, [&] { return fmt("placeholder for unknown derivation output '%s'", outputName); }); auto compressed = compressHash(placeholder.hash, 20); auto clearText = "nix-computed-output:" + compressed.to_string(HashFormat::Nix32, false) + ":" + std::string{outputName}; diff --git a/src/libutil/include/nix/util/configuration.hh b/src/libutil/include/nix/util/configuration.hh index c8d7b7f24..541febdb5 100644 --- a/src/libutil/include/nix/util/configuration.hh +++ b/src/libutil/include/nix/util/configuration.hh @@ -465,6 +465,19 @@ struct ExperimentalFeatureSettings : Config */ void require(const ExperimentalFeature &, std::string reason = "") const; + /** + * Require an experimental feature be enabled, throwing an error if it is + * not. The reason is lazily evaluated only if the feature is disabled. + */ + template + requires std::invocable && std::convertible_to, std::string> + void require(const ExperimentalFeature & feature, GetReason && getReason) const + { + if (isEnabled(feature)) + return; + require(feature, getReason()); + } + /** * `std::nullopt` pointer means no feature, which means there is nothing that could be * disabled, and so the function returns true in that case.