Skip to content

libexpr: move eval memory allocation to own struct #14047

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 26, 2025
Merged

libexpr: move eval memory allocation to own struct #14047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/libcmd/repl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ void NixRepl::loadFlake(const std::string & flakeRefS)

void NixRepl::initEnv()
{
env = &state->allocEnv(envSize);
env = &state->mem.allocEnv(envSize);
env->up = &state->baseEnv;
displ = 0;
staticEnv->vars.clear();
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr-c/nix_api_value.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ nix_err nix_bindings_builder_insert(nix_c_context * context, BindingsBuilder * b
context->last_err_code = NIX_OK;
try {
auto & v = check_value_not_null(value);
nix::Symbol s = bb->builder.state.get().symbols.create(name);
nix::Symbol s = bb->builder.symbols.get().create(name);
bb->builder.insert(s, &v);
}
NIXC_CATCH_ERRS
Expand Down
10 changes: 5 additions & 5 deletions src/libexpr/attr-set.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,27 @@ Bindings Bindings::emptyBindings;
/* Allocate a new array of attributes for an attribute set with a specific
capacity. The space is implicitly reserved after the Bindings
structure. */
Bindings * EvalState::allocBindings(size_t capacity)
Bindings * EvalMemory::allocBindings(size_t capacity)
{
if (capacity == 0)
return &Bindings::emptyBindings;
if (capacity > std::numeric_limits<Bindings::size_type>::max())
throw Error("attribute set of size %d is too big", capacity);
nrAttrsets++;
nrAttrsInAttrsets += capacity;
stats.nrAttrsets++;
stats.nrAttrsInAttrsets += capacity;
return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings();
}

Value & BindingsBuilder::alloc(Symbol name, PosIdx pos)
{
auto value = state.get().allocValue();
auto value = mem.get().allocValue();
bindings->push_back(Attr(name, value, pos));
return *value;
}

Value & BindingsBuilder::alloc(std::string_view name, PosIdx pos)
{
return alloc(state.get().symbols.create(name), pos);
return alloc(symbols.get().create(name), pos);
}

void Bindings::sort()
Expand Down
52 changes: 29 additions & 23 deletions src/libexpr/eval.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,15 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env)

static constexpr size_t BASE_ENV_SIZE = 128;

EvalMemory::EvalMemory()
#if NIX_USE_BOEHMGC
: valueAllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
, env1AllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
#endif
{
assertGCInitialized();
}

EvalState::EvalState(
const LookupPath & lookupPathFromArguments,
ref<Store> store,
Expand Down Expand Up @@ -274,12 +283,10 @@ EvalState::EvalState(
, fileEvalCache(make_ref<decltype(fileEvalCache)::element_type>())
, regexCache(makeRegexCache())
#if NIX_USE_BOEHMGC
, valueAllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
, env1AllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
, baseEnvP(std::allocate_shared<Env *>(traceable_allocator<Env *>(), &allocEnv(BASE_ENV_SIZE)))
, baseEnvP(std::allocate_shared<Env *>(traceable_allocator<Env *>(), &mem.allocEnv(BASE_ENV_SIZE)))
, baseEnv(**baseEnvP)
#else
, baseEnv(allocEnv(BASE_ENV_SIZE))
, baseEnv(mem.allocEnv(BASE_ENV_SIZE))
#endif
, staticBaseEnv{std::make_shared<StaticEnv>(nullptr, nullptr)}
{
Expand All @@ -288,8 +295,6 @@ EvalState::EvalState(

countCalls = getEnv("NIX_COUNT_CALLS").value_or("0") != "0";

assertGCInitialized();

static_assert(sizeof(Env) <= 16, "environment must be <= 16 bytes");
static_assert(sizeof(Counter) == 64, "counters must be 64 bytes");

Expand Down Expand Up @@ -885,11 +890,10 @@ inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval)
}
}

ListBuilder::ListBuilder(EvalState & state, size_t size)
ListBuilder::ListBuilder(size_t size)
: size(size)
, elems(size <= 2 ? inlineElems : (Value **) allocBytes(size * sizeof(Value *)))
{
state.nrListElems += size;
Copy link
Member

@Ericson2314 Ericson2314 Sep 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure that every ListBuilder is going through the new mem.listBuilder?

OK nevermind, I thought there was some sort of default arg but there is not. This is easy to verify.

Copy link
Contributor Author

@Radvendii Radvendii Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I search the codebase, I don't find any other instances. Neither with LSP goto-references nor with a text search for ListBuilder. But it could be getting initialized in some way that is tripping up both searches.

}

Value * EvalState::getBool(bool b)
Expand Down Expand Up @@ -1183,7 +1187,7 @@ void ExprPath::eval(EvalState & state, Env & env, Value & v)

Env * ExprAttrs::buildInheritFromEnv(EvalState & state, Env & up)
{
Env & inheritEnv = state.allocEnv(inheritFromExprs->size());
Env & inheritEnv = state.mem.allocEnv(inheritFromExprs->size());
inheritEnv.up = &up;

Displacement displ = 0;
Expand All @@ -1202,7 +1206,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
if (recursive) {
/* Create a new environment that contains the attributes in
this `rec'. */
Env & env2(state.allocEnv(attrs.size()));
Env & env2(state.mem.allocEnv(attrs.size()));
env2.up = &env;
dynamicEnv = &env2;
Env * inheritEnv = inheritFromExprs ? buildInheritFromEnv(state, env2) : nullptr;
Expand Down Expand Up @@ -1294,7 +1298,7 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)
{
/* Create a new environment that contains the attributes in this
`let'. */
Env & env2(state.allocEnv(attrs->attrs.size()));
Env & env2(state.mem.allocEnv(attrs->attrs.size()));
env2.up = &env;

Env * inheritEnv = attrs->inheritFromExprs ? attrs->buildInheritFromEnv(state, env2) : nullptr;
Expand Down Expand Up @@ -1500,7 +1504,7 @@ void EvalState::callFunction(Value & fun, std::span<Value *> args, Value & vRes,
ExprLambda & lambda(*vCur.lambda().fun);

auto size = (!lambda.arg ? 0 : 1) + (lambda.hasFormals() ? lambda.formals->formals.size() : 0);
Env & env2(allocEnv(size));
Env & env2(mem.allocEnv(size));
env2.up = vCur.lambda().env;

Displacement displ = 0;
Expand Down Expand Up @@ -1789,7 +1793,7 @@ values, or passed explicitly with '--arg' or '--argstr'. See

void ExprWith::eval(EvalState & state, Env & env, Value & v)
{
Env & env2(state.allocEnv(1));
Env & env2(state.mem.allocEnv(1));
env2.up = &env;
env2.values[0] = attrs->maybeThunk(state, env);

Expand Down Expand Up @@ -2916,10 +2920,12 @@ void EvalState::printStatistics()
std::chrono::microseconds cpuTimeDuration = getCpuUserTime();
float cpuTime = std::chrono::duration_cast<std::chrono::duration<float>>(cpuTimeDuration).count();

uint64_t bEnvs = nrEnvs * sizeof(Env) + nrValuesInEnvs * sizeof(Value *);
uint64_t bLists = nrListElems * sizeof(Value *);
uint64_t bValues = nrValues * sizeof(Value);
uint64_t bAttrsets = nrAttrsets * sizeof(Bindings) + nrAttrsInAttrsets * sizeof(Attr);
auto & memstats = mem.getStats();

uint64_t bEnvs = memstats.nrEnvs * sizeof(Env) + memstats.nrValuesInEnvs * sizeof(Value *);
uint64_t bLists = memstats.nrListElems * sizeof(Value *);
uint64_t bValues = memstats.nrValues * sizeof(Value);
uint64_t bAttrsets = memstats.nrAttrsets * sizeof(Bindings) + memstats.nrAttrsInAttrsets * sizeof(Attr);

#if NIX_USE_BOEHMGC
GC_word heapSize, totalBytes;
Expand All @@ -2945,28 +2951,28 @@ void EvalState::printStatistics()
#endif
};
topObj["envs"] = {
{"number", nrEnvs.load()},
{"elements", nrValuesInEnvs.load()},
{"number", memstats.nrEnvs.load()},
{"elements", memstats.nrValuesInEnvs.load()},
{"bytes", bEnvs},
};
topObj["nrExprs"] = Expr::nrExprs.load();
topObj["list"] = {
{"elements", nrListElems.load()},
{"elements", memstats.nrListElems.load()},
{"bytes", bLists},
{"concats", nrListConcats.load()},
};
topObj["values"] = {
{"number", nrValues.load()},
{"number", memstats.nrValues.load()},
{"bytes", bValues},
};
topObj["symbols"] = {
{"number", symbols.size()},
{"bytes", symbols.totalSize()},
};
topObj["sets"] = {
{"number", nrAttrsets.load()},
{"number", memstats.nrAttrsets.load()},
{"bytes", bAttrsets},
{"elements", nrAttrsInAttrsets.load()},
{"elements", memstats.nrAttrsInAttrsets.load()},
};
topObj["sizes"] = {
{"Env", sizeof(Env)},
Expand Down
14 changes: 8 additions & 6 deletions src/libexpr/include/nix/expr/attr-set.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace nix {

class EvalState;
class EvalMemory;
struct Value;

/**
Expand Down Expand Up @@ -426,7 +426,7 @@ public:
return res;
}

friend class EvalState;
friend class EvalMemory;
};

static_assert(std::forward_iterator<Bindings::iterator>);
Expand All @@ -448,12 +448,13 @@ private:
Bindings * bindings;
Bindings::size_type capacity_;

friend class EvalState;
friend class EvalMemory;

BindingsBuilder(EvalState & state, Bindings * bindings, size_type capacity)
BindingsBuilder(EvalMemory & mem, SymbolTable & symbols, Bindings * bindings, size_type capacity)
: bindings(bindings)
, capacity_(capacity)
, state(state)
, mem(mem)
, symbols(symbols)
{
}

Expand All @@ -471,7 +472,8 @@ private:
}

public:
std::reference_wrapper<EvalState> state;
std::reference_wrapper<EvalMemory> mem;
std::reference_wrapper<SymbolTable> symbols;

void insert(Symbol name, Value * value, PosIdx pos = noPos)
{
Expand Down
10 changes: 5 additions & 5 deletions src/libexpr/include/nix/expr/eval-inline.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ inline void * allocBytes(size_t n)
}

[[gnu::always_inline]]
Value * EvalState::allocValue()
Value * EvalMemory::allocValue()
{
#if NIX_USE_BOEHMGC
/* We use the boehm batch allocator to speed up allocations of Values (of which there are many).
Expand All @@ -48,15 +48,15 @@ Value * EvalState::allocValue()
void * p = allocBytes(sizeof(Value));
#endif

nrValues++;
stats.nrValues++;
return (Value *) p;
}

[[gnu::always_inline]]
Env & EvalState::allocEnv(size_t size)
Env & EvalMemory::allocEnv(size_t size)
{
nrEnvs++;
nrValuesInEnvs += size;
stats.nrEnvs++;
stats.nrValuesInEnvs += size;

Env * env;

Expand Down
Loading
Loading