From 738924b70564294e0ecb361a795ec7780a6e8bf6 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Sat, 6 Sep 2025 00:23:54 +0300 Subject: [PATCH] libexpr: Slim down Bindings to 8 bytes (on 64 bit systems) Since the only construction and push_back() calls to Bindings happen through the `BindingsBuilder` [1] we don't need to keep `capacity` around on the heap anymore. This saves 8 bytes (because of the member alignment padding) per one Bindings allocation. This isn't that much, but it does save significant memory. This also shows that the Bindings don't necessarily have to be mutable, which opens up opportunities for doing small bindings optimization and storing a 1-element Bindings directly in Value. For the following scenario: nix-env --query --available --out-path --file ../nixpkgs --eval-system x86_64-linux (nixpkgs revision: ddcddd7b09a417ca9a88899f4bd43a8edb72308d) This patch results in reduction of `sets.bytes` 13115104016 -> 12653087640, which amounts to 462 MB less bytes allocated for Bindings. [1]: Not actually, `getBuiltins` does mutate bindings, but this is pretty inconsequential and doesn't lead to problems. --- src/libexpr-c/nix_api_value.cc | 2 +- src/libexpr-tests/value/print.cc | 32 ++++++++-------- src/libexpr/attr-set.cc | 6 +-- src/libexpr/eval.cc | 4 +- src/libexpr/include/nix/expr/attr-set.hh | 49 ++++++++++++------------ src/libexpr/include/nix/expr/eval.hh | 2 +- 6 files changed, 47 insertions(+), 48 deletions(-) diff --git a/src/libexpr-c/nix_api_value.cc b/src/libexpr-c/nix_api_value.cc index 0f6595e49..093daf2f8 100644 --- a/src/libexpr-c/nix_api_value.cc +++ b/src/libexpr-c/nix_api_value.cc @@ -594,7 +594,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.symbols.create(name); + nix::Symbol s = bb->builder.state.get().symbols.create(name); bb->builder.insert(s, &v); } NIXC_CATCH_ERRS diff --git a/src/libexpr-tests/value/print.cc b/src/libexpr-tests/value/print.cc index 739d4e40b..1959fddf2 100644 --- a/src/libexpr-tests/value/print.cc +++ b/src/libexpr-tests/value/print.cc @@ -61,7 +61,7 @@ TEST_F(ValuePrintingTests, tAttrs) Value vTwo; vTwo.mkInt(2); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.symbols.create("one"), &vOne); builder.insert(state.symbols.create("two"), &vTwo); @@ -196,11 +196,11 @@ TEST_F(ValuePrintingTests, depthAttrs) Value vTwo; vTwo.mkInt(2); - BindingsBuilder builderEmpty(state, state.allocBindings(0)); + BindingsBuilder builderEmpty = state.buildBindings(0); Value vAttrsEmpty; vAttrsEmpty.mkAttrs(builderEmpty.finish()); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.symbols.create("one"), &vOne); builder.insert(state.symbols.create("two"), &vTwo); builder.insert(state.symbols.create("nested"), &vAttrsEmpty); @@ -208,7 +208,7 @@ TEST_F(ValuePrintingTests, depthAttrs) Value vAttrs; vAttrs.mkAttrs(builder.finish()); - BindingsBuilder builder2(state, state.allocBindings(10)); + BindingsBuilder builder2 = state.buildBindings(10); builder2.insert(state.symbols.create("one"), &vOne); builder2.insert(state.symbols.create("two"), &vTwo); builder2.insert(state.symbols.create("nested"), &vAttrs); @@ -233,14 +233,14 @@ TEST_F(ValuePrintingTests, depthList) Value vTwo; vTwo.mkInt(2); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.symbols.create("one"), &vOne); builder.insert(state.symbols.create("two"), &vTwo); Value vAttrs; vAttrs.mkAttrs(builder.finish()); - BindingsBuilder builder2(state, state.allocBindings(10)); + BindingsBuilder builder2 = state.buildBindings(10); builder2.insert(state.symbols.create("one"), &vOne); builder2.insert(state.symbols.create("two"), &vTwo); builder2.insert(state.symbols.create("nested"), &vAttrs); @@ -295,7 +295,7 @@ TEST_F(ValuePrintingTests, attrsTypeFirst) Value vApple; vApple.mkStringNoCopy("apple"); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.symbols.create("type"), &vType); builder.insert(state.symbols.create("apple"), &vApple); @@ -374,7 +374,7 @@ TEST_F(ValuePrintingTests, ansiColorsAttrs) Value vTwo; vTwo.mkInt(2); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.symbols.create("one"), &vOne); builder.insert(state.symbols.create("two"), &vTwo); @@ -392,7 +392,7 @@ TEST_F(ValuePrintingTests, ansiColorsDerivation) Value vDerivation; vDerivation.mkStringNoCopy("derivation"); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.s.type, &vDerivation); Value vAttrs; @@ -437,7 +437,7 @@ TEST_F(ValuePrintingTests, ansiColorsDerivationError) Value vDerivation; vDerivation.mkStringNoCopy("derivation"); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.s.type, &vDerivation); builder.insert(state.s.drvPath, &vError); @@ -553,12 +553,12 @@ TEST_F(ValuePrintingTests, ansiColorsBlackhole) TEST_F(ValuePrintingTests, ansiColorsAttrsRepeated) { - BindingsBuilder emptyBuilder(state, state.allocBindings(1)); + BindingsBuilder emptyBuilder = state.buildBindings(1); Value vEmpty; vEmpty.mkAttrs(emptyBuilder.finish()); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.symbols.create("a"), &vEmpty); builder.insert(state.symbols.create("b"), &vEmpty); @@ -570,7 +570,7 @@ TEST_F(ValuePrintingTests, ansiColorsAttrsRepeated) TEST_F(ValuePrintingTests, ansiColorsListRepeated) { - BindingsBuilder emptyBuilder(state, state.allocBindings(1)); + BindingsBuilder emptyBuilder = state.buildBindings(1); Value vEmpty; vEmpty.mkAttrs(emptyBuilder.finish()); @@ -586,7 +586,7 @@ TEST_F(ValuePrintingTests, ansiColorsListRepeated) TEST_F(ValuePrintingTests, listRepeated) { - BindingsBuilder emptyBuilder(state, state.allocBindings(1)); + BindingsBuilder emptyBuilder = state.buildBindings(1); Value vEmpty; vEmpty.mkAttrs(emptyBuilder.finish()); @@ -609,7 +609,7 @@ TEST_F(ValuePrintingTests, ansiColorsAttrsElided) Value vTwo; vTwo.mkInt(2); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.symbols.create("one"), &vOne); builder.insert(state.symbols.create("two"), &vTwo); @@ -635,8 +635,6 @@ TEST_F(ValuePrintingTests, ansiColorsAttrsElided) TEST_F(ValuePrintingTests, ansiColorsListElided) { - BindingsBuilder emptyBuilder(state, state.allocBindings(1)); - Value vOne; vOne.mkInt(1); diff --git a/src/libexpr/attr-set.cc b/src/libexpr/attr-set.cc index 3a06441e9..eb44b0dd9 100644 --- a/src/libexpr/attr-set.cc +++ b/src/libexpr/attr-set.cc @@ -16,19 +16,19 @@ Bindings * EvalState::allocBindings(size_t capacity) throw Error("attribute set of size %d is too big", capacity); nrAttrsets++; nrAttrsInAttrsets += capacity; - return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings((Bindings::size_t) capacity); + return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings(); } Value & BindingsBuilder::alloc(Symbol name, PosIdx pos) { - auto value = state.allocValue(); + auto value = state.get().allocValue(); bindings->push_back(Attr(name, value, pos)); return *value; } Value & BindingsBuilder::alloc(std::string_view name, PosIdx pos) { - return alloc(state.symbols.create(name), pos); + return alloc(state.get().symbols.create(name), pos); } void Bindings::sort() diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index fd2108537..9d740c717 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -205,7 +205,7 @@ EvalState::EvalState( , settings{settings} , symbols(StaticEvalSymbols::staticSymbolTable()) , repair(NoRepair) - , emptyBindings(0) + , emptyBindings(Bindings()) , storeFS(makeMountedSourceAccessor({ {CanonPath::root, makeEmptySourceAccessor()}, /* In the pure eval case, we can simply require @@ -1218,7 +1218,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) *vOverrides, [&]() { return vOverrides->determinePos(noPos); }, "while evaluating the `__overrides` attribute"); - bindings.grow(state.allocBindings(bindings.capacity() + vOverrides->attrs()->size())); + bindings.grow(state.buildBindings(bindings.capacity() + vOverrides->attrs()->size())); for (auto & i : *vOverrides->attrs()) { AttrDefs::iterator j = attrs.find(i.name); if (j != attrs.end()) { diff --git a/src/libexpr/include/nix/expr/attr-set.hh b/src/libexpr/include/nix/expr/attr-set.hh index 85bba1099..b5e927a7e 100644 --- a/src/libexpr/include/nix/expr/attr-set.hh +++ b/src/libexpr/include/nix/expr/attr-set.hh @@ -5,6 +5,7 @@ #include "nix/expr/symbol-table.hh" #include +#include namespace nix { @@ -54,16 +55,14 @@ public: PosIdx pos; private: - size_t size_, capacity_; + size_t size_ = 0; Attr attrs[0]; - Bindings(size_t capacity) - : size_(0) - , capacity_(capacity) - { - } - - Bindings(const Bindings & bindings) = delete; + Bindings() = default; + Bindings(const Bindings &) = delete; + Bindings(Bindings &&) = delete; + Bindings & operator=(const Bindings &) = delete; + Bindings & operator=(Bindings &&) = delete; public: size_t size() const @@ -82,7 +81,6 @@ public: void push_back(const Attr & attr) { - assert(size_ < capacity_); attrs[size_++] = attr; } @@ -136,11 +134,6 @@ public: void sort(); - size_t capacity() const - { - return capacity_; - } - /** * Returns the attributes in lexicographically sorted order. */ @@ -165,22 +158,29 @@ public: * order at the end. The only way to consume a BindingsBuilder is to * call finish(), which sorts the bindings. */ -class BindingsBuilder +class BindingsBuilder final { - Bindings * bindings; - public: // needed by std::back_inserter using value_type = Attr; + using size_type = Bindings::size_t; - EvalState & state; +private: + Bindings * bindings; + Bindings::size_t capacity_; - BindingsBuilder(EvalState & state, Bindings * bindings) + friend class EvalState; + + BindingsBuilder(EvalState & state, Bindings * bindings, size_type capacity) : bindings(bindings) + , capacity_(capacity) , state(state) { } +public: + std::reference_wrapper state; + void insert(Symbol name, Value * value, PosIdx pos = noPos) { insert(Attr(name, value, pos)); @@ -193,6 +193,7 @@ public: void push_back(const Attr & attr) { + assert(bindings->size() < capacity_); bindings->push_back(attr); } @@ -211,16 +212,16 @@ public: return bindings; } - size_t capacity() + size_t capacity() const noexcept { - return bindings->capacity(); + return capacity_; } - void grow(Bindings * newBindings) + void grow(BindingsBuilder newBindings) { for (auto & i : *bindings) - newBindings->push_back(i); - bindings = newBindings; + newBindings.push_back(i); + std::swap(*this, newBindings); } friend struct ExprAttrs; diff --git a/src/libexpr/include/nix/expr/eval.hh b/src/libexpr/include/nix/expr/eval.hh index 04729b100..5015a009b 100644 --- a/src/libexpr/include/nix/expr/eval.hh +++ b/src/libexpr/include/nix/expr/eval.hh @@ -879,7 +879,7 @@ public: BindingsBuilder buildBindings(size_t capacity) { - return BindingsBuilder(*this, allocBindings(capacity)); + return BindingsBuilder(*this, allocBindings(capacity), capacity); } ListBuilder buildList(size_t size)