Allocate RootValues more efficiently

src/libcmd/include/nix/cmd/installable-attr-path.hh

@@ -20,13 +20,14 @@
 
 #include <regex>
 #include <queue>
+#include "nix/expr/root-value.hh"
 
 namespace nix {
 
 class InstallableAttrPath : public InstallableValue
 {
     SourceExprCommand & cmd;
-    RootValue v;
+    UniqueRootValue v;
     std::string attrPath;
     ExtendedOutputsSpec extendedOutputsSpec;
 

src/libcmd/installable-attr-path.cc

@@ -32,7 +32,7 @@ InstallableAttrPath::InstallableAttrPath(
     ExtendedOutputsSpec extendedOutputsSpec)
     : InstallableValue(state)
     , cmd(cmd)
-    , v(allocRootValue(v))
+    , v(UniqueRootValue(v))
     , attrPath(attrPath)
     , extendedOutputsSpec(std::move(extendedOutputsSpec))
 {

src/libcmd/repl.cc

@@ -327,7 +327,7 @@ void NixRepl::loadDebugTraceEnv(DebugTrace & dt)
 
         // add staticenv vars.
         for (auto & [name, value] : *(vm.get()))
-            addVarToScope(state->symbols.create(name), *value);
+            addVarToScope(state->symbols.create(name), **value);
     }
 }
 
@@ -931,7 +931,7 @@ ReplExitStatus AbstractNixRepl::runSimple(ref<EvalState> evalState, const ValMap
 
     // add 'extra' vars.
     for (auto & [name, value] : extraEnv)
-        repl->addVarToScope(repl->state->symbols.create(name), *value);
+        repl->addVarToScope(repl->state->symbols.create(name), **value);
 
     return repl->mainLoop();
 }

src/libexpr/eval-cache.cc

@@ -361,7 +361,7 @@ Value * EvalCache::getRootValue()
 {
     if (!value) {
         debug("getting root value");
-        value = allocRootValue(rootLoader());
+        value = UniqueRootValue(rootLoader());
     }
     return *value;
 }
@@ -378,7 +378,7 @@ AttrCursor::AttrCursor(
     , cachedValue(std::move(cachedValue))
 {
     if (value)
-        _value = allocRootValue(value);
+        _value = UniqueRootValue(value);
 }
 
 AttrKey AttrCursor::getKey()
@@ -401,9 +401,9 @@ Value & AttrCursor::getValue()
             auto attr = vParent.attrs()->get(parent->second);
             if (!attr)
                 throw Error("attribute '%s' is unexpectedly missing", getAttrPathStr());
-            _value = allocRootValue(attr->value);
+            _value = UniqueRootValue(attr->value);
         } else
-            _value = allocRootValue(root->getRootValue());
+            _value = UniqueRootValue(root->getRootValue());
     }
     return **_value;
 }

src/libexpr/eval.cc

@@ -109,11 +109,6 @@ const StringData & StringData::make(EvalMemory & mem, std::string_view s)
     return res;
 }
 
-RootValue allocRootValue(Value * v)
-{
-    return std::allocate_shared<Value *>(traceable_allocator<Value *>(), v);
-}
-
 // Pretty print types for assertion errors
 std::ostream & operator<<(std::ostream & os, const ValueType t)
 {
@@ -580,7 +575,7 @@ Value * EvalState::addPrimOp(PrimOp && primOp)
     v->mkPrimOp(new PrimOp(primOp));
 
     if (primOp.internal)
-        internalPrimOps.emplace(primOp.name, v);
+        internalPrimOps.emplace(primOp.name, UniqueRootValue(v));
     else {
         staticBaseEnv->vars.emplace_back(envName, baseEnvDispl);
         baseEnv.values[baseEnvDispl++] = v;
@@ -754,11 +749,11 @@ void mapStaticEnvBindings(const SymbolTable & st, const StaticEnv & se, const En
         if (se.isWith && env.values[0]->isFinished()) {
             // add 'with' bindings.
             for (auto & j : *env.values[0]->attrs())
-                vm.insert_or_assign(std::string(st[j.name]), j.value);
+                vm.insert_or_assign(std::string(st[j.name]), UniqueRootValue(j.value));
         } else {
             // iterate through staticenv bindings and add them.
             for (auto & i : se.vars)
-                vm.insert_or_assign(std::string(st[i.first]), env.values[i.second]);
+                vm.insert_or_assign(std::string(st[i.first]), UniqueRootValue(env.values[i.second]));
         }
     }
 }
@@ -1167,24 +1162,28 @@ void EvalState::evalFile(const SourcePath & path, Value & v, bool mustBeTrivial)
         importResolutionCache->emplace(path, *resolvedPath);
     }
 
