From d37a5d400059601f0c4b30ccca5a7ba138caacf5 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 28 Sep 2025 00:02:20 +0200 Subject: [PATCH] libexpr: don't inline small lists into values this has no measurable performance impact thanks to the new caches. Change-Id: Ib403a9a567161675f78e8c5d314d6340183d181d --- lix/libexpr/eval.cc | 4 +- lix/libexpr/value.hh | 69 +++++++---------------- tests/functional/lang/eval-okay-print.exp | 2 +- tests/unit/libexpr/value/print.cc | 42 +++++++------- 4 files changed, 42 insertions(+), 75 deletions(-) diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index 1010edd05..9aafba783 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -858,9 +858,7 @@ Value EvalMemory::newList(size_t size) { Value v; v.mkList(size); - if (size > 2) { - v._bigList.elems = allocType(size); - } + v._list.elems = allocType(size); stats.nrListElems += size; return v; } diff --git a/lix/libexpr/value.hh b/lix/libexpr/value.hh index 430b237c9..e9cbce3b7 100644 --- a/lix/libexpr/value.hh +++ b/lix/libexpr/value.hh @@ -20,7 +20,6 @@ namespace nix { class BindingsBuilder; - typedef enum { tInt = 1, tBool, @@ -28,9 +27,7 @@ typedef enum { tPath, tNull, tAttrs, - tList1, - tList2, - tListN, + tList, tThunk, tApp, tLambda, @@ -378,19 +375,9 @@ public: /// allocating memory. Value(list_t, std::span items) { - if (items.size() == 1) { - this->internalType = tList1; - this->_smallList[0] = items[0]; - this->_smallList[1] = nullptr; - } else if (items.size() == 2) { - this->internalType = tList2; - this->_smallList[0] = items[0]; - this->_smallList[1] = items[1]; - } else { - this->internalType = tListN; - this->_bigList.size = items.size(); - this->_bigList.elems = items.data(); - } + this->internalType = tList; + this->_list.size = items.size(); + this->_list.elems = items.data(); } /// Constructs a nix language value of type "list", with an element array @@ -406,24 +393,12 @@ public: > Value(list_t, SizedIterableT & items, TransformerT const & transformer) { - if (items.size() == 1) { - this->internalType = tList1; - this->_smallList[0] = transformer(*items.begin()); - this->_smallList[1] = nullptr; - } else if (items.size() == 2) { - this->internalType = tList2; - auto it = items.begin(); - this->_smallList[0] = transformer(*it); - it++; - this->_smallList[1] = transformer(*it); - } else { - this->internalType = tListN; - this->_bigList.size = items.size(); - this->_bigList.elems = gcAllocType(items.size()); - auto it = items.begin(); - for (size_t i = 0; i < items.size(); i++, it++) { - this->_bigList.elems[i] = transformer(*it); - } + this->internalType = tList; + this->_list.size = items.size(); + this->_list.elems = gcAllocType(items.size()); + auto it = items.begin(); + for (size_t i = 0; i < items.size(); i++, it++) { + this->_list.elems[i] = transformer(*it); } } @@ -594,8 +569,7 @@ public: struct { size_t size; Value * * elems; - } _bigList; - Value * _smallList[2]; + } _list; struct { Env * env; Expr * expr; @@ -640,7 +614,8 @@ public: case tPath: return nPath; case tNull: return nNull; case tAttrs: return nAttrs; - case tList1: case tList2: case tListN: return nList; + case tList: + return nList; case tLambda: case tPrimOp: case tPrimOpApp: return nFunction; case tExternal: return nExternal; case tFloat: return nFloat; @@ -720,14 +695,8 @@ public: inline void mkList(size_t size) { clearValue(); - if (size == 1) - internalType = tList1; - else if (size == 2) - internalType = tList2; - else { - internalType = tListN; - _bigList.size = size; - } + internalType = tList; + _list.size = size; } inline void mkThunk(Env * e, Expr & ex) @@ -787,22 +756,22 @@ public: bool isList() const { - return internalType == tList1 || internalType == tList2 || internalType == tListN; + return internalType == tList; } Value * * listElems() { - return internalType == tList1 || internalType == tList2 ? _smallList : _bigList.elems; + return _list.elems; } Value * const * listElems() const { - return internalType == tList1 || internalType == tList2 ? _smallList : _bigList.elems; + return _list.elems; } size_t listSize() const { - return internalType == tList1 ? 1 : internalType == tList2 ? 2 : _bigList.size; + return _list.size; } /** diff --git a/tests/functional/lang/eval-okay-print.exp b/tests/functional/lang/eval-okay-print.exp index 0d960fb70..7f3ad3733 100644 --- a/tests/functional/lang/eval-okay-print.exp +++ b/tests/functional/lang/eval-okay-print.exp @@ -1 +1 @@ -[ null [ [ «repeated» ] ] ] +[ null [ «repeated» ] ] diff --git a/tests/unit/libexpr/value/print.cc b/tests/unit/libexpr/value/print.cc index ef77f6a07..c2296cbad 100644 --- a/tests/unit/libexpr/value/print.cc +++ b/tests/unit/libexpr/value/print.cc @@ -83,9 +83,9 @@ TEST_F(ValuePrintingTests, tList) vTwo.mkInt(2); Value vList = evaluator.mem.newList(5); - vList._bigList.elems[0] = &vOne; - vList._bigList.elems[1] = &vTwo; - vList._bigList.size = 3; + vList._list.elems[0] = &vOne; + vList._list.elems[1] = &vTwo; + vList._list.size = 3; test(vList, "[ 1 2 «nullptr» ]"); } @@ -261,10 +261,10 @@ TEST_F(ValuePrintingTests, depthList) vNested.mkAttrs(builder2.finish()); Value vList = evaluator.mem.newList(5); - vList._bigList.elems[0] = &vOne; - vList._bigList.elems[1] = &vTwo; - vList._bigList.elems[2] = &vNested; - vList._bigList.size = 3; + vList._list.elems[0] = &vOne; + vList._list.elems[1] = &vTwo; + vList._list.elems[2] = &vNested; + vList._list.size = 3; test(vList, "[ 1 2 { ... } ]", PrintOptions { .maxDepth = 1 }); test(vList, "[ 1 2 { nested = { ... }; one = 1; two = 2; } ]", PrintOptions { .maxDepth = 2 }); @@ -534,9 +534,9 @@ TEST_F(ValuePrintingTests, ansiColorsList) vTwo.mkInt(2); Value vList = evaluator.mem.newList(5); - vList._bigList.elems[0] = &vOne; - vList._bigList.elems[1] = &vTwo; - vList._bigList.size = 3; + vList._list.elems[0] = &vOne; + vList._list.elems[1] = &vTwo; + vList._list.size = 3; test(vList, "[ " ANSI_CYAN "1" ANSI_NORMAL " " ANSI_CYAN "2" ANSI_NORMAL " " ANSI_MAGENTA "«nullptr»" ANSI_NORMAL " ]", @@ -672,9 +672,9 @@ TEST_F(ValuePrintingTests, ansiColorsListRepeated) vInner.mkAttrs(innerBuilder.finish()); Value vList = evaluator.mem.newList(3); - vList._bigList.elems[0] = &vInner; - vList._bigList.elems[1] = &vInner; - vList._bigList.size = 2; + vList._list.elems[0] = &vInner; + vList._list.elems[1] = &vInner; + vList._list.size = 2; test(vList, "[ { x = " ANSI_CYAN "0" ANSI_NORMAL "; } " ANSI_MAGENTA "«repeated»" ANSI_NORMAL " ]", @@ -695,9 +695,9 @@ TEST_F(ValuePrintingTests, listRepeated) vInner.mkAttrs(innerBuilder.finish()); Value vList = evaluator.mem.newList(3); - vList._bigList.elems[0] = &vInner; - vList._bigList.elems[1] = &vInner; - vList._bigList.size = 2; + vList._list.elems[0] = &vInner; + vList._list.elems[1] = &vInner; + vList._list.size = 2; test(vList, "[ { x = 0; } «repeated» ]", PrintOptions { }); test(vList, @@ -752,9 +752,9 @@ TEST_F(ValuePrintingTests, ansiColorsListElided) vTwo.mkInt(2); Value vList = evaluator.mem.newList(4); - vList._bigList.elems[0] = &vOne; - vList._bigList.elems[1] = &vTwo; - vList._bigList.size = 2; + vList._list.elems[0] = &vOne; + vList._list.elems[1] = &vTwo; + vList._list.size = 2; test(vList, "[ " ANSI_CYAN "1" ANSI_NORMAL " " ANSI_FAINT "«1 item elided»" ANSI_NORMAL " ]", @@ -766,8 +766,8 @@ TEST_F(ValuePrintingTests, ansiColorsListElided) Value vThree; vThree.mkInt(3); - vList._bigList.elems[2] = &vThree; - vList._bigList.size = 3; + vList._list.elems[2] = &vThree; + vList._list.size = 3; test(vList, "[ " ANSI_CYAN "1" ANSI_NORMAL " " ANSI_FAINT "«2 items elided»" ANSI_NORMAL " ]",