Merge pull request #14828 from Zaczero/zaczero/libstore-registerValidPaths

libstore: reuse parsed derivations in registerValidPaths

src/libstore-tests/meson.build

@@ -124,6 +124,7 @@ if get_option('benchmarks')
     'bench-main.cc',
     'derivation-parser-bench.cc',
     'ref-scan-bench.cc',
+    'register-valid-paths-bench.cc',
   )
 
   benchmark_exe = executable(

src/libstore-tests/register-valid-paths-bench.cc

@@ -0,0 +1,79 @@
+#include <benchmark/benchmark.h>
+
+#include "nix/store/derivations.hh"
+#include "nix/store/local-store.hh"
+#include "nix/store/store-open.hh"
+#include "nix/util/file-system.hh"
+#include "nix/util/hash.hh"
+#include "nix/util/tests/test-data.hh"
+
+#ifndef _WIN32
+
+#  include <filesystem>
+#  include <fstream>
+
+using namespace nix;
+
+static void BM_RegisterValidPathsDerivations(benchmark::State & state)
+{
+    const int derivationCount = state.range(0);
+
+    for (auto _ : state) {
+        state.PauseTiming();
+
+        auto tmpRoot = createTempDir();
+        auto realStoreDir = tmpRoot / "nix/store";
+        std::filesystem::create_directories(realStoreDir);
+
+        std::shared_ptr<Store> store = openStore(fmt("local?root=%s", tmpRoot.string()));
+        auto localStore = std::dynamic_pointer_cast<LocalStore>(store);
+        if (!localStore)
+            throw Error("expected local store");
+
+        ValidPathInfos infos;
+        for (int i = 0; i < derivationCount; ++i) {
+            std::string drvName = fmt("register-valid-paths-bench-%d", i);
+            auto drvPath = StorePath::random(drvName + ".drv");
+
+            Derivation drv;
+            drv.name = drvName;
+            drv.outputs.emplace("out", DerivationOutput{DerivationOutput::Deferred{}});
+            drv.platform = "x86_64-linux";
+            drv.builder = "foo";
+            drv.env["out"] = "";
+            drv.fillInOutputPaths(*localStore);
+
+            auto drvContents = drv.unparse(*localStore, /*maskOutputs=*/false);
+
+            /* Create an on-disk store object without registering it
+               in the SQLite DB. LocalFSStore::getFSAccessor(path, false)
+               allows reading store objects based on their filesystem
+               presence alone. */
+            std::ofstream out(realStoreDir / std::string(drvPath.to_string()), std::ios::binary);
+            out.write(drvContents.data(), drvContents.size());
+            if (!out)
+                throw SysError("writing derivation to store");
+
+            ValidPathInfo info{drvPath, UnkeyedValidPathInfo(*localStore, Hash::dummy)};
+            info.narSize = drvContents.size();
+
+            infos.emplace(drvPath, std::move(info));
+        }
+
+        state.ResumeTiming();
+
+        localStore->registerValidPaths(infos);
+
+        state.PauseTiming();
+        localStore.reset();
+        store.reset();
+        std::filesystem::remove_all(tmpRoot);
+        state.ResumeTiming();
+    }
+
+    state.SetItemsProcessed(state.iterations() * derivationCount);
+}
+
+BENCHMARK(BM_RegisterValidPathsDerivations)->Arg(10);
+
+#endif

src/libstore/include/nix/store/local-store.hh

@@ -420,7 +420,7 @@ private:
 
     uint64_t queryValidPathId(State & state, const StorePath & path);
 
-    uint64_t addValidPath(State & state, const ValidPathInfo & info, bool checkOutputs = true);
+    uint64_t addValidPath(State & state, const ValidPathInfo & info);
 
     void invalidatePath(State & state, const StorePath & path);
 

src/libstore/local-store.cc

@@ -647,7 +647,7 @@ void LocalStore::cacheDrvOutputMapping(
         [&]() { state.stmts->AddDerivationOutput.use()(deriver)(outputName) (printStorePath(output)).exec(); });
 }
 
-uint64_t LocalStore::addValidPath(State & state, const ValidPathInfo & info, bool checkOutputs)
+uint64_t LocalStore::addValidPath(State & state, const ValidPathInfo & info)
 {
     if (info.ca.has_value() && !info.isContentAddressed(*this))
         throw Error(
@@ -668,17 +668,16 @@ uint64_t LocalStore::addValidPath(State & state, const ValidPathInfo & info, boo
        efficiently query whether a path is an output of some
        derivation. */
     if (info.path.isDerivation()) {
-        auto drv = readInvalidDerivation(info.path);
+        auto parsedDrv = readInvalidDerivation(info.path);
 
         /* Verify that the output paths in the derivation are correct
            (i.e., follow the scheme for computing output paths from
            derivations).  Note that if this throws an error, then the
            DB transaction is rolled back, so the path validity
            registration above is undone. */
-        if (checkOutputs)
+        parsedDrv.checkInvariants(*this, info.path);
-            drv.checkInvariants(*this, info.path);
 
-        for (auto & i : drv.outputsAndOptPaths(*this)) {
+        for (auto & i : parsedDrv.outputsAndOptPaths(*this)) {
             /* Floating CA derivations have indeterminate output paths until
                they are built, so don't register anything in that case */
             if (i.second.second)
@@ -929,7 +928,7 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos)
             if (isValidPath_(*state, i.path))
                 updatePathInfo(*state, i);
             else
-                addValidPath(*state, i, false);
+                addValidPath(*state, i);
             paths.insert(i.path);
         }
 
@@ -939,15 +938,6 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos)
                 state->stmts->AddReference.use()(referrer)(queryValidPathId(*state, j)).exec();
         }
 
-        /* Check that the derivation outputs are correct.  We can't do
-           this in addValidPath() above, because the references might
-           not be valid yet. */
-        for (auto & [_, i] : infos)
-            if (i.path.isDerivation()) {
-                // FIXME: inefficient; we already loaded the derivation in addValidPath().
-                readInvalidDerivation(i.path).checkInvariants(*this, i.path);
-            }
-
         /* Do a topological sort of the paths.  This will throw an
            error if a cycle is detected and roll back the
            transaction.  Cycles can only occur when a derivation