From 18ec3d1094e821df381dc1b12b13472086bfe021 Mon Sep 17 00:00:00 2001 From: Bernardo Meurer Costa Date: Mon, 13 Oct 2025 01:44:40 +0300 Subject: [PATCH] libstore: Avoid copying derivations to the store if they are already valid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/libstore-tests/meson.build | 1 + src/libstore-tests/write-derivation.cc | 57 ++++++++++++++++++++++++++ src/libstore/derivations.cc | 36 ++++++++-------- 3 files changed, 77 insertions(+), 17 deletions(-) create mode 100644 src/libstore-tests/write-derivation.cc diff --git a/src/libstore-tests/meson.build b/src/libstore-tests/meson.build index e8e90ad81..4d464ad89 100644 --- a/src/libstore-tests/meson.build +++ b/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('.') ] diff --git a/src/libstore-tests/write-derivation.cc b/src/libstore-tests/write-derivation.cc new file mode 100644 index 000000000..3f7de05d3 --- /dev/null +++ b/src/libstore-tests/write-derivation.cc @@ -0,0 +1,57 @@ +#include +#include + +#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 config_) + : LibStoreTest(config_->openDummyStore()) + , config(std::move(config_)) + { + config->readOnly = false; + } + + WriteDerivationTest() + : WriteDerivationTest(make_ref(DummyStoreConfig::Params{})) + { + } + + ref 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(testing::HasSubstrIgnoreANSIMatcher( + "operation 'addToStoreFromDump' is not supported by store 'dummy://'"))); +} + +} // namespace nix diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 6d7dbc99c..f634bccfb 100644 --- a/src/libstore/derivations.cc +++ b/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 {