Skip to content

Commit d5bc8b9

Browse files
committed
libexpr: move eval memory allocation to own struct
Co-authored-by: eldritch horrors <[email protected]> Co-authored-by: Sergei Zimmerman <[email protected]> See original commit on lix: https://git.lix.systems/lix-project/lix/commit/f5754dc90ae9b1207656d0e29ad2704d3ef1e554
1 parent 4609528 commit d5bc8b9

File tree

10 files changed

+124
-74
lines changed

10 files changed

+124
-74
lines changed

src/libcmd/repl.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -760,7 +760,7 @@ void NixRepl::loadFlake(const std::string & flakeRefS)
760760

761761
void NixRepl::initEnv()
762762
{
763-
env = &state->allocEnv(envSize);
763+
env = &state->mem.allocEnv(envSize);
764764
env->up = &state->baseEnv;
765765
displ = 0;
766766
staticEnv->vars.clear();

src/libexpr-c/nix_api_value.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,7 @@ nix_err nix_bindings_builder_insert(nix_c_context * context, BindingsBuilder * b
679679
context->last_err_code = NIX_OK;
680680
try {
681681
auto & v = check_value_not_null(value);
682-
nix::Symbol s = bb->builder.state.get().symbols.create(name);
682+
nix::Symbol s = bb->builder.symbols.get().create(name);
683683
bb->builder.insert(s, &v);
684684
}
685685
NIXC_CATCH_ERRS

src/libexpr/attr-set.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,27 +10,27 @@ Bindings Bindings::emptyBindings;
1010
/* Allocate a new array of attributes for an attribute set with a specific
1111
capacity. The space is implicitly reserved after the Bindings
1212
structure. */
13-
Bindings * EvalState::allocBindings(size_t capacity)
13+
Bindings * EvalMemory::allocBindings(size_t capacity)
1414
{
1515
if (capacity == 0)
1616
return &Bindings::emptyBindings;
1717
if (capacity > std::numeric_limits<Bindings::size_type>::max())
1818
throw Error("attribute set of size %d is too big", capacity);
19-
nrAttrsets++;
20-
nrAttrsInAttrsets += capacity;
19+
stats.nrAttrsets++;
20+
stats.nrAttrsInAttrsets += capacity;
2121
return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings();
2222
}
2323

2424
Value & BindingsBuilder::alloc(Symbol name, PosIdx pos)
2525
{
26-
auto value = state.get().allocValue();
26+
auto value = mem.get().allocValue();
2727
bindings->push_back(Attr(name, value, pos));
2828
return *value;
2929
}
3030

3131
Value & BindingsBuilder::alloc(std::string_view name, PosIdx pos)
3232
{
33-
return alloc(state.get().symbols.create(name), pos);
33+
return alloc(symbols.get().create(name), pos);
3434
}
3535

3636
void Bindings::sort()

src/libexpr/eval.cc

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,15 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env)
194194

195195
static constexpr size_t BASE_ENV_SIZE = 128;
196196

197+
EvalMemory::EvalMemory()
198+
#if NIX_USE_BOEHMGC
199+
: valueAllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
200+
, env1AllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
201+
#endif
202+
{
203+
assertGCInitialized();
204+
}
205+
197206
EvalState::EvalState(
198207
const LookupPath & lookupPathFromArguments,
199208
ref<Store> store,
@@ -274,12 +283,10 @@ EvalState::EvalState(
274283
, fileEvalCache(make_ref<decltype(fileEvalCache)::element_type>())
275284
, regexCache(makeRegexCache())
276285
#if NIX_USE_BOEHMGC
277-
, valueAllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
278-
, env1AllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
279-
, baseEnvP(std::allocate_shared<Env *>(traceable_allocator<Env *>(), &allocEnv(BASE_ENV_SIZE)))
286+
, baseEnvP(std::allocate_shared<Env *>(traceable_allocator<Env *>(), &mem.allocEnv(BASE_ENV_SIZE)))
280287
, baseEnv(**baseEnvP)
281288
#else
282-
, baseEnv(allocEnv(BASE_ENV_SIZE))
289+
, baseEnv(mem.allocEnv(BASE_ENV_SIZE))
283290
#endif
284291
, staticBaseEnv{std::make_shared<StaticEnv>(nullptr, nullptr)}
285292
{
@@ -288,8 +295,6 @@ EvalState::EvalState(
288295

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

291-
assertGCInitialized();
292-
293298
static_assert(sizeof(Env) <= 16, "environment must be <= 16 bytes");
294299
static_assert(sizeof(Counter) == 64, "counters must be 64 bytes");
295300

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

888-
ListBuilder::ListBuilder(EvalState & state, size_t size)
893+
ListBuilder::ListBuilder(size_t size)
889894
: size(size)
890895
, elems(size <= 2 ? inlineElems : (Value **) allocBytes(size * sizeof(Value *)))
891896
{
892-
state.nrListElems += size;
893897
}
894898

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

11841188
Env * ExprAttrs::buildInheritFromEnv(EvalState & state, Env & up)
11851189
{
1186-
Env & inheritEnv = state.allocEnv(inheritFromExprs->size());
1190+
Env & inheritEnv = state.mem.allocEnv(inheritFromExprs->size());
11871191
inheritEnv.up = &up;
11881192

11891193
Displacement displ = 0;
@@ -1202,7 +1206,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
12021206
if (recursive) {
12031207
/* Create a new environment that contains the attributes in
12041208
this `rec'. */
1205-
Env & env2(state.allocEnv(attrs.size()));
1209+
Env & env2(state.mem.allocEnv(attrs.size()));
12061210
env2.up = &env;
12071211
dynamicEnv = &env2;
12081212
Env * inheritEnv = inheritFromExprs ? buildInheritFromEnv(state, env2) : nullptr;
@@ -1294,7 +1298,7 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)
12941298
{
12951299
/* Create a new environment that contains the attributes in this
12961300
`let'. */
1297-
Env & env2(state.allocEnv(attrs->attrs.size()));
1301+
Env & env2(state.mem.allocEnv(attrs->attrs.size()));
12981302
env2.up = &env;
12991303

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

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

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

17901794
void ExprWith::eval(EvalState & state, Env & env, Value & v)
17911795
{
1792-
Env & env2(state.allocEnv(1));
1796+
Env & env2(state.mem.allocEnv(1));
17931797
env2.up = &env;
17941798
env2.values[0] = attrs->maybeThunk(state, env);
17951799

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

2919-
uint64_t bEnvs = nrEnvs * sizeof(Env) + nrValuesInEnvs * sizeof(Value *);
2920-
uint64_t bLists = nrListElems * sizeof(Value *);
2921-
uint64_t bValues = nrValues * sizeof(Value);
2922-
uint64_t bAttrsets = nrAttrsets * sizeof(Bindings) + nrAttrsInAttrsets * sizeof(Attr);
2923+
auto & memstats = mem.getStats();
2924+
2925+
uint64_t bEnvs = memstats.nrEnvs * sizeof(Env) + memstats.nrValuesInEnvs * sizeof(Value *);
2926+
uint64_t bLists = memstats.nrListElems * sizeof(Value *);
2927+
uint64_t bValues = memstats.nrValues * sizeof(Value);
2928+
uint64_t bAttrsets = memstats.nrAttrsets * sizeof(Bindings) + memstats.nrAttrsInAttrsets * sizeof(Attr);
29232929

29242930
#if NIX_USE_BOEHMGC
29252931
GC_word heapSize, totalBytes;
@@ -2945,28 +2951,28 @@ void EvalState::printStatistics()
29452951
#endif
29462952
};
29472953
topObj["envs"] = {
2948-
{"number", nrEnvs.load()},
2949-
{"elements", nrValuesInEnvs.load()},
2954+
{"number", memstats.nrEnvs.load()},
2955+
{"elements", memstats.nrValuesInEnvs.load()},
29502956
{"bytes", bEnvs},
29512957
};
29522958
topObj["nrExprs"] = Expr::nrExprs.load();
29532959
topObj["list"] = {
2954-
{"elements", nrListElems.load()},
2960+
{"elements", memstats.nrListElems.load()},
29552961
{"bytes", bLists},
29562962
{"concats", nrListConcats.load()},
29572963
};
29582964
topObj["values"] = {
2959-
{"number", nrValues.load()},
2965+
{"number", memstats.nrValues.load()},
29602966
{"bytes", bValues},
29612967
};
29622968
topObj["symbols"] = {
29632969
{"number", symbols.size()},
29642970
{"bytes", symbols.totalSize()},
29652971
};
29662972
topObj["sets"] = {
2967-
{"number", nrAttrsets.load()},
2973+
{"number", memstats.nrAttrsets.load()},
29682974
{"bytes", bAttrsets},
2969-
{"elements", nrAttrsInAttrsets.load()},
2975+
{"elements", memstats.nrAttrsInAttrsets.load()},
29702976
};
29712977
topObj["sizes"] = {
29722978
{"Env", sizeof(Env)},

src/libexpr/include/nix/expr/attr-set.hh

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
namespace nix {
1515

16-
class EvalState;
16+
class EvalMemory;
1717
struct Value;
1818

1919
/**
@@ -426,7 +426,7 @@ public:
426426
return res;
427427
}
428428

429-
friend class EvalState;
429+
friend class EvalMemory;
430430
};
431431

432432
static_assert(std::forward_iterator<Bindings::iterator>);
@@ -448,12 +448,13 @@ private:
448448
Bindings * bindings;
449449
Bindings::size_type capacity_;
450450

451-
friend class EvalState;
451+
friend class EvalMemory;
452452

453-
BindingsBuilder(EvalState & state, Bindings * bindings, size_type capacity)
453+
BindingsBuilder(EvalMemory & mem, SymbolTable & symbols, Bindings * bindings, size_type capacity)
454454
: bindings(bindings)
455455
, capacity_(capacity)
456-
, state(state)
456+
, mem(mem)
457+
, symbols(symbols)
457458
{
458459
}
459460

@@ -471,7 +472,8 @@ private:
471472
}
472473

473474
public:
474-
std::reference_wrapper<EvalState> state;
475+
std::reference_wrapper<EvalMemory> mem;
476+
std::reference_wrapper<SymbolTable> symbols;
475477

476478
void insert(Symbol name, Value * value, PosIdx pos = noPos)
477479
{

src/libexpr/include/nix/expr/eval-inline.hh

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ inline void * allocBytes(size_t n)
2626
}
2727

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

51-
nrValues++;
51+
stats.nrValues++;
5252
return (Value *) p;
5353
}
5454

5555
[[gnu::always_inline]]
56-
Env & EvalState::allocEnv(size_t size)
56+
Env & EvalMemory::allocEnv(size_t size)
5757
{
58-
nrEnvs++;
59-
nrValuesInEnvs += size;
58+
stats.nrEnvs++;
59+
stats.nrValuesInEnvs += size;
6060

6161
Env * env;
6262

0 commit comments

Comments
 (0)