1
1
Fork 0
mirror of https://github.com/NixOS/nix.git synced 2025-11-09 12:06:01 +01:00

libexpr: Convert Symbol comparisons to switch statements

Now that Symbols are statically allocated at compile time with known IDs,
we can use switch statements instead of if-else chains for Symbol comparisons.
This provides better performance through compiler optimizations like jump tables.

Changes:
- Add public getId() method to Symbol class to access the internal ID
- Convert if-else chains comparing Symbol values to switch statements
  in primops.cc's derivationStrictInternal function
- Simplify control flow by removing the 'handled' flag and moving the
  default attribute handling into the switch's default case

The static and runtime Symbol IDs are guaranteed to match by the
copyIntoSymbolTable implementation which asserts this invariant.

Co-authored-by: John Ericson <git@JohnEricson.me>
This commit is contained in:
Jörg Thalheim 2025-08-31 13:55:11 +02:00
parent 363620dd24
commit cbcb434cb3
2 changed files with 75 additions and 33 deletions

View file

@ -61,6 +61,15 @@ public:
return id > 0; return id > 0;
} }
/**
* The ID is a private implementation detail that should generally not be observed. However, we expose here just for
* sake of `switch...case`, which needs to dispatch on numbers. */
[[gnu::always_inline]]
constexpr uint32_t getId() const noexcept
{
return id;
}
constexpr auto operator<=>(const Symbol & other) const noexcept = default; constexpr auto operator<=>(const Symbol & other) const noexcept = default;
friend class std::hash<Symbol>; friend class std::hash<Symbol>;

View file

