refactor(3p/nix/libexpr): Back Nix lists with std::vector r/1377

This change does away with the previous special-casing of lists of
certain element sizes, and the use of raw C-style arrays.

Lists are now backed by a std::vector of nix::Value*, which uses the
traceable GC allocator.

This change is unfortunately quite noisy because the accessor methods
were updated/removed accordingly, so all callsites of Nix-related
lists have changed.

For some operations in primops.cc where keeping the previous code
structure would have been more difficult with a "proper" vector, the
implementation has been replaced with std::vector methods. For
example, list concatenation now uses appropriate range inserts.

Anecdotally the performance of this is about equal, to even slightly
better, than the previous implementation.

All language tests pass and the depot paths I've used for testing
still evaluate.

Change-Id: Ib5eca6c0207429cb323a330c838c3a2200b2c693
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1266
Tested-by: BuildkiteCI
Reviewed-by: isomer <isomer@tvl.fyi>
Reviewed-by: Kane York <rikingcoding@gmail.com>
Reviewed-by: glittershark <grfn@gws.fyi>

1 files changed, 11 insertions, 26 deletions

diff --git a/third_party/nix/src/libexpr/value.hh b/third_party/nix/src/libexpr/value.hh
index cb1b8796cf..35c5e4d0a0 100644
--- a/third_party/nix/src/libexpr/value.hh
+++ b/third_party/nix/src/libexpr/value.hh
@@ -1,6 +1,10 @@
 #pragma once
 
+#include <tuple>
+#include <vector>
+
 #include <gc/gc_allocator.h>
+#include <gc/gc_cpp.h>
 
 #include "libexpr/symbol-table.hh"
 #include "libutil/types.hh"
@@ -14,9 +18,7 @@ typedef enum {
   tPath,
   tNull,
   tAttrs,
-  tList1,
-  tList2,
-  tListN,
+  tList,
   tThunk,
   tApp,
   tLambda,
@@ -119,11 +121,6 @@ struct NixString {
   const char** context;  // must be in sorted order
 };
 
-struct NixBigList {
-  size_t size;
-  Value** elems;
-};
-
 struct NixThunk {
   Env* env;
   Expr* expr;
@@ -142,7 +139,9 @@ struct NixPrimOpApp {
   Value *left, *right;
 };
 
-struct Value {
+using NixList = std::vector<Value*, traceable_allocator<Value*>>;
+
+struct Value : public gc {
   ValueType type;
   union {  // TODO(tazjin): std::variant
     NixInt integer;
@@ -150,8 +149,7 @@ struct Value {
     NixString string;
     const char* path;
     Bindings* attrs;
-    NixBigList bigList;
-    Value* smallList[2];
+    NixList* list;
     NixThunk thunk;
     NixApp app;  // TODO(tazjin): "app"?
     NixLambda lambda;
@@ -161,21 +159,9 @@ struct Value {
     NixFloat fpoint;
   };
 
-  bool isList() const {
-    return type == tList1 || type == tList2 || type == tListN;
-  }
-
-  Value** listElems() {
-    return type == tList1 || type == tList2 ? smallList : bigList.elems;
-  }
-
-  const Value* const* listElems() const {
-    return type == tList1 || type == tList2 ? smallList : bigList.elems;
-  }
+  bool isList() const { return type == tList; }
 
-  size_t listSize() const {
-    return type == tList1 ? 1 : type == tList2 ? 2 : bigList.size;
-  }
+  size_t listSize() const { return list->size(); }
 };
 
 /* After overwriting an app node, be sure to clear pointers in the
@@ -242,7 +228,6 @@ void mkPath(Value& v, const char* s);
    not included. */
 size_t valueSize(Value& v);
 
-typedef std::vector<Value*, traceable_allocator<Value*>> ValueVector;
 typedef std::map<Symbol, Value*, std::less<Symbol>,
                  traceable_allocator<std::pair<const Symbol, Value*>>>
     ValueMap;