Skip to content

Commit 1b96a70

Browse files
committed
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.
1 parent 39c4665 commit 1b96a70

File tree

4 files changed

+23
-6
lines changed

4 files changed

+23
-6
lines changed

src/libstore/derivations.cc

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,9 @@ Derivation parseDerivation(
426426
if (*versionS == "xp-dyn-drv"sv) {
427427
// Only version we have so far
428428
version = DerivationATermVersion::DynamicDerivations;
429-
xpSettings.require(Xp::DynamicDerivations, fmt("derivation '%s', ATerm format version 'xp-dyn-drv'", name));
429+
xpSettings.require(Xp::DynamicDerivations, [&] {
430+
return fmt("derivation '%s', ATerm format version 'xp-dyn-drv'", name);
431+
});
430432
} else {
431433
throw FormatError("Unknown derivation ATerm format version '%s'", *versionS);
432434
}
@@ -1454,7 +1456,8 @@ Derivation Derivation::fromJSON(const nlohmann::json & _json, const Experimental
14541456
node.value = getStringSet(valueAt(json, "outputs"));
14551457
auto drvs = getObject(valueAt(json, "dynamicOutputs"));
14561458
for (auto & [outputId, childNode] : drvs) {
1457-
xpSettings.require(Xp::DynamicDerivations, fmt("dynamic output '%s' in JSON", outputId));
1459+
xpSettings.require(
1460+
Xp::DynamicDerivations, [&] { return fmt("dynamic output '%s' in JSON", outputId); });
14581461
node.childMap[outputId] = doInput(childNode);
14591462
}
14601463
return node;

src/libstore/derived-path.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,9 @@ void drvRequireExperiment(const SingleDerivedPath & drv, const ExperimentalFeatu
8686
// plain drv path; no experimental features required.
8787
},
8888
[&](const SingleDerivedPath::Built & b) {
89-
xpSettings.require(
90-
Xp::DynamicDerivations,
91-
fmt("building output '%s' of '%s'", b.output, b.drvPath->getBaseStorePath().to_string()));
89+
xpSettings.require(Xp::DynamicDerivations, [&] {
90+
return fmt("building output '%s' of '%s'", b.output, b.drvPath->getBaseStorePath().to_string());
91+
});
9292
},
9393
},
9494
drv.raw());

src/libstore/downstream-placeholder.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ DownstreamPlaceholder DownstreamPlaceholder::unknownDerivation(
2424
OutputNameView outputName,
2525
const ExperimentalFeatureSettings & xpSettings)
2626
{
27-
xpSettings.require(Xp::DynamicDerivations, fmt("placeholder for unknown derivation output '%s'", outputName));
27+
xpSettings.require(
28+
Xp::DynamicDerivations, [&] { return fmt("placeholder for unknown derivation output '%s'", outputName); });
2829
auto compressed = compressHash(placeholder.hash, 20);
2930
auto clearText =
3031
"nix-computed-output:" + compressed.to_string(HashFormat::Nix32, false) + ":" + std::string{outputName};

src/libutil/include/nix/util/configuration.hh

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,19 @@ struct ExperimentalFeatureSettings : Config
465465
*/
466466
void require(const ExperimentalFeature &, std::string reason = "") const;
467467

468+
/**
469+
* Require an experimental feature be enabled, throwing an error if it is
470+
* not. The reason is lazily evaluated only if the feature is disabled.
471+
*/
472+
template<typename GetReason>
473+
requires std::invocable<GetReason> && std::convertible_to<std::invoke_result_t<GetReason>, std::string>
474+
void require(const ExperimentalFeature & feature, GetReason && getReason) const
475+
{
476+
if (isEnabled(feature))
477+
return;
478+
require(feature, getReason());
479+
}
480+
468481
/**
469482
* `std::nullopt` pointer means no feature, which means there is nothing that could be
470483
* disabled, and so the function returns true in that case.

0 commit comments

Comments
 (0)