-    if (auto v2 = getConcurrent(*fileEvalCache, *resolvedPath)) {
-        forceValue(**v2, noPos);
-        v = **v2;
-        return;
+    {
+        Value * v2 = nullptr;
+        fileEvalCache->cvisit(*resolvedPath, [&](auto & i) { v2 = *i.second; });
+        if (v2) {
+            forceValue(*v2, noPos);
+            v = *v2;
+            return;
+        }
     }
 
     Value * vExpr;
     ExprParseFile expr{*resolvedPath, mustBeTrivial};
 
     fileEvalCache->try_emplace_and_cvisit(
         *resolvedPath,
-        nullptr,
+        UniqueRootValue(nullptr),
         [&](auto & i) {
             vExpr = allocValue();
             vExpr->mkThunk(&baseEnv, &expr);
-            i.second = vExpr;
+            *i.second = vExpr;
         },
-        [&](auto & i) { vExpr = i.second; });
+        [&](auto & i) { vExpr = *i.second; });
 
     forceValue(*vExpr, noPos);
 

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

@@ -8,6 +8,7 @@
 
 #include <functional>
 #include <variant>
+#include "nix/expr/root-value.hh"
 
 namespace nix::eval_cache {
 
@@ -44,7 +45,7 @@ public:
 private:
     typedef fun<Value *()> RootLoader;
     RootLoader rootLoader;
-    RootValue value;
+    UniqueRootValue value;
 
     Value * getRootValue();
 
@@ -111,7 +112,7 @@ public:
 private:
     using Parent = std::optional<std::pair<ref<AttrCursor>, Symbol>>;
     const Parent parent;
-    RootValue _value;
+    UniqueRootValue _value;
     std::optional<std::pair<AttrId, AttrValue>> cachedValue;
 
     AttrKey getKey();

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

@@ -6,6 +6,7 @@
 #include "nix/expr/eval-profiler.hh"
 #include "nix/util/types.hh"
 #include "nix/expr/value.hh"
+#include "nix/expr/root-value.hh"
 #include "nix/expr/nixexpr.hh"
 #include "nix/expr/symbol-table.hh"
 #include "nix/util/configuration.hh"
@@ -167,9 +168,7 @@ struct Constant
     bool impureOnly = false;
 };
 
-typedef std::
-    map<std::string, Value *, std::less<std::string>, traceable_allocator<std::pair<const std::string, Value *>>>
-        ValMap;
+typedef std::map<std::string, UniqueRootValue> ValMap;
 
 typedef boost::unordered_flat_map<PosIdx, DocComment, std::hash<PosIdx>> DocCommentMap;
 
@@ -473,13 +472,7 @@ private:
     /**
      * A cache from resolved paths to values.
      */
-    const ref<boost::concurrent_flat_map<
-        SourcePath,
-        Value *,
-        std::hash<SourcePath>,
-        std::equal_to<SourcePath>,
-        traceable_allocator<std::pair<const SourcePath, Value *>>>>
-        fileEvalCache;
+    const ref<boost::concurrent_flat_map<SourcePath, UniqueRootValue>> fileEvalCache;
 
     /**
      * Associate source positions of certain AST nodes with their preceding doc comment, if they have one.
@@ -817,13 +810,7 @@ public:
     /**
      * Internal primops not exposed to the user.
      */
-    boost::unordered_flat_map<
-        std::string,
-        Value *,
-        StringViewHash,
-        std::equal_to<>,
-        traceable_allocator<std::pair<const std::string, Value *>>>
-        internalPrimOps;
+    boost::unordered_flat_map<std::string, UniqueRootValue, StringViewHash> internalPrimOps;
 
     /**
      * Name and documentation about every constant.

src/libexpr/include/nix/expr/get-drvs.hh

@@ -32,6 +32,7 @@ private:
      */
     bool failed = false;
 
+    // FIXME: make this a UniqueRootValue.
     const Bindings *attrs = nullptr, *meta = nullptr;
 
     const Bindings * getMeta();

src/libexpr/include/nix/expr/meson.build

