From 268d1b1036775763fa52dbb112606889322813ea Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 25 Jul 2025 16:50:21 +0200 Subject: [PATCH] Add paths to the store asynchronously Adding paths to the store can be slow due to I/O overhead, but especially when going through the daemon because of the round-trip latency of every wopAddToStore call. So we now do the addToStore() calls asynchronously from a separate thread from the evaluator. This slightly speeds up the local store, and makes going through the daemon almost as fast as a local store. --- src/libcmd/installable-attr-path.cc | 4 +- src/libcmd/installable-flake.cc | 1 + src/libcmd/repl.cc | 1 + src/libexpr-c/nix_api_expr.cc | 5 + src/libexpr-c/nix_api_value.cc | 1 + src/libexpr/eval-cache.cc | 1 + src/libexpr/eval.cc | 23 +++ src/libexpr/include/nix/expr/eval.hh | 7 + src/libexpr/primops.cc | 6 +- src/libexpr/primops/context.cc | 1 + src/libstore/async-path-writer.cc | 173 ++++++++++++++++++ src/libstore/derivations.cc | 15 ++ .../include/nix/store/async-path-writer.hh | 19 ++ src/libstore/include/nix/store/derivations.hh | 11 ++ src/libstore/include/nix/store/meson.build | 1 + src/libstore/meson.build | 1 + src/nix-build/nix-build.cc | 6 +- src/nix-env/nix-env.cc | 2 + src/nix-env/user-env.cc | 5 +- src/nix/app.cc | 2 + 20 files changed, 281 insertions(+), 4 deletions(-) create mode 100644 src/libstore/async-path-writer.cc create mode 100644 src/libstore/include/nix/store/async-path-writer.hh diff --git a/src/libcmd/installable-attr-path.cc b/src/libcmd/installable-attr-path.cc index 28c3db3fc79..3a80aa384de 100644 --- a/src/libcmd/installable-attr-path.cc +++ b/src/libcmd/installable-attr-path.cc @@ -89,7 +89,8 @@ DerivedPathsWithInfo InstallableAttrPath::toDerivedPaths() } DerivedPathsWithInfo res; - for (auto & [drvPath, outputs] : byDrvPath) + for (auto & [drvPath, outputs] : byDrvPath) { + state->waitForPath(drvPath); res.push_back({ .path = DerivedPath::Built{ @@ -102,6 +103,7 @@ DerivedPathsWithInfo InstallableAttrPath::toDerivedPaths() so we can fill in this info. */ }), }); + } return res; } diff --git a/src/libcmd/installable-flake.cc b/src/libcmd/installable-flake.cc index 97f7eb645fa..77210ef8108 100644 --- a/src/libcmd/installable-flake.cc +++ b/src/libcmd/installable-flake.cc @@ -102,6 +102,7 @@ DerivedPathsWithInfo InstallableFlake::toDerivedPaths() } auto drvPath = attr->forceDerivation(); + state->waitForPath(drvPath); std::optional priority; diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index ea3f44a7cbc..b42b6d3f87a 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -333,6 +333,7 @@ StorePath NixRepl::getDerivationPath(Value & v) auto drvPath = packageInfo->queryDrvPath(); if (!drvPath) throw Error("expression did not evaluate to a valid derivation (no 'drvPath' attribute)"); + state->waitForPath(*drvPath); if (!state->store->isValidPath(*drvPath)) throw Error("expression evaluated to invalid derivation '%s'", state->store->printStorePath(*drvPath)); return *drvPath; diff --git a/src/libexpr-c/nix_api_expr.cc b/src/libexpr-c/nix_api_expr.cc index 02e901de9f2..454a7652bf5 100644 --- a/src/libexpr-c/nix_api_expr.cc +++ b/src/libexpr-c/nix_api_expr.cc @@ -69,6 +69,7 @@ nix_err nix_expr_eval_from_string( nix::Expr * parsedExpr = state->state.parseExprFromString(expr, state->state.rootPath(nix::CanonPath(path))); state->state.eval(parsedExpr, value->value); state->state.forceValue(value->value, nix::noPos); + state->state.waitForAllPaths(); } NIXC_CATCH_ERRS } @@ -80,6 +81,7 @@ nix_err nix_value_call(nix_c_context * context, EvalState * state, Value * fn, n try { state->state.callFunction(fn->value, arg->value, value->value, nix::noPos); state->state.forceValue(value->value, nix::noPos); + state->state.waitForAllPaths(); } NIXC_CATCH_ERRS } @@ -92,6 +94,7 @@ nix_err nix_value_call_multi( try { state->state.callFunction(fn->value, {(nix::Value **) args, nargs}, value->value, nix::noPos); state->state.forceValue(value->value, nix::noPos); + state->state.waitForAllPaths(); } NIXC_CATCH_ERRS } @@ -102,6 +105,7 @@ nix_err nix_value_force(nix_c_context * context, EvalState * state, nix_value * context->last_err_code = NIX_OK; try { state->state.forceValue(value->value, nix::noPos); + state->state.waitForAllPaths(); } NIXC_CATCH_ERRS } @@ -112,6 +116,7 @@ nix_err nix_value_force_deep(nix_c_context * context, EvalState * state, nix_val context->last_err_code = NIX_OK; try { state->state.forceValueDeep(value->value); + state->state.waitForAllPaths(); } NIXC_CATCH_ERRS } diff --git a/src/libexpr-c/nix_api_value.cc b/src/libexpr-c/nix_api_value.cc index fb90e2872e6..0fd69e63757 100644 --- a/src/libexpr-c/nix_api_value.cc +++ b/src/libexpr-c/nix_api_value.cc @@ -345,6 +345,7 @@ nix_value * nix_get_attr_byname(nix_c_context * context, const nix_value * value if (attr) { nix_gc_incref(nullptr, attr->value); state->state.forceValue(*attr->value, nix::noPos); + state->state.waitForAllPaths(); return as_nix_value_ptr(attr->value); } nix_set_err_msg(context, NIX_ERR_KEY, "missing attribute"); diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 2b3f3de2c94..0999943cc34 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -705,6 +705,7 @@ StorePath AttrCursor::forceDerivation() auto aDrvPath = getAttr(root->state.sDrvPath); auto drvPath = root->state.store->parseStorePath(aDrvPath->getString()); drvPath.requireDerivation(); + root->state.waitForPath(drvPath); if (!root->state.store->isValidPath(drvPath) && !settings.readOnlyMode) { /* The eval cache contains 'drvPath', but the actual path has been garbage-collected. So force it to be regenerated. */ diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 126f09e4cd3..69024e4a1da 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -22,6 +22,7 @@ #include "nix/fetchers/fetch-to-store.hh" #include "nix/fetchers/tarball.hh" #include "nix/fetchers/input-cache.hh" +#include "nix/store/async-path-writer.hh" #include "parser-tab.hh" @@ -324,6 +325,7 @@ EvalState::EvalState( , debugRepl(nullptr) , debugStop(false) , trylevel(0) + , asyncPathWriter(AsyncPathWriter::make(store)) , regexCache(makeRegexCache()) #if NIX_USE_BOEHMGC , valueAllocCache(std::allocate_shared(traceable_allocator(), nullptr)) @@ -1028,6 +1030,7 @@ std::string EvalState::mkSingleDerivedPathStringRaw(const SingleDerivedPath & p) auto optStaticOutputPath = std::visit( overloaded{ [&](const SingleDerivedPath::Opaque & o) { + waitForPath(o.path); auto drv = store->readDerivation(o.path); auto i = drv.outputs.find(b.output); if (i == drv.outputs.end()) @@ -3282,4 +3285,24 @@ void forceNoNullByte(std::string_view s, std::function pos) } } +void EvalState::waitForPath(const StorePath & path) +{ + asyncPathWriter->waitForPath(path); +} + +void EvalState::waitForPath(const SingleDerivedPath & path) +{ + std::visit( + overloaded{ + [&](const DerivedPathOpaque & p) { waitForPath(p.path); }, + [&](const SingleDerivedPathBuilt & p) { waitForPath(*p.drvPath); }, + }, + path.raw()); +} + +void EvalState::waitForAllPaths() +{ + asyncPathWriter->waitForAllPaths(); +} + } // namespace nix diff --git a/src/libexpr/include/nix/expr/eval.hh b/src/libexpr/include/nix/expr/eval.hh index ac700e7485a..5ef629b3fb9 100644 --- a/src/libexpr/include/nix/expr/eval.hh +++ b/src/libexpr/include/nix/expr/eval.hh @@ -47,6 +47,7 @@ struct SingleDerivedPath; enum RepairFlag : bool; struct MemorySourceAccessor; struct MountedSourceAccessor; +struct AsyncPathWriter; namespace eval_cache { class EvalCache; @@ -322,6 +323,8 @@ public: std::list debugTraces; std::map> exprEnvs; + ref asyncPathWriter; + const std::shared_ptr getStaticEnv(const Expr & expr) const { auto i = exprEnvs.find(&expr); @@ -936,6 +939,10 @@ public: DocComment getDocCommentForPos(PosIdx pos); + void waitForPath(const StorePath & path); + void waitForPath(const SingleDerivedPath & path); + void waitForAllPaths(); + private: /** diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 3ce681e0093..8b74ac937b4 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -64,6 +64,7 @@ StringMap EvalState::realiseContext(const NixStringContext & context, StorePathS for (auto & c : context) { auto ensureValid = [&](const StorePath & p) { + waitForPath(p); if (!store->isValidPath(p)) error(store->printStorePath(p)).debugThrow(); }; @@ -298,6 +299,7 @@ static void import(EvalState & state, const PosIdx pos, Value & vPath, Value * v if (!state.store->isStorePath(path2)) return std::nullopt; auto storePath = state.store->parseStorePath(path2); + state.waitForPath(storePath); if (!(state.store->isValidPath(storePath) && isDerivation(path2))) return std::nullopt; return storePath; @@ -1589,6 +1591,8 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName [&](const NixStringContextElem::DrvDeep & d) { /* !!! This doesn't work if readOnlyMode is set. */ StorePathSet refs; + // FIXME: don't need to wait, we only need the references. + state.waitForPath(d.drvPath); state.store->computeFSClosure(d.drvPath, refs); for (auto & j : refs) { drv.inputSrcs.insert(j); @@ -1729,7 +1733,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName } /* Write the resulting term into the Nix store directory. */ - auto drvPath = writeDerivation(*state.store, drv, state.repair); + auto drvPath = writeDerivation(*state.store, *state.asyncPathWriter, drv, state.repair); auto drvPathS = state.store->printStorePath(drvPath); printMsg(lvlChatty, "instantiated '%1%' -> '%2%'", drvName, drvPathS); diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index 28fa06dcd46..cf471f7dbff 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -75,6 +75,7 @@ static void prim_unsafeDiscardOutputDependency(EvalState & state, const PosIdx p NixStringContext context2; for (auto && c : context) { if (auto * ptr = std::get_if(&c.raw)) { + state.waitForPath(ptr->drvPath); // FIXME: why? context2.emplace(NixStringContextElem::Opaque{.path = ptr->drvPath}); } else { /* Can reuse original item */ diff --git a/src/libstore/async-path-writer.cc b/src/libstore/async-path-writer.cc new file mode 100644 index 00000000000..887b466e87b --- /dev/null +++ b/src/libstore/async-path-writer.cc @@ -0,0 +1,173 @@ +#include "nix/store/async-path-writer.hh" +#include "nix/util/archive.hh" + +#include +#include + +namespace nix { + +struct AsyncPathWriterImpl : AsyncPathWriter +{ + ref store; + + struct Item + { + StorePath storePath; + std::string contents; + std::string name; + Hash hash; + StorePathSet references; + RepairFlag repair; + std::promise promise; + }; + + struct State + { + std::vector items; + std::unordered_map> futures; + bool quit = false; + }; + + Sync state_; + + std::thread workerThread; + + std::condition_variable wakeupCV; + + AsyncPathWriterImpl(ref store) + : store(store) + { + workerThread = std::thread([&]() { + while (true) { + std::vector items; + + { + auto state(state_.lock()); + while (!state->quit && state->items.empty()) + state.wait(wakeupCV); + if (state->items.empty() && state->quit) + return; + std::swap(items, state->items); + } + + try { + writePaths(items); + for (auto & item : items) + item.promise.set_value(); + } catch (...) { + for (auto & item : items) + item.promise.set_exception(std::current_exception()); + } + } + }); + } + + ~AsyncPathWriterImpl() + { + state_.lock()->quit = true; + wakeupCV.notify_all(); + workerThread.join(); + } + + StorePath + addPath(std::string contents, std::string name, StorePathSet references, RepairFlag repair, bool readOnly) override + { + auto hash = hashString(HashAlgorithm::SHA256, contents); + + auto storePath = store->makeFixedOutputPathFromCA( + name, + TextInfo{ + .hash = hash, + .references = references, + }); + + if (!readOnly) { + auto state(state_.lock()); + std::promise promise; + state->futures.insert_or_assign(storePath, promise.get_future()); + state->items.push_back( + Item{ + .storePath = storePath, + .contents = std::move(contents), + .name = std::move(name), + .hash = hash, + .references = std::move(references), + .repair = repair, + .promise = std::move(promise), + }); + wakeupCV.notify_all(); + } + + return storePath; + } + + void waitForPath(const StorePath & path) override + { + auto future = ({ + auto state = state_.lock(); + auto i = state->futures.find(path); + if (i == state->futures.end()) + return; + i->second; + }); + future.get(); + } + + void waitForAllPaths() override + { + auto futures = ({ + auto state(state_.lock()); + std::move(state->futures); + }); + for (auto & future : futures) + future.second.get(); + } + + void writePaths(const std::vector & items) + { +// FIXME: addMultipeToStore() shouldn't require a NAR hash. +#if 0 + Store::PathsSource sources; + RepairFlag repair = NoRepair; + + for (auto & item : items) { + ValidPathInfo info{item.storePath, Hash(HashAlgorithm::SHA256)}; + info.references = item.references; + info.ca = ContentAddress { + .method = ContentAddressMethod::Raw::Text, + .hash = item.hash, + }; + if (item.repair) repair = item.repair; + auto source = sinkToSource([&](Sink & sink) + { + dumpString(item.contents, sink); + }); + sources.push_back({std::move(info), std::move(source)}); + } + + Activity act(*logger, lvlDebug, actUnknown, fmt("adding %d paths to the store", items.size())); + + store->addMultipleToStore(std::move(sources), act, repair); +#endif + + for (auto & item : items) { + StringSource source(item.contents); + auto storePath = store->addToStoreFromDump( + source, + item.storePath.name(), + FileSerialisationMethod::Flat, + ContentAddressMethod::Raw::Text, + HashAlgorithm::SHA256, + item.references, + item.repair); + assert(storePath == item.storePath); + } + } +}; + +ref AsyncPathWriter::make(ref store) +{ + return make_ref(store); +} + +} // namespace nix diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 279713c71f0..8c5b0cb89ca 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -9,6 +9,7 @@ #include "nix/store/common-protocol-impl.hh" #include "nix/util/strings-inline.hh" #include "nix/util/json-utils.hh" +#include "nix/store/async-path-writer.hh" #include #include @@ -131,6 +132,20 @@ StorePath writeDerivation(Store & store, const Derivation & drv, RepairFlag repa }); } +StorePath writeDerivation( + Store & store, AsyncPathWriter & asyncPathWriter, const Derivation & drv, RepairFlag repair, bool readOnly) +{ + auto references = drv.inputSrcs; + for (auto & i : drv.inputDrvs.map) + references.insert(i.first); + return asyncPathWriter.addPath( + drv.unparse(store, false), + std::string(drv.name) + drvExtension, + references, + repair, + readOnly || settings.readOnlyMode); +} + namespace { /** * This mimics std::istream to some extent. We use this much smaller implementation diff --git a/src/libstore/include/nix/store/async-path-writer.hh b/src/libstore/include/nix/store/async-path-writer.hh new file mode 100644 index 00000000000..80997dc6ac2 --- /dev/null +++ b/src/libstore/include/nix/store/async-path-writer.hh @@ -0,0 +1,19 @@ +#pragma once + +#include "nix/store/store-api.hh" + +namespace nix { + +struct AsyncPathWriter +{ + virtual StorePath addPath( + std::string contents, std::string name, StorePathSet references, RepairFlag repair, bool readOnly = false) = 0; + + virtual void waitForPath(const StorePath & path) = 0; + + virtual void waitForAllPaths() = 0; + + static ref make(ref store); +}; + +} // namespace nix diff --git a/src/libstore/include/nix/store/derivations.hh b/src/libstore/include/nix/store/derivations.hh index 41cd179f425..9f1f025a5c7 100644 --- a/src/libstore/include/nix/store/derivations.hh +++ b/src/libstore/include/nix/store/derivations.hh @@ -16,6 +16,7 @@ namespace nix { struct StoreDirConfig; +struct AsyncPathWriter; /* Abstract syntax of derivations. */ @@ -406,6 +407,16 @@ class Store; */ StorePath writeDerivation(Store & store, const Derivation & drv, RepairFlag repair = NoRepair, bool readOnly = false); +/** + * Asynchronously write a derivation to the Nix store, and return its path. + */ +StorePath writeDerivation( + Store & store, + AsyncPathWriter & asyncPathWriter, + const Derivation & drv, + RepairFlag repair = NoRepair, + bool readOnly = false); + /** * Read a derivation from a file. */ diff --git a/src/libstore/include/nix/store/meson.build b/src/libstore/include/nix/store/meson.build index a1843041760..e8e639f2a9e 100644 --- a/src/libstore/include/nix/store/meson.build +++ b/src/libstore/include/nix/store/meson.build @@ -10,6 +10,7 @@ config_pub_h = configure_file( ) headers = [config_pub_h] + files( + 'async-path-writer.hh', 'binary-cache-store.hh', 'build-result.hh', 'build/derivation-goal.hh', diff --git a/src/libstore/meson.build b/src/libstore/meson.build index 2aff1729077..fc889c24d24 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -253,6 +253,7 @@ config_priv_h = configure_file( subdir('nix-meson-build-support/common') sources = files( + 'async-path-writer.cc', 'binary-cache-store.cc', 'build-result.cc', 'build/derivation-goal.cc', diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 9fd9b935c96..f876cf9ddbf 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -451,7 +451,9 @@ static void main_nix_build(int argc, char ** argv) throw UsageError("nix-shell requires a single derivation"); auto & packageInfo = drvs.front(); - auto drv = evalStore->derivationFromPath(packageInfo.requireDrvPath()); + auto drvPath = packageInfo.requireDrvPath(); + state->waitForPath(drvPath); + auto drv = evalStore->derivationFromPath(drvPath); std::vector pathsToBuild; RealisedPath::Set pathsToCopy; @@ -475,6 +477,7 @@ static void main_nix_build(int argc, char ** argv) throw Error("the 'bashInteractive' attribute in did not evaluate to a derivation"); auto bashDrv = drv->requireDrvPath(); + state->waitForPath(bashDrv); pathsToBuild.push_back( DerivedPath::Built{ .drvPath = makeConstantStorePathRef(bashDrv), @@ -684,6 +687,7 @@ static void main_nix_build(int argc, char ** argv) for (auto & packageInfo : drvs) { auto drvPath = packageInfo.requireDrvPath(); + state->waitForPath(drvPath); auto outputName = packageInfo.queryOutputName(); if (outputName == "") diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index f165c069cd8..1022f620b6c 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -746,6 +746,8 @@ static void opSet(Globals & globals, Strings opFlags, Strings opArgs) drv.setName(globals.forceName); auto drvPath = drv.queryDrvPath(); + if (drvPath) + globals.state->waitForPath(*drvPath); std::vector paths{ drvPath ? (DerivedPath) (DerivedPath::Built{ .drvPath = makeConstantStorePathRef(*drvPath), diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc index dab6871ed89..43d42d0feb0 100644 --- a/src/nix-env/user-env.cc +++ b/src/nix-env/user-env.cc @@ -37,8 +37,10 @@ bool createUserEnv( exist already. */ std::vector drvsToBuild; for (auto & i : elems) - if (auto drvPath = i.queryDrvPath()) + if (auto drvPath = i.queryDrvPath()) { + state.waitForPath(*drvPath); drvsToBuild.push_back({*drvPath}); + } debug("building user environment dependencies"); state.store->buildPaths(toDerivedPaths(drvsToBuild), state.repair ? bmRepair : bmNormal); @@ -151,6 +153,7 @@ bool createUserEnv( debug("building user environment"); std::vector topLevelDrvs; topLevelDrvs.push_back({topLevelDrv}); + state.waitForPath(topLevelDrv); state.store->buildPaths(toDerivedPaths(topLevelDrvs), state.repair ? bmRepair : bmNormal); /* Switch the current user environment to the output path. */ diff --git a/src/nix/app.cc b/src/nix/app.cc index a043d1b00cc..8b9b20e4ba7 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -74,6 +74,7 @@ UnresolvedApp InstallableValue::toApp(EvalState & state) std::visit( overloaded{ [&](const NixStringContextElem::DrvDeep & d) -> DerivedPath { + state.waitForPath(d.drvPath); /* We want all outputs of the drv */ return DerivedPath::Built{ .drvPath = makeConstantStorePathRef(d.drvPath), @@ -81,6 +82,7 @@ UnresolvedApp InstallableValue::toApp(EvalState & state) }; }, [&](const NixStringContextElem::Built & b) -> DerivedPath { + state.waitForPath(*b.drvPath); return DerivedPath::Built{ .drvPath = b.drvPath, .outputs = OutputsSpec::Names{b.output},