From 19f89eb6842747570f262c003d977f02cb155968 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 21 Jul 2025 19:48:20 +0200 Subject: [PATCH] Avoid isValidPath(), use queryPathInfo() instead Since recently we call isValidPath() a lot from the evaluator, specifically from LocalStoreAccessor::requireStoreObject(). Unfortunately, isValidPath() uses but does not populate the in-memory path info cache; only queryPathInfo() does that. So isValidPath() is fast *if* we happened to call queryPathInfo() on the same path previously. This is not the case when lazy-trees is enabled, so we got a lot of superfluous, high-latency calls to the daemon (which show up in verbose output as `performing daemon worker op: 1`). Similarly, `fetchToStore()` called `isValidPath()` as well. The fix is to use `queryPathInfo()`, which for one particular eval reduced the number of daemon calls from 15246 to 2324. This may cause Nix to fetch some unnecessary information from the daemon, but that probably doesn't matter much given the high latency. --- src/libfetchers/fetch-to-store.cc | 2 +- src/libstore/include/nix/store/store-api.hh | 13 +++++++++++-- src/libstore/local-fs-store.cc | 2 +- src/libstore/store-api.cc | 19 +++++++++++++++++++ 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/libfetchers/fetch-to-store.cc b/src/libfetchers/fetch-to-store.cc index d3e416c7f..ae1cc04e6 100644 --- a/src/libfetchers/fetch-to-store.cc +++ b/src/libfetchers/fetch-to-store.cc @@ -52,7 +52,7 @@ std::pair fetchToStore2( auto hash = Hash::parseSRI(fetchers::getStrAttr(*res, "hash")); auto storePath = store.makeFixedOutputPathFromCA(name, ContentAddressWithReferences::fromParts(method, hash, {})); - if (mode == FetchMode::DryRun || store.isValidPath(storePath)) { + if (mode == FetchMode::DryRun || store.maybeQueryPathInfo(storePath)) { debug("source path '%s' cache hit in '%s' (hash '%s')", path, store.printStorePath(storePath), hash.to_string(HashFormat::SRI, true)); return {storePath, hash}; } diff --git a/src/libstore/include/nix/store/store-api.hh b/src/libstore/include/nix/store/store-api.hh index e0a3e67d1..09b0d15b4 100644 --- a/src/libstore/include/nix/store/store-api.hh +++ b/src/libstore/include/nix/store/store-api.hh @@ -269,7 +269,9 @@ public: StorePath followLinksToStorePath(std::string_view path) const; /** - * Check whether a path is valid. + * Check whether a path is valid. NOTE: this function does not + * generally cache whether a path is valid. You may want to use + * `maybeQueryPathInfo()`, which does cache. */ bool isValidPath(const StorePath & path); @@ -308,10 +310,17 @@ public: /** * Query information about a valid path. It is permitted to omit - * the name part of the store path. + * the name part of the store path. Throws an exception if the + * path is not valid. */ ref queryPathInfo(const StorePath & path); + /** + * Like `queryPathInfo()`, but returns `nullptr` if the path is + * not valid. + */ + std::shared_ptr maybeQueryPathInfo(const StorePath & path); + /** * Asynchronous version of queryPathInfo(). */ diff --git a/src/libstore/local-fs-store.cc b/src/libstore/local-fs-store.cc index add3b04d2..620819234 100644 --- a/src/libstore/local-fs-store.cc +++ b/src/libstore/local-fs-store.cc @@ -44,7 +44,7 @@ struct LocalStoreAccessor : PosixSourceAccessor void requireStoreObject(const CanonPath & path) { auto [storePath, rest] = store->toStorePath(store->storeDir + path.abs()); - if (requireValidPath && !store->isValidPath(storePath)) + if (requireValidPath && !store->maybeQueryPathInfo(storePath)) throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath)); } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 39de6808d..70f463059 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -633,6 +633,25 @@ ref Store::queryPathInfo(const StorePath & storePath) } +std::shared_ptr Store::maybeQueryPathInfo(const StorePath & storePath) +{ + std::promise> promise; + + queryPathInfo(storePath, + {[&](std::future> result) { + try { + promise.set_value(result.get()); + } catch (InvalidPath &) { + promise.set_value(nullptr); + } catch (...) { + promise.set_exception(std::current_exception()); + } + }}); + + return promise.get_future().get(); +} + + static bool goodStorePath(const StorePath & expected, const StorePath & actual) { return