@@ -33,6 +33,7 @@ headers = [ config_pub_h ] + files(
   'print.hh',
   'provenance.hh',
   'repl-exit-status.hh',
+  'root-value.hh',
   'search-path.hh',
   'static-string-data.hh',
   'symbol-table.hh',

src/libexpr/include/nix/expr/root-value.hh

@@ -0,0 +1,96 @@
+#pragma once
+///@file
+
+#include <memory>
+#include <utility>
+
+namespace nix {
+
+struct Value;
+
+/**
+ * Allocate a slot from the root value pool, i.e. a GC-visible
+ * `Value *` cell that keeps the value it points to alive across
+ * garbage collections. Use `UniqueRootValue`/`RootValue` rather than
+ * calling this directly.
+ */
+Value ** allocRootValueSlot(Value * v);
+
+/**
+ * Clear the given slot and return it to the root value pool.
+ */
+void freeRootValueSlot(Value ** slot);
+
+/**
+ * A move-only handle rooting a Value, i.e. keeping it and everything
+ * reachable from it alive across garbage collections. Prefer this
+ * over `RootValue` unless the handle must be copyable (e.g. when it's
+ * captured in a `std::function`-backed lambda).
+ */
+class UniqueRootValue
+{
+    Value ** slot = nullptr;
+
+public:
+    UniqueRootValue() = default;
+
+    explicit UniqueRootValue(Value * v)
+        : slot(allocRootValueSlot(v))
+    {
+    }
+
+    UniqueRootValue(const UniqueRootValue &) = delete;
+    UniqueRootValue & operator=(const UniqueRootValue &) = delete;
+
+    UniqueRootValue(UniqueRootValue && other) noexcept
+        : slot(std::exchange(other.slot, nullptr))
+    {
+    }
+
+    UniqueRootValue & operator=(UniqueRootValue && other) noexcept
+    {
+        if (this == &other)
+            return *this;
+        if (slot)
+            freeRootValueSlot(slot);
+        slot = std::exchange(other.slot, nullptr);
+        return *this;
+    }
+
+    ~UniqueRootValue()
+    {
+        if (slot)
+            freeRootValueSlot(slot);
+    }
+
+    /**
+     * Release the slot, i.e. stop rooting the value.
+     */
+    void reset()
+    {
+        if (slot) {
+            freeRootValueSlot(slot);
+            slot = nullptr;
+        }
+    }
+
+    Value *& operator*() const
+    {
+        return *slot;
+    }
+
+    explicit operator bool() const
+    {
+        return slot != nullptr;
+    }
+};
+
+/**
+ * A copyable, shared handle rooting a Value. Only use this instead of
+ * `UniqueRootValue` if the handle must be copyable.
+ */
+typedef std::shared_ptr<Value *> RootValue;
+
+RootValue allocRootValue(Value * v);
+
+} // namespace nix

src/libexpr/include/nix/expr/value.hh

@@ -10,6 +10,7 @@
 #include <memory_resource>
 #include <exception>
 #include <span>
+#include <utility>
 #include <string_view>
 #include <type_traits>
 #include <concepts>
@@ -1541,12 +1542,5 @@ typedef boost::unordered_flat_map<
     traceable_allocator<std::pair<const Symbol, Value *>>>
     ValueMap;
 
-/**
- * A value allocated in traceable memory.
- */
-typedef std::shared_ptr<Value *> RootValue;
-
-RootValue allocRootValue(Value * v);
-
 void forceNoNullByte(std::string_view s, std::function<Pos()> = nullptr);
 } // namespace nix

src/libexpr/json-to-value.cc

@@ -18,7 +18,7 @@ class JSONSax : nlohmann::json_sax<json>
     {
     protected:
         std::unique_ptr<JSONState> parent;
-        RootValue v;
+        UniqueRootValue v;
     public:
         virtual std::unique_ptr<JSONState> resolve(EvalState &)
         {
@@ -31,7 +31,7 @@ class JSONSax : nlohmann::json_sax<json>
         }
 
         explicit JSONState(Value * v)
-            : v(allocRootValue(v))
+            : v(UniqueRootValue(v))
         {
         }
 
@@ -40,7 +40,7 @@ class JSONSax : nlohmann::json_sax<json>
         Value & value(EvalState & state)
         {
             if (!v)
-                v = allocRootValue(state.allocValue());
+                v = UniqueRootValue(state.allocValue());
             return **v;
         }
 
@@ -65,7 +65,7 @@ class JSONSax : nlohmann::json_sax<json>
 
         void add() override
         {
-            v = nullptr;
+            v.reset();
         }
     public:
         void key(string_t & name, EvalState & state)
@@ -91,7 +91,7 @@ class JSONSax : nlohmann::json_sax<json>
         void add() override
         {
             values.push_back(*v);
-            v = nullptr;
+            v.reset();
         }
     public:
         JSONListState(std::unique_ptr<JSONState> && p, std::size_t reserve)