Merge pull request #13919 from xokdvium/smaller-bindings

libexpr: Slim down Bindings to 8 bytes (on 64 bit systems)

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

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);
 

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()

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()) {

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

@@ -5,6 +5,7 @@
 #include "nix/expr/symbol-table.hh"
 
 #include <algorithm>
+#include <functional>
 
 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<EvalState> 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;

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)