libstore: Avoid copying derivations to the store if they are already valid

This avoids the quite costly copying of derivations to the daemon over the
wire in case it already exists in the eval store.

For a fresh instantiatation (after running nix-collect-garbage) this doesn't
significantly slow down eval:

taskset -c 2,3 hyperfine --reference "result-old/bin/nix eval -f ../nixpkgs hello --store unix:///tmp/nix_socket" --prepare "nix-collect-garbage --store /tmp/store1111 --no-keep-derivations" "result/bin/nix eval -f ../nixpkgs hello --store unix:///tmp/nix_socket"
Benchmark 1: result-old/bin/nix eval -f ../nixpkgs hello --store unix:///tmp/nix_socket
  Time (mean ± σ):     388.7 ms ±  10.5 ms    [User: 157.0 ms, System: 61.3 ms]
  Range (min … max):   379.4 ms … 415.9 ms    10 runs

Benchmark 2: result/bin/nix eval -f ../nixpkgs hello --store unix:///tmp/nix_socket
  Time (mean ± σ):     389.2 ms ±   4.8 ms    [User: 158.5 ms, System: 60.7 ms]
  Range (min … max):   381.2 ms … 397.6 ms    10 runs

But if the derivations are already instantiated this shows a pretty neat speedup:

taskset -c 2,3 hyperfine --reference "result-old/bin/nix eval -f ../nixpkgs hello --store unix:///tmp/nix_socket" "result/bin/nix eval -f ../nixpkgs hello --store unix:///tmp/nix_socket"
Benchmark 1: result-old/bin/nix eval -f ../nixpkgs hello --store unix:///tmp/nix_socket
  Time (mean ± σ):     240.4 ms ±   3.1 ms    [User: 148.1 ms, System: 57.0 ms]
  Range (min … max):   233.8 ms … 245.0 ms    12 runs

Benchmark 2: result/bin/nix eval -f ../nixpkgs hello --store unix:///tmp/nix_socket
  Time (mean ± σ):     226.5 ms ±   4.5 ms    [User: 147.8 ms, System: 55.2 ms]
  Range (min … max):   214.0 ms … 231.2 ms    13 runs

Co-authored-by: Sergei Zimmerman <[email protected]>

Author: lovesegfault

src/libstore-tests/meson.build

@@ -83,6 +83,7 @@ sources = files(
   'store-reference.cc',
   'uds-remote-store.cc',
   'worker-protocol.cc',
+  'write-derivation.cc',
 )
 
 include_dirs = [ include_directories('.') ]

src/libstore-tests/write-derivation.cc

@@ -0,0 +1,57 @@
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include "nix/util/tests/gmock-matchers.hh"
+#include "nix/store/derivations.hh"
+#include "nix/store/dummy-store-impl.hh"
+#include "nix/store/tests/libstore.hh"
+
+namespace nix {
+namespace {
+
+class WriteDerivationTest : public LibStoreTest
+{
+protected:
+    WriteDerivationTest(ref<DummyStoreConfig> config_)
+        : LibStoreTest(config_->openDummyStore())
+        , config(std::move(config_))
+    {
+        config->readOnly = false;
+    }
+
+    WriteDerivationTest()
+        : WriteDerivationTest(make_ref<DummyStoreConfig>(DummyStoreConfig::Params{}))
+    {
+    }
+
+    ref<DummyStoreConfig> config;
+};
+
+static Derivation makeSimpleDrv()
+{
+    Derivation drv;
+    drv.name = "simple-derivation";
+    drv.platform = "system";
+    drv.builder = "foo";
+    drv.args = {"bar", "baz"};
+    drv.env = StringPairs{{"BIG_BAD", "WOLF"}};
+    return drv;
+}
+
+} // namespace
+
+TEST_F(WriteDerivationTest, addToStoreFromDumpCalledOnce)
+{
+    auto drv = makeSimpleDrv();
+
+    auto path1 = writeDerivation(*store, drv, NoRepair);
+    config->readOnly = true;
+    auto path2 = writeDerivation(*store, drv, NoRepair);
+    EXPECT_EQ(path1, path2);
+    EXPECT_THAT(
+        [&] { writeDerivation(*store, drv, Repair); },
+        ::testing::ThrowsMessage<Error>(testing::HasSubstrIgnoreANSIMatcher(
+            "operation 'addToStoreFromDump' is not supported by store 'dummy://'")));
+}
+
+} // namespace nix

src/libstore/derivations.cc

@@ -115,23 +115,25 @@ StorePath writeDerivation(Store & store, const Derivation & drv, RepairFlag repa
        held during a garbage collection). */
     auto suffix = std::string(drv.name) + drvExtension;
     auto contents = drv.unparse(store, false);
-    return readOnly || settings.readOnlyMode ? store.makeFixedOutputPathFromCA(
-                                                   suffix,
-                                                   TextInfo{
-                                                       .hash = hashString(HashAlgorithm::SHA256, contents),
-                                                       .references = std::move(references),
-                                                   })
-                                             : ({
-                                                   StringSource s{contents};
-                                                   store.addToStoreFromDump(
-                                                       s,
-                                                       suffix,
-                                                       FileSerialisationMethod::Flat,
-                                                       ContentAddressMethod::Raw::Text,
-                                                       HashAlgorithm::SHA256,
-                                                       references,
-                                                       repair);
-                                               });
+    auto hash = hashString(HashAlgorithm::SHA256, contents);
+    auto ca = TextInfo{.hash = hash, .references = references};
+    auto path = store.makeFixedOutputPathFromCA(suffix, ca);
+
+    if (readOnly || settings.readOnlyMode || (store.isValidPath(path) && !repair))
+        return path;
+
+    StringSource s{contents};
+    auto path2 = store.addToStoreFromDump(
+        s,
+        suffix,
+        FileSerialisationMethod::Flat,
+        ContentAddressMethod::Raw::Text,
+        HashAlgorithm::SHA256,
+        references,
+        repair);
+    assert(path2 == path);
+
+    return path;
 }
 
 namespace {