@ -1454,19 +1454,22 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
continue; continue;
} }
if (i->name == state.s.contentAddressed && state.forceBool(*i->value, pos, context_below)) { switch (i->name.getId()) {
case EvalState::s.contentAddressed.getId():
if (state.forceBool(*i->value, pos, context_below)) {
contentAddressed = true; contentAddressed = true;
experimentalFeatureSettings.require(Xp::CaDerivations); experimentalFeatureSettings.require(Xp::CaDerivations);
} }
break;
else if (i->name == state.s.impure && state.forceBool(*i->value, pos, context_below)) { case EvalState::s.impure.getId():
if (state.forceBool(*i->value, pos, context_below)) {
isImpure = true; isImpure = true;
experimentalFeatureSettings.require(Xp::ImpureDerivations); experimentalFeatureSettings.require(Xp::ImpureDerivations);
} }
break;
/* The `args' attribute is special: it supplies the /* The `args' attribute is special: it supplies the
command-line arguments to the builder. */ command-line arguments to the builder. */
else if (i->name == state.s.args) { case EvalState::s.args.getId():
state.forceList(*i->value, pos, context_below); state.forceList(*i->value, pos, context_below);
for (auto elem : i->value->listView()) { for (auto elem : i->value->listView()) {
auto s = state auto s = state
@ -1475,11 +1478,10 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
.toOwned(); .toOwned();
drv.args.push_back(s); drv.args.push_back(s);
} }
} break;
/* All other attributes are passed to the builder through /* All other attributes are passed to the builder through
the environment. */ the environment. */
else { default:
if (jsonObject) { if (jsonObject) {
@ -1488,49 +1490,69 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
jsonObject->structuredAttrs.emplace(key, printValueAsJSON(state, true, *i->value, pos, context)); jsonObject->structuredAttrs.emplace(key, printValueAsJSON(state, true, *i->value, pos, context));
if (i->name == state.s.builder) switch (i->name.getId()) {
case EvalState::s.builder.getId():
drv.builder = state.forceString(*i->value, context, pos, context_below); drv.builder = state.forceString(*i->value, context, pos, context_below);
else if (i->name == state.s.system) break;
case EvalState::s.system.getId():
drv.platform = state.forceStringNoCtx(*i->value, pos, context_below); drv.platform = state.forceStringNoCtx(*i->value, pos, context_below);
else if (i->name == state.s.outputHash) break;
case EvalState::s.outputHash.getId():
outputHash = state.forceStringNoCtx(*i->value, pos, context_below); outputHash = state.forceStringNoCtx(*i->value, pos, context_below);
else if (i->name == state.s.outputHashAlgo) break;
case EvalState::s.outputHashAlgo.getId():
outputHashAlgo = parseHashAlgoOpt(state.forceStringNoCtx(*i->value, pos, context_below)); outputHashAlgo = parseHashAlgoOpt(state.forceStringNoCtx(*i->value, pos, context_below));
else if (i->name == state.s.outputHashMode) break;
case EvalState::s.outputHashMode.getId():
handleHashMode(state.forceStringNoCtx(*i->value, pos, context_below)); handleHashMode(state.forceStringNoCtx(*i->value, pos, context_below));
else if (i->name == state.s.outputs) { break;
/* Require outputs to be a list of strings. */ case EvalState::s.outputs.getId(): {
/* Require 'outputs' to be a list of strings. */
state.forceList(*i->value, pos, context_below); state.forceList(*i->value, pos, context_below);
Strings ss; Strings ss;
for (auto elem : i->value->listView()) for (auto elem : i->value->listView())
ss.emplace_back(state.forceStringNoCtx(*elem, pos, context_below)); ss.emplace_back(state.forceStringNoCtx(*elem, pos, context_below));
handleOutputs(ss); handleOutputs(ss);
break;
}
default:
break;
} }
if (i->name == state.s.allowedReferences) switch (i->name.getId()) {
case EvalState::s.allowedReferences.getId():
warn( warn(
"In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'allowedReferences'; use 'outputChecks.<output>.allowedReferences' instead", "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'allowedReferences'; use 'outputChecks.<output>.allowedReferences' instead",
drvName); drvName);
if (i->name == state.s.allowedRequisites) break;
case EvalState::s.allowedRequisites.getId():
warn( warn(
"In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'allowedRequisites'; use 'outputChecks.<output>.allowedRequisites' instead", "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'allowedRequisites'; use 'outputChecks.<output>.allowedRequisites' instead",
drvName); drvName);
if (i->name == state.s.disallowedReferences) break;
case EvalState::s.disallowedReferences.getId():
warn( warn(
"In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedReferences'; use 'outputChecks.<output>.disallowedReferences' instead", "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedReferences'; use 'outputChecks.<output>.disallowedReferences' instead",
drvName); drvName);
if (i->name == state.s.disallowedRequisites) break;
case EvalState::s.disallowedRequisites.getId():
warn( warn(
"In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedRequisites'; use 'outputChecks.<output>.disallowedRequisites' instead", "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedRequisites'; use 'outputChecks.<output>.disallowedRequisites' instead",
drvName); drvName);
if (i->name == state.s.maxSize) break;
case EvalState::s.maxSize.getId():
warn( warn(
"In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'maxSize'; use 'outputChecks.<output>.maxSize' instead", "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'maxSize'; use 'outputChecks.<output>.maxSize' instead",
drvName); drvName);
if (i->name == state.s.maxClosureSize) break;
case EvalState::s.maxClosureSize.getId():
warn( warn(
"In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'maxClosureSize'; use 'outputChecks.<output>.maxClosureSize' instead", "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'maxClosureSize'; use 'outputChecks.<output>.maxClosureSize' instead",
drvName); drvName);
break;
default:
break;
}
} else { } else {
auto s = state.coerceToString(pos, *i->value, context, context_below, true).toOwned(); auto s = state.coerceToString(pos, *i->value, context, context_below, true).toOwned();
@ -1541,21 +1563,32 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
drv.structuredAttrs = StructuredAttrs::parse(s); drv.structuredAttrs = StructuredAttrs::parse(s);
} else { } else {
drv.env.emplace(key, s); drv.env.emplace(key, s);
if (i->name == state.s.builder) switch (i->name.getId()) {
case EvalState::s.builder.getId():
drv.builder = std::move(s); drv.builder = std::move(s);
else if (i->name == state.s.system) break;
case EvalState::s.system.getId():
drv.platform = std::move(s); drv.platform = std::move(s);
else if (i->name == state.s.outputHash) break;
case EvalState::s.outputHash.getId():
outputHash = std::move(s); outputHash = std::move(s);
else if (i->name == state.s.outputHashAlgo) break;
case EvalState::s.outputHashAlgo.getId():
outputHashAlgo = parseHashAlgoOpt(s); outputHashAlgo = parseHashAlgoOpt(s);
else if (i->name == state.s.outputHashMode) break;
case EvalState::s.outputHashMode.getId():
handleHashMode(s); handleHashMode(s);
else if (i->name == state.s.outputs) break;
case EvalState::s.outputs.getId():
handleOutputs(tokenizeString<Strings>(s)); handleOutputs(tokenizeString<Strings>(s));
break;
default:
break;
} }
} }
} }
break;
}
} catch (Error & e) { } catch (Error & e) {
e.addTrace( e.addTrace(