Skip to content

Commit 19f89eb

Browse files
committed
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.
1 parent 4728153 commit 19f89eb

File tree

4 files changed

+32
-4
lines changed

4 files changed

+32
-4
lines changed

src/libfetchers/fetch-to-store.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ std::pair<StorePath, Hash> fetchToStore2(
5252
auto hash = Hash::parseSRI(fetchers::getStrAttr(*res, "hash"));
5353
auto storePath = store.makeFixedOutputPathFromCA(name,
5454
ContentAddressWithReferences::fromParts(method, hash, {}));
55-
if (mode == FetchMode::DryRun || store.isValidPath(storePath)) {
55+
if (mode == FetchMode::DryRun || store.maybeQueryPathInfo(storePath)) {
5656
debug("source path '%s' cache hit in '%s' (hash '%s')", path, store.printStorePath(storePath), hash.to_string(HashFormat::SRI, true));
5757
return {storePath, hash};
5858
}

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,9 @@ public:
269269
StorePath followLinksToStorePath(std::string_view path) const;
270270

271271
/**
272-
* Check whether a path is valid.
272+
* Check whether a path is valid. NOTE: this function does not
273+
* generally cache whether a path is valid. You may want to use
274+
* `maybeQueryPathInfo()`, which does cache.
273275
*/
274276
bool isValidPath(const StorePath & path);
275277

@@ -308,10 +310,17 @@ public:
308310

309311
/**
310312
* Query information about a valid path. It is permitted to omit
311-
* the name part of the store path.
313+
* the name part of the store path. Throws an exception if the
314+
* path is not valid.
312315
*/
313316
ref<const ValidPathInfo> queryPathInfo(const StorePath & path);
314317

318+
/**
319+
* Like `queryPathInfo()`, but returns `nullptr` if the path is
320+
* not valid.
321+
*/
322+
std::shared_ptr<const ValidPathInfo> maybeQueryPathInfo(const StorePath & path);
323+
315324
/**
316325
* Asynchronous version of queryPathInfo().
317326
*/

src/libstore/local-fs-store.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ struct LocalStoreAccessor : PosixSourceAccessor
4444
void requireStoreObject(const CanonPath & path)
4545
{
4646
auto [storePath, rest] = store->toStorePath(store->storeDir + path.abs());
47-
if (requireValidPath && !store->isValidPath(storePath))
47+
if (requireValidPath && !store->maybeQueryPathInfo(storePath))
4848
throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath));
4949
}
5050

src/libstore/store-api.cc

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,25 @@ ref<const ValidPathInfo> Store::queryPathInfo(const StorePath & storePath)
633633
}
634634

635635

636+
std::shared_ptr<const ValidPathInfo> Store::maybeQueryPathInfo(const StorePath & storePath)
637+
{
638+
std::promise<std::shared_ptr<const ValidPathInfo>> promise;
639+
640+
queryPathInfo(storePath,
641+
{[&](std::future<ref<const ValidPathInfo>> result) {
642+
try {
643+
promise.set_value(result.get());
644+
} catch (InvalidPath &) {
645+
promise.set_value(nullptr);
646+
} catch (...) {
647+
promise.set_exception(std::current_exception());
648+
}
649+
}});
650+
651+
return promise.get_future().get();
652+
}
653+
654+
636655
static bool goodStorePath(const StorePath & expected, const StorePath & actual)
637656
{
638657
return

0 commit comments

Comments
 (0)