From e0204f8d462041387651af388074491fd0bf36d6 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 19 Apr 2016 18:50:15 +0200 Subject: [PATCH] Move path info caching from BinaryCacheStore to Store Caching path info is generally useful. For instance, it speeds up "nix path-info -rS /run/current-system" (i.e. showing the closure sizes of all paths in the closure of the current system) from 5.6s to 0.15s. This also eliminates some APIs like Store::queryDeriver() and Store::queryReferences(). --- perl/lib/Nix/Store.xs | 21 +++-- src/libstore/binary-cache-store.cc | 118 ++++++++++------------------- src/libstore/binary-cache-store.hh | 44 +---------- src/libstore/build.cc | 10 +-- src/libstore/gc.cc | 4 +- src/libstore/local-store.cc | 99 ++++++++++++------------ src/libstore/local-store.hh | 10 +-- src/libstore/misc.cc | 17 +++-- src/libstore/remote-store.cc | 62 +++++---------- src/libstore/remote-store.hh | 10 +-- src/libstore/store-api.cc | 62 ++++++++++++--- src/libstore/store-api.hh | 62 ++++++++++++--- src/libstore/worker-protocol.hh | 6 +- src/libutil/lru-cache.hh | 6 ++ src/libutil/ref.hh | 6 ++ src/nix-daemon/nix-daemon.cc | 16 ++-- src/nix-store/dotgraph.cc | 14 +--- src/nix-store/nix-store.cc | 43 ++++++----- src/nix-store/xmlgraph.cc | 13 +--- src/nix/sigs.cc | 32 ++++---- src/nix/verify.cc | 16 ++-- 21 files changed, 318 insertions(+), 353 deletions(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index bb322875..6723ca38 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -70,8 +70,7 @@ int isValidPath(char * path) SV * queryReferences(char * path) PPCODE: try { - PathSet paths; - store()->queryReferences(path, paths); + PathSet paths = store()->queryPathInfo(path)->references; for (PathSet::iterator i = paths.begin(); i != paths.end(); ++i) XPUSHs(sv_2mortal(newSVpv(i->c_str(), 0))); } catch (Error & e) { @@ -82,7 +81,7 @@ SV * queryReferences(char * path) SV * queryPathHash(char * path) PPCODE: try { - Hash hash = store()->queryPathHash(path); + auto hash = store()->queryPathInfo(path)->narHash; string s = "sha256:" + printHash32(hash); XPUSHs(sv_2mortal(newSVpv(s.c_str(), 0))); } catch (Error & e) { @@ -93,7 +92,7 @@ SV * queryPathHash(char * path) SV * queryDeriver(char * path) PPCODE: try { - Path deriver = store()->queryDeriver(path); + auto deriver = store()->queryPathInfo(path)->deriver; if (deriver == "") XSRETURN_UNDEF; XPUSHs(sv_2mortal(newSVpv(deriver.c_str(), 0))); } catch (Error & e) { @@ -104,17 +103,17 @@ SV * queryDeriver(char * path) SV * queryPathInfo(char * path, int base32) PPCODE: try { - ValidPathInfo info = store()->queryPathInfo(path); - if (info.deriver == "") + auto info = store()->queryPathInfo(path); + if (info->deriver == "") XPUSHs(&PL_sv_undef); else - XPUSHs(sv_2mortal(newSVpv(info.deriver.c_str(), 0))); - string s = "sha256:" + (base32 ? printHash32(info.narHash) : printHash(info.narHash)); + XPUSHs(sv_2mortal(newSVpv(info->deriver.c_str(), 0))); + string s = "sha256:" + (base32 ? printHash32(info->narHash) : printHash(info->narHash)); XPUSHs(sv_2mortal(newSVpv(s.c_str(), 0))); - mXPUSHi(info.registrationTime); - mXPUSHi(info.narSize); + mXPUSHi(info->registrationTime); + mXPUSHi(info->narSize); AV * arr = newAV(); - for (PathSet::iterator i = info.references.begin(); i != info.references.end(); ++i) + for (PathSet::iterator i = info->references.begin(); i != info->references.end(); ++i) av_push(arr, newSVpv(i->c_str(), 0)); XPUSHs(sv_2mortal(newRV((SV *) arr))); } catch (Error & e) { diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 7016eedf..81800d4c 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -40,11 +40,6 @@ void BinaryCacheStore::notImpl() throw Error("operation not implemented for binary cache stores"); } -const BinaryCacheStore::Stats & BinaryCacheStore::getStats() -{ - return stats; -} - Path BinaryCacheStore::narInfoFileFor(const Path & storePath) { assertStorePath(storePath); @@ -100,67 +95,15 @@ void BinaryCacheStore::addToCache(const ValidPathInfo & info, { auto state_(state.lock()); - state_->narInfoCache.upsert(narInfo->path, narInfo); - stats.narInfoCacheSize = state_->narInfoCache.size(); + state_->pathInfoCache.upsert(narInfo->path, std::shared_ptr(narInfo)); + stats.pathInfoCacheSize = state_->pathInfoCache.size(); } stats.narInfoWrite++; } -NarInfo BinaryCacheStore::readNarInfo(const Path & storePath) +bool BinaryCacheStore::isValidPathUncached(const Path & storePath) { - { - auto state_(state.lock()); - auto res = state_->narInfoCache.get(storePath); - if (res) { - stats.narInfoReadAverted++; - if (!*res) - throw InvalidPath(format("path ‘%s’ is not valid") % storePath); - return **res; - } - } - - auto narInfoFile = narInfoFileFor(storePath); - auto data = getFile(narInfoFile); - if (!data) { - stats.narInfoMissing++; - auto state_(state.lock()); - state_->narInfoCache.upsert(storePath, 0); - stats.narInfoCacheSize = state_->narInfoCache.size(); - throw InvalidPath(format("path ‘%s’ is not valid") % storePath); - } - - auto narInfo = make_ref(*data, narInfoFile); - if (narInfo->path != storePath) - throw Error(format("NAR info file for store path ‘%1%’ does not match ‘%2%’") % narInfo->path % storePath); - - stats.narInfoRead++; - - if (publicKeys) { - if (!narInfo->checkSignatures(*publicKeys)) - throw Error(format("no good signature on NAR info file ‘%1%’") % narInfoFile); - } - - { - auto state_(state.lock()); - state_->narInfoCache.upsert(storePath, narInfo); - stats.narInfoCacheSize = state_->narInfoCache.size(); - } - - return *narInfo; -} - -bool BinaryCacheStore::isValidPath(const Path & storePath) -{ - { - auto state_(state.lock()); - auto res = state_->narInfoCache.get(storePath); - if (res) { - stats.narInfoReadAverted++; - return *res != 0; - } - } - // FIXME: this only checks whether a .narinfo with a matching hash // part exists. So ‘f4kb...-foo’ matches ‘f4kb...-bar’, even // though they shouldn't. Not easily fixed. @@ -169,20 +112,20 @@ bool BinaryCacheStore::isValidPath(const Path & storePath) void BinaryCacheStore::narFromPath(const Path & storePath, Sink & sink) { - auto res = readNarInfo(storePath); + auto info = queryPathInfo(storePath).cast(); - auto nar = getFile(res.url); + auto nar = getFile(info->url); - if (!nar) throw Error(format("file ‘%s’ missing from binary cache") % res.url); + if (!nar) throw Error(format("file ‘%s’ missing from binary cache") % info->url); stats.narRead++; stats.narReadCompressedBytes += nar->size(); /* Decompress the NAR. FIXME: would be nice to have the remote side do this. */ - if (res.compression == "none") + if (info->compression == "none") ; - else if (res.compression == "xz") + else if (info->compression == "xz") nar = decompressXZ(*nar); else throw Error(format("unknown NAR compression type ‘%1%’") % nar); @@ -200,13 +143,13 @@ void BinaryCacheStore::exportPath(const Path & storePath, bool sign, Sink & sink { assert(!sign); - auto res = readNarInfo(storePath); + auto res = queryPathInfo(storePath); narFromPath(storePath, sink); // FIXME: check integrity of NAR. - sink << exportMagic << storePath << res.references << res.deriver << 0; + sink << exportMagic << storePath << res->references << res->deriver << 0; } Paths BinaryCacheStore::importPaths(bool requireSignature, Source & source, @@ -244,9 +187,24 @@ struct NopSink : ParseSink { }; -ValidPathInfo BinaryCacheStore::queryPathInfo(const Path & storePath) +std::shared_ptr BinaryCacheStore::queryPathInfoUncached(const Path & storePath) { - return ValidPathInfo(readNarInfo(storePath)); + auto narInfoFile = narInfoFileFor(storePath); + auto data = getFile(narInfoFile); + if (!data) return 0; + + auto narInfo = make_ref(*data, narInfoFile); + if (narInfo->path != storePath) + throw Error(format("NAR info file for store path ‘%1%’ does not match ‘%2%’") % narInfo->path % storePath); + + stats.narInfoRead++; + + if (publicKeys) { + if (!narInfo->checkSignatures(*publicKeys)) + throw Error(format("no good signature on NAR info file ‘%1%’") % narInfoFile); + } + + return std::shared_ptr(narInfo); } void BinaryCacheStore::querySubstitutablePathInfos(const PathSet & paths, @@ -257,16 +215,16 @@ void BinaryCacheStore::querySubstitutablePathInfos(const PathSet & paths, if (!localStore) return; for (auto & storePath : paths) { - if (!localStore->isValidPath(storePath)) { + try { + auto info = localStore->queryPathInfo(storePath); + SubstitutablePathInfo sub; + sub.references = info->references; + sub.downloadSize = 0; + sub.narSize = info->narSize; + infos.emplace(storePath, sub); + } catch (InvalidPath &) { left.insert(storePath); - continue; } - ValidPathInfo info = localStore->queryPathInfo(storePath); - SubstitutablePathInfo sub; - sub.references = info.references; - sub.downloadSize = 0; - sub.narSize = info.narSize; - infos.emplace(storePath, sub); } if (settings.useSubstitutes) @@ -332,16 +290,16 @@ void BinaryCacheStore::buildPaths(const PathSet & paths, BuildMode buildMode) if (!localStore->isValidPath(storePath)) localStore->ensurePath(storePath); - ValidPathInfo info = localStore->queryPathInfo(storePath); + auto info = localStore->queryPathInfo(storePath); - for (auto & ref : info.references) + for (auto & ref : info->references) if (ref != storePath) ensurePath(ref); StringSink sink; dumpPath(storePath, sink); - addToCache(info, *sink.s); + addToCache(*info, *sink.s); } } diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 95e5d68b..4e4346a4 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -3,8 +3,6 @@ #include "crypto.hh" #include "store-api.hh" -#include "lru-cache.hh" -#include "sync.hh" #include "pool.hh" #include @@ -22,13 +20,6 @@ private: std::shared_ptr localStore; - struct State - { - LRUCache> narInfoCache{64 * 1024}; - }; - - Sync state; - protected: BinaryCacheStore(std::shared_ptr localStore, const Path & secretKeyFile); @@ -47,42 +38,17 @@ public: virtual void init(); - struct Stats - { - std::atomic narInfoRead{0}; - std::atomic narInfoReadAverted{0}; - std::atomic narInfoMissing{0}; - std::atomic narInfoWrite{0}; - std::atomic narInfoCacheSize{0}; - std::atomic narRead{0}; - std::atomic narReadBytes{0}; - std::atomic narReadCompressedBytes{0}; - std::atomic narWrite{0}; - std::atomic narWriteAverted{0}; - std::atomic narWriteBytes{0}; - std::atomic narWriteCompressedBytes{0}; - std::atomic narWriteCompressionTimeMs{0}; - }; - - const Stats & getStats(); - private: - Stats stats; - std::string narMagic; std::string narInfoFileFor(const Path & storePath); void addToCache(const ValidPathInfo & info, const string & nar); -protected: - - NarInfo readNarInfo(const Path & storePath); - public: - bool isValidPath(const Path & path) override; + bool isValidPathUncached(const Path & path) override; PathSet queryValidPaths(const PathSet & paths) override { notImpl(); } @@ -90,18 +56,12 @@ public: PathSet queryAllValidPaths() override { notImpl(); } - ValidPathInfo queryPathInfo(const Path & path) override; - - Hash queryPathHash(const Path & path) override - { notImpl(); } + std::shared_ptr queryPathInfoUncached(const Path & path) override; void queryReferrers(const Path & path, PathSet & referrers) override { notImpl(); } - Path queryDeriver(const Path & path) override - { return ""; } - PathSet queryValidDerivers(const Path & path) override { return {}; } diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 4b1c177f..ae807806 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2724,7 +2724,7 @@ void DerivationGoal::registerOutputs() if (buildMode == bmCheck) { if (!worker.store.isValidPath(path)) continue; - ValidPathInfo info = worker.store.queryPathInfo(path); + auto info = *worker.store.queryPathInfo(path); if (hash.first != info.narHash) { if (settings.keepFailed) { Path dst = path + checkSuffix; @@ -3778,14 +3778,14 @@ bool Worker::pathContentsGood(const Path & path) std::map::iterator i = pathContentsGoodCache.find(path); if (i != pathContentsGoodCache.end()) return i->second; printMsg(lvlInfo, format("checking path ‘%1%’...") % path); - ValidPathInfo info = store.queryPathInfo(path); + auto info = store.queryPathInfo(path); bool res; if (!pathExists(path)) res = false; else { - HashResult current = hashPath(info.narHash.type, path); + HashResult current = hashPath(info->narHash.type, path); Hash nullHash(htSHA256); - res = info.narHash == nullHash || info.narHash == current.first; + res = info->narHash == nullHash || info->narHash == current.first; } pathContentsGoodCache[path] = res; if (!res) printMsg(lvlError, format("path ‘%1%’ is corrupted or missing!") % path); @@ -3881,7 +3881,7 @@ void LocalStore::repairPath(const Path & path) if (goal->getExitCode() != Goal::ecSuccess) { /* Since substituting the path didn't work, if we have a valid deriver, then rebuild the deriver. */ - Path deriver = queryDeriver(path); + auto deriver = queryPathInfo(path)->deriver; if (deriver != "" && isValidPath(deriver)) { goals.clear(); goals.insert(worker.makeDerivationGoal(deriver, StringSet(), bmRepair)); diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 52afa1b1..13123838 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -407,7 +407,7 @@ void LocalStore::deletePathRecursive(GCState & state, const Path & path) queryReferrers(path, referrers); for (auto & i : referrers) if (i != path) deletePathRecursive(state, i); - size = queryPathInfo(path).narSize; + size = queryPathInfo(path)->narSize; invalidatePathChecked(path); } @@ -485,7 +485,7 @@ bool LocalStore::canReachRoot(GCState & state, PathSet & visited, const Path & p if (state.gcKeepDerivations && isDerivation(path)) { PathSet outputs = queryDerivationOutputs(path); for (auto & i : outputs) - if (isValidPath(i) && queryDeriver(i) == path) + if (isValidPath(i) && queryPathInfo(i)->deriver == path) incoming.insert(i); } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index d6e1e4d7..cef2eb3f 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -577,6 +577,12 @@ uint64_t LocalStore::addValidPath(State & state, } } + { + auto state_(Store::state.lock()); + state_->pathInfoCache.upsert(info.path, std::make_shared(info)); + stats.pathInfoCacheSize = state_->pathInfoCache.size(); + } + return id; } @@ -595,44 +601,44 @@ Hash parseHashField(const Path & path, const string & s) } -ValidPathInfo LocalStore::queryPathInfo(const Path & path) +std::shared_ptr LocalStore::queryPathInfoUncached(const Path & path) { - ValidPathInfo info; - info.path = path; + auto info = std::make_shared(); + info->path = path; assertStorePath(path); - return retrySQLite([&]() { + return retrySQLite>([&]() { auto state(_state.lock()); /* Get the path info. */ auto useQueryPathInfo(state->stmtQueryPathInfo.use()(path)); if (!useQueryPathInfo.next()) - throw Error(format("path ‘%1%’ is not valid") % path); + return std::shared_ptr(); - info.id = useQueryPathInfo.getInt(0); + info->id = useQueryPathInfo.getInt(0); - info.narHash = parseHashField(path, useQueryPathInfo.getStr(1)); + info->narHash = parseHashField(path, useQueryPathInfo.getStr(1)); - info.registrationTime = useQueryPathInfo.getInt(2); + info->registrationTime = useQueryPathInfo.getInt(2); auto s = (const char *) sqlite3_column_text(state->stmtQueryPathInfo, 3); - if (s) info.deriver = s; + if (s) info->deriver = s; /* Note that narSize = NULL yields 0. */ - info.narSize = useQueryPathInfo.getInt(4); + info->narSize = useQueryPathInfo.getInt(4); - info.ultimate = useQueryPathInfo.getInt(5) == 1; + info->ultimate = useQueryPathInfo.getInt(5) == 1; s = (const char *) sqlite3_column_text(state->stmtQueryPathInfo, 6); - if (s) info.sigs = tokenizeString(s, " "); + if (s) info->sigs = tokenizeString(s, " "); /* Get the references. */ - auto useQueryReferences(state->stmtQueryReferences.use()(info.id)); + auto useQueryReferences(state->stmtQueryReferences.use()(info->id)); while (useQueryReferences.next()) - info.references.insert(useQueryReferences.getStr(0)); + info->references.insert(useQueryReferences.getStr(0)); return info; }); @@ -661,17 +667,17 @@ uint64_t LocalStore::queryValidPathId(State & state, const Path & path) } -bool LocalStore::isValidPath(State & state, const Path & path) +bool LocalStore::isValidPath_(State & state, const Path & path) { return state.stmtQueryPathInfo.use()(path).next(); } -bool LocalStore::isValidPath(const Path & path) +bool LocalStore::isValidPathUncached(const Path & path) { return retrySQLite([&]() { auto state(_state.lock()); - return isValidPath(*state, path); + return isValidPath_(*state, path); }); } @@ -716,12 +722,6 @@ void LocalStore::queryReferrers(const Path & path, PathSet & referrers) } -Path LocalStore::queryDeriver(const Path & path) -{ - return queryPathInfo(path).deriver; -} - - PathSet LocalStore::queryValidDerivers(const Path & path) { assertStorePath(path); @@ -996,12 +996,6 @@ void LocalStore::querySubstitutablePathInfos(const PathSet & paths, } -Hash LocalStore::queryPathHash(const Path & path) -{ - return queryPathInfo(path).narHash; -} - - void LocalStore::registerValidPath(const ValidPathInfo & info) { ValidPathInfos infos; @@ -1026,7 +1020,7 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos) for (auto & i : infos) { assert(i.narHash.type == htSHA256); - if (isValidPath(*state, i.path)) + if (isValidPath_(*state, i.path)) updatePathInfo(*state, i); else addValidPath(*state, i, false); @@ -1071,6 +1065,12 @@ void LocalStore::invalidatePath(State & state, const Path & path) /* Note that the foreign key constraints on the Refs table take care of deleting the references entries for `path'. */ + + { + auto state_(Store::state.lock()); + state_->pathInfoCache.erase(path); + stats.pathInfoCacheSize = state_->pathInfoCache.size(); + } } @@ -1225,8 +1225,7 @@ void LocalStore::exportPath(const Path & path, bool sign, printMsg(lvlTalkative, format("exporting path ‘%1%’") % path); - if (!isValidPath(path)) - throw Error(format("path ‘%1%’ is not valid") % path); + auto info = queryPathInfo(path); HashAndWriteSink hashAndWriteSink(sink); @@ -1236,15 +1235,11 @@ void LocalStore::exportPath(const Path & path, bool sign, filesystem corruption from spreading to other machines. Don't complain if the stored hash is zero (unknown). */ Hash hash = hashAndWriteSink.currentHash(); - Hash storedHash = queryPathHash(path); - if (hash != storedHash && storedHash != Hash(storedHash.type)) + if (hash != info->narHash && info->narHash != Hash(info->narHash.type)) throw Error(format("hash of path ‘%1%’ has changed from ‘%2%’ to ‘%3%’!") % path - % printHash(storedHash) % printHash(hash)); + % printHash(info->narHash) % printHash(hash)); - PathSet references; - queryReferences(path, references); - - hashAndWriteSink << exportMagic << path << references << queryDeriver(path); + hashAndWriteSink << exportMagic << path << info->references << info->deriver; if (sign) { Hash hash = hashAndWriteSink.currentHash(); @@ -1440,7 +1435,7 @@ void LocalStore::invalidatePathChecked(const Path & path) SQLiteTxn txn(state->db); - if (isValidPath(*state, path)) { + if (isValidPath_(*state, path)) { PathSet referrers; queryReferrers(*state, path, referrers); referrers.erase(path); /* ignore self-references */ if (!referrers.empty()) @@ -1486,38 +1481,38 @@ bool LocalStore::verifyStore(bool checkContents, bool repair) for (auto & i : validPaths) { try { - ValidPathInfo info = queryPathInfo(i); + auto info = std::const_pointer_cast(std::shared_ptr(queryPathInfo(i))); /* Check the content hash (optionally - slow). */ printMsg(lvlTalkative, format("checking contents of ‘%1%’") % i); - HashResult current = hashPath(info.narHash.type, i); + HashResult current = hashPath(info->narHash.type, i); - if (info.narHash != nullHash && info.narHash != current.first) { + if (info->narHash != nullHash && info->narHash != current.first) { printMsg(lvlError, format("path ‘%1%’ was modified! " "expected hash ‘%2%’, got ‘%3%’") - % i % printHash(info.narHash) % printHash(current.first)); + % i % printHash(info->narHash) % printHash(current.first)); if (repair) repairPath(i); else errors = true; } else { bool update = false; /* Fill in missing hashes. */ - if (info.narHash == nullHash) { + if (info->narHash == nullHash) { printMsg(lvlError, format("fixing missing hash on ‘%1%’") % i); - info.narHash = current.first; + info->narHash = current.first; update = true; } /* Fill in missing narSize fields (from old stores). */ - if (info.narSize == 0) { + if (info->narSize == 0) { printMsg(lvlError, format("updating size field on ‘%1%’ to %2%") % i % current.second); - info.narSize = current.second; + info->narSize = current.second; update = true; } if (update) { auto state(_state.lock()); - updatePathInfo(*state, info); + updatePathInfo(*state, *info); } } @@ -1656,11 +1651,11 @@ void LocalStore::addSignatures(const Path & storePath, const StringSet & sigs) SQLiteTxn txn(state->db); - auto info = queryPathInfo(storePath); + auto info = std::const_pointer_cast(std::shared_ptr(queryPathInfo(storePath))); - info.sigs.insert(sigs.begin(), sigs.end()); + info->sigs.insert(sigs.begin(), sigs.end()); - updatePathInfo(*state, info); + updatePathInfo(*state, *info); txn.commit(); }); diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 14ff92c3..daf394c9 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -102,20 +102,16 @@ public: /* Implementations of abstract store API methods. */ - bool isValidPath(const Path & path) override; + bool isValidPathUncached(const Path & path) override; PathSet queryValidPaths(const PathSet & paths) override; PathSet queryAllValidPaths() override; - ValidPathInfo queryPathInfo(const Path & path) override; - - Hash queryPathHash(const Path & path) override; + std::shared_ptr queryPathInfoUncached(const Path & path) override; void queryReferrers(const Path & path, PathSet & referrers) override; - Path queryDeriver(const Path & path) override; - PathSet queryValidDerivers(const Path & path) override; PathSet queryDerivationOutputs(const Path & path) override; @@ -270,7 +266,7 @@ private: void optimisePath_(OptimiseStats & stats, const Path & path, InodeHash & inodeHash); // Internal versions that are not wrapped in retry_sqlite. - bool isValidPath(State & state, const Path & path); + bool isValidPath_(State & state, const Path & path); void queryReferrers(State & state, const Path & path, PathSet & referrers); /* Add signatures to a ValidPathInfo using the secret keys diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 12472f01..5c284d1b 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -35,12 +35,13 @@ void Store::computeFSClosure(const Path & path, if (includeDerivers && isDerivation(path)) { PathSet outputs = queryDerivationOutputs(path); for (auto & i : outputs) - if (isValidPath(i) && queryDeriver(i) == path) + if (isValidPath(i) && queryPathInfo(i)->deriver == path) edges.insert(i); } } else { - queryReferences(path, edges); + auto info = queryPathInfo(path); + edges = info->references; if (includeOutputs && isDerivation(path)) { PathSet outputs = queryDerivationOutputs(path); @@ -48,10 +49,8 @@ void Store::computeFSClosure(const Path & path, if (isValidPath(i)) edges.insert(i); } - if (includeDerivers) { - Path deriver = queryDeriver(path); - if (isValidPath(deriver)) edges.insert(deriver); - } + if (includeDerivers && isValidPath(info->deriver)) + edges.insert(info->deriver); } for (auto & i : edges) @@ -189,8 +188,10 @@ Paths Store::topoSortPaths(const PathSet & paths) parents.insert(path); PathSet references; - if (isValidPath(path)) - queryReferences(path, references); + try { + references = queryPathInfo(path)->references; + } catch (InvalidPath &) { + } for (auto & i : references) /* Don't traverse into paths that don't exist. That can diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 761e8354..55196397 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -141,7 +141,7 @@ void RemoteStore::setOptions(ref conn) } -bool RemoteStore::isValidPath(const Path & path) +bool RemoteStore::isValidPathUncached(const Path & path) { auto conn(connections->get()); conn->to << wopIsValidPath << path; @@ -239,48 +239,27 @@ void RemoteStore::querySubstitutablePathInfos(const PathSet & paths, } -ValidPathInfo RemoteStore::queryPathInfo(const Path & path) +std::shared_ptr RemoteStore::queryPathInfoUncached(const Path & path) { auto conn(connections->get()); conn->to << wopQueryPathInfo << path; conn->processStderr(); - ValidPathInfo info; - info.path = path; - info.deriver = readString(conn->from); - if (info.deriver != "") assertStorePath(info.deriver); - info.narHash = parseHash(htSHA256, readString(conn->from)); - info.references = readStorePaths(conn->from); - info.registrationTime = readInt(conn->from); - info.narSize = readLongLong(conn->from); + auto info = std::make_shared(); + info->path = path; + info->deriver = readString(conn->from); + if (info->deriver != "") assertStorePath(info->deriver); + info->narHash = parseHash(htSHA256, readString(conn->from)); + info->references = readStorePaths(conn->from); + info->registrationTime = readInt(conn->from); + info->narSize = readLongLong(conn->from); if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 16) { - info.ultimate = readInt(conn->from) != 0; - info.sigs = readStrings(conn->from); + info->ultimate = readInt(conn->from) != 0; + info->sigs = readStrings(conn->from); } return info; } -Hash RemoteStore::queryPathHash(const Path & path) -{ - auto conn(connections->get()); - conn->to << wopQueryPathHash << path; - conn->processStderr(); - string hash = readString(conn->from); - return parseHash(htSHA256, hash); -} - - -void RemoteStore::queryReferences(const Path & path, - PathSet & references) -{ - auto conn(connections->get()); - conn->to << wopQueryReferences << path; - conn->processStderr(); - PathSet references2 = readStorePaths(conn->from); - references.insert(references2.begin(), references2.end()); -} - - void RemoteStore::queryReferrers(const Path & path, PathSet & referrers) { @@ -292,17 +271,6 @@ void RemoteStore::queryReferrers(const Path & path, } -Path RemoteStore::queryDeriver(const Path & path) -{ - auto conn(connections->get()); - conn->to << wopQueryDeriver << path; - conn->processStderr(); - Path drvPath = readString(conn->from); - if (drvPath != "") assertStorePath(drvPath); - return drvPath; -} - - PathSet RemoteStore::queryValidDerivers(const Path & path) { auto conn(connections->get()); @@ -517,6 +485,12 @@ void RemoteStore::collectGarbage(const GCOptions & options, GCResults & results) results.paths = readStrings(conn->from); results.bytesFreed = readLongLong(conn->from); readLongLong(conn->from); // obsolete + + { + auto state_(Store::state.lock()); + state_->pathInfoCache.clear(); + stats.pathInfoCacheSize = 0; + } } diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 45bc4180..4ea8e8a5 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -26,22 +26,16 @@ public: /* Implementations of abstract store API methods. */ - bool isValidPath(const Path & path) override; + bool isValidPathUncached(const Path & path) override; PathSet queryValidPaths(const PathSet & paths) override; PathSet queryAllValidPaths() override; - ValidPathInfo queryPathInfo(const Path & path) override; - - Hash queryPathHash(const Path & path) override; - - void queryReferences(const Path & path, PathSet & references) override; + std::shared_ptr queryPathInfoUncached(const Path & path) override; void queryReferrers(const Path & path, PathSet & referrers) override; - Path queryDeriver(const Path & path) override; - PathSet queryValidDerivers(const Path & path) override; PathSet queryDerivationOutputs(const Path & path) override; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index cc91ed28..6543ed1f 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -225,10 +225,48 @@ Path computeStorePathForText(const string & name, const string & s, } -void Store::queryReferences(const Path & path, PathSet & references) +bool Store::isValidPath(const Path & storePath) { - ValidPathInfo info = queryPathInfo(path); - references.insert(info.references.begin(), info.references.end()); + { + auto state_(state.lock()); + auto res = state_->pathInfoCache.get(storePath); + if (res) { + stats.narInfoReadAverted++; + return *res != 0; + } + } + + return isValidPathUncached(storePath); +} + + +ref Store::queryPathInfo(const Path & storePath) +{ + { + auto state_(state.lock()); + auto res = state_->pathInfoCache.get(storePath); + if (res) { + stats.narInfoReadAverted++; + if (!*res) + throw InvalidPath(format("path ‘%s’ is not valid") % storePath); + return ref(*res); + } + } + + auto info = queryPathInfoUncached(storePath); + + { + auto state_(state.lock()); + state_->pathInfoCache.upsert(storePath, info); + stats.pathInfoCacheSize = state_->pathInfoCache.size(); + } + + if (!info) { + stats.narInfoMissing++; + throw InvalidPath(format("path ‘%s’ is not valid") % storePath); + } + + return ref(info); } @@ -243,19 +281,19 @@ string Store::makeValidityRegistration(const PathSet & paths, for (auto & i : paths) { s += i + "\n"; - ValidPathInfo info = queryPathInfo(i); + auto info = queryPathInfo(i); if (showHash) { - s += printHash(info.narHash) + "\n"; - s += (format("%1%\n") % info.narSize).str(); + s += printHash(info->narHash) + "\n"; + s += (format("%1%\n") % info->narSize).str(); } - Path deriver = showDerivers ? info.deriver : ""; + Path deriver = showDerivers ? info->deriver : ""; s += deriver + "\n"; - s += (format("%1%\n") % info.references.size()).str(); + s += (format("%1%\n") % info->references.size()).str(); - for (auto & j : info.references) + for (auto & j : info->references) s += j + "\n"; } @@ -263,6 +301,12 @@ string Store::makeValidityRegistration(const PathSet & paths, } +const Store::Stats & Store::getStats() +{ + return stats; +} + + ValidPathInfo decodeValidPathInfo(std::istream & str, bool hashGiven) { ValidPathInfo info; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 8827654f..d45e401c 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -3,11 +3,14 @@ #include "hash.hh" #include "serialise.hh" #include "crypto.hh" +#include "lru-cache.hh" +#include "sync.hh" -#include +#include #include #include #include +#include namespace nix { @@ -130,6 +133,8 @@ struct ValidPathInfo /* Verify a single signature. */ bool checkSignature(const PublicKeys & publicKeys, const std::string & sig) const; + + virtual ~ValidPathInfo() { } }; typedef list ValidPathInfos; @@ -169,12 +174,27 @@ class FSAccessor; class Store : public std::enable_shared_from_this { +protected: + + struct State + { + LRUCache> pathInfoCache{64 * 1024}; + }; + + Sync state; + public: virtual ~Store() { } /* Check whether a path is valid. */ - virtual bool isValidPath(const Path & path) = 0; + bool isValidPath(const Path & path); + +protected: + + virtual bool isValidPathUncached(const Path & path) = 0; + +public: /* Query which of the given paths is valid. */ virtual PathSet queryValidPaths(const PathSet & paths) = 0; @@ -183,24 +203,19 @@ public: virtual PathSet queryAllValidPaths() = 0; /* Query information about a valid path. */ - virtual ValidPathInfo queryPathInfo(const Path & path) = 0; + ref queryPathInfo(const Path & path); - /* Query the hash of a valid path. */ - virtual Hash queryPathHash(const Path & path) = 0; +protected: - /* Query the set of outgoing FS references for a store path. The - result is not cleared. */ - virtual void queryReferences(const Path & path, PathSet & references); + virtual std::shared_ptr queryPathInfoUncached(const Path & path) = 0; + +public: /* Queries the set of incoming FS references for a store path. The result is not cleared. */ virtual void queryReferrers(const Path & path, PathSet & referrers) = 0; - /* Query the deriver of a store path. Return the empty string if - no deriver has been set. */ - virtual Path queryDeriver(const Path & path) = 0; - /* Return all currently valid derivations that have `path' as an output. (Note that the result of `queryDeriver()' is the derivation that was actually used to produce `path', which may @@ -373,6 +388,29 @@ public: relation. If p refers to q, then p preceeds q in this list. */ Paths topoSortPaths(const PathSet & paths); + struct Stats + { + std::atomic narInfoRead{0}; + std::atomic narInfoReadAverted{0}; + std::atomic narInfoMissing{0}; + std::atomic narInfoWrite{0}; + std::atomic pathInfoCacheSize{0}; + std::atomic narRead{0}; + std::atomic narReadBytes{0}; + std::atomic narReadCompressedBytes{0}; + std::atomic narWrite{0}; + std::atomic narWriteAverted{0}; + std::atomic narWriteBytes{0}; + std::atomic narWriteCompressedBytes{0}; + std::atomic narWriteCompressionTimeMs{0}; + }; + + const Stats & getStats(); + +protected: + + Stats stats; + }; diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index c10598d5..d62244d1 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -14,8 +14,8 @@ namespace nix { typedef enum { wopIsValidPath = 1, wopHasSubstitutes = 3, - wopQueryPathHash = 4, - wopQueryReferences = 5, + wopQueryPathHash = 4, // obsolete + wopQueryReferences = 5, // obsolete wopQueryReferrers = 6, wopAddToStore = 7, wopAddTextToStore = 8, @@ -26,7 +26,7 @@ typedef enum { wopSyncWithGC = 13, wopFindRoots = 14, wopExportPath = 16, - wopQueryDeriver = 18, + wopQueryDeriver = 18, // obsolete wopSetOptions = 19, wopCollectGarbage = 20, wopQuerySubstitutablePathInfo = 21, diff --git a/src/libutil/lru-cache.hh b/src/libutil/lru-cache.hh index 4344d660..35983aa2 100644 --- a/src/libutil/lru-cache.hh +++ b/src/libutil/lru-cache.hh @@ -79,6 +79,12 @@ public: { return data.size(); } + + void clear() + { + data.clear(); + lru.clear(); + } }; } diff --git a/src/libutil/ref.hh b/src/libutil/ref.hh index 349f24f7..9f5da091 100644 --- a/src/libutil/ref.hh +++ b/src/libutil/ref.hh @@ -43,6 +43,12 @@ public: return p; } + template + ref cast() + { + return ref(std::dynamic_pointer_cast(p)); + } + template operator ref () { diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc index c3cdb839..dfb5da4a 100644 --- a/src/nix-daemon/nix-daemon.cc +++ b/src/nix-daemon/nix-daemon.cc @@ -199,7 +199,7 @@ static void performOp(ref store, bool trusted, unsigned int clientVe case wopQueryPathHash: { Path path = readStorePath(from); startWork(); - Hash hash = store->queryPathHash(path); + auto hash = store->queryPathInfo(path)->narHash; stopWork(); to << printHash(hash); break; @@ -213,7 +213,7 @@ static void performOp(ref store, bool trusted, unsigned int clientVe startWork(); PathSet paths; if (op == wopQueryReferences) - store->queryReferences(path, paths); + paths = store->queryPathInfo(path)->references; else if (op == wopQueryReferrers) store->queryReferrers(path, paths); else if (op == wopQueryValidDerivers) @@ -237,7 +237,7 @@ static void performOp(ref store, bool trusted, unsigned int clientVe case wopQueryDeriver: { Path path = readStorePath(from); startWork(); - Path deriver = store->queryDeriver(path); + auto deriver = store->queryPathInfo(path)->deriver; stopWork(); to << deriver; break; @@ -496,13 +496,13 @@ static void performOp(ref store, bool trusted, unsigned int clientVe case wopQueryPathInfo: { Path path = readStorePath(from); startWork(); - ValidPathInfo info = store->queryPathInfo(path); + auto info = store->queryPathInfo(path); stopWork(); - to << info.deriver << printHash(info.narHash) << info.references - << info.registrationTime << info.narSize; + to << info->deriver << printHash(info->narHash) << info->references + << info->registrationTime << info->narSize; if (GET_PROTOCOL_MINOR(clientVersion) >= 16) { - to << info.ultimate - << info.sigs; + to << info->ultimate + << info->sigs; } break; } diff --git a/src/nix-store/dotgraph.cc b/src/nix-store/dotgraph.cc index 8735cf9b..356a8251 100644 --- a/src/nix-store/dotgraph.cc +++ b/src/nix-store/dotgraph.cc @@ -110,19 +110,13 @@ void printDotGraph(ref store, const PathSet & roots) cout << makeNode(path, symbolicName(path), "#ff0000"); - PathSet references; - store->queryReferences(path, references); - - for (PathSet::iterator i = references.begin(); - i != references.end(); ++i) - { - if (*i != path) { - workList.insert(*i); - cout << makeEdge(*i, path); + for (auto & p : store->queryPathInfo(path)->references) { + if (p != path) { + workList.insert(p); + cout << makeEdge(p, path); } } - #if 0 StoreExpr ne = storeExprFromPath(path); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 179015b5..3a7fcdf4 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -51,7 +51,7 @@ ref ensureLocalStore() static Path useDeriver(Path path) { if (isDerivation(path)) return path; - Path drvPath = store->queryDeriver(path); + Path drvPath = store->queryPathInfo(path)->deriver; if (drvPath == "") throw Error(format("deriver of path ‘%1%’ is not known") % path); return drvPath; @@ -247,8 +247,7 @@ static void printTree(const Path & path, cout << format("%1%%2%\n") % firstPad % path; - PathSet references; - store->queryReferences(path, references); + auto references = store->queryPathInfo(path)->references; /* Topologically sort under the relation A < B iff A \in closure(B). That is, if derivation A is an (possibly indirect) @@ -335,7 +334,10 @@ static void opQuery(Strings opFlags, Strings opArgs) PathSet ps = maybeUseOutputs(followLinksToStorePath(i), useOutput, forceRealise); for (auto & j : ps) { if (query == qRequisites) store->computeFSClosure(j, paths, false, includeOutputs); - else if (query == qReferences) store->queryReferences(j, paths); + else if (query == qReferences) { + for (auto & p : store->queryPathInfo(j)->references) + paths.insert(p); + } else if (query == qReferrers) store->queryReferrers(j, paths); else if (query == qReferrersClosure) store->computeFSClosure(j, paths, true); } @@ -349,7 +351,7 @@ static void opQuery(Strings opFlags, Strings opArgs) case qDeriver: for (auto & i : opArgs) { - Path deriver = store->queryDeriver(followLinksToStorePath(i)); + Path deriver = store->queryPathInfo(followLinksToStorePath(i))->deriver; cout << format("%1%\n") % (deriver == "" ? "unknown-deriver" : deriver); } @@ -372,12 +374,12 @@ static void opQuery(Strings opFlags, Strings opArgs) for (auto & i : opArgs) { PathSet paths = maybeUseOutputs(followLinksToStorePath(i), useOutput, forceRealise); for (auto & j : paths) { - ValidPathInfo info = store->queryPathInfo(j); + auto info = store->queryPathInfo(j); if (query == qHash) { - assert(info.narHash.type == htSHA256); - cout << format("sha256:%1%\n") % printHash32(info.narHash); + assert(info->narHash.type == htSHA256); + cout << format("sha256:%1%\n") % printHash32(info->narHash); } else if (query == qSize) - cout << format("%1%\n") % info.narSize; + cout << format("%1%\n") % info->narSize; } } break; @@ -782,14 +784,14 @@ static void opVerifyPath(Strings opFlags, Strings opArgs) for (auto & i : opArgs) { Path path = followLinksToStorePath(i); printMsg(lvlTalkative, format("checking path ‘%1%’...") % path); - ValidPathInfo info = store->queryPathInfo(path); - HashSink sink(info.narHash.type); + auto info = store->queryPathInfo(path); + HashSink sink(info->narHash.type); store->narFromPath(path, sink); auto current = sink.finish(); - if (current.first != info.narHash) { + if (current.first != info->narHash) { printMsg(lvlError, format("path ‘%1%’ was modified! expected hash ‘%2%’, got ‘%3%’") - % path % printHash(info.narHash) % printHash(current.first)); + % path % printHash(info->narHash) % printHash(current.first)); status = 1; } } @@ -901,13 +903,14 @@ static void opServe(Strings opFlags, Strings opArgs) PathSet paths = readStorePaths(in); // !!! Maybe we want a queryPathInfos? for (auto & i : paths) { - if (!store->isValidPath(i)) - continue; - ValidPathInfo info = store->queryPathInfo(i); - out << info.path << info.deriver << info.references; - // !!! Maybe we want compression? - out << info.narSize // downloadSize - << info.narSize; + try { + auto info = store->queryPathInfo(i); + out << info->path << info->deriver << info->references; + // !!! Maybe we want compression? + out << info->narSize // downloadSize + << info->narSize; + } catch (InvalidPath &) { + } } out << ""; break; diff --git a/src/nix-store/xmlgraph.cc b/src/nix-store/xmlgraph.cc index b6e1c1c4..0f7be7f7 100644 --- a/src/nix-store/xmlgraph.cc +++ b/src/nix-store/xmlgraph.cc @@ -50,15 +50,10 @@ void printXmlGraph(ref store, const PathSet & roots) cout << makeNode(path); - PathSet references; - store->queryReferences(path, references); - - for (PathSet::iterator i = references.begin(); - i != references.end(); ++i) - { - if (*i != path) { - workList.insert(*i); - cout << makeEdge(*i, path); + for (auto & p : store->queryPathInfo(path)->references) { + if (p != path) { + workList.insert(p); + cout << makeEdge(p, path); } } diff --git a/src/nix/sigs.cc b/src/nix/sigs.cc index bcc46c3e..6cff5a08 100644 --- a/src/nix/sigs.cc +++ b/src/nix/sigs.cc @@ -64,19 +64,21 @@ struct CmdCopySigs : StorePathsCommand StringSet newSigs; for (auto & store2 : substituters) { - if (!store2->isValidPath(storePath)) continue; - auto info2 = store2->queryPathInfo(storePath); + try { + auto info2 = store2->queryPathInfo(storePath); - /* Don't import signatures that don't match this - binary. */ - if (info.narHash != info2.narHash || - info.narSize != info2.narSize || - info.references != info2.references) - continue; + /* Don't import signatures that don't match this + binary. */ + if (info->narHash != info2->narHash || + info->narSize != info2->narSize || + info->references != info2->references) + continue; - for (auto & sig : info2.sigs) - if (!info.sigs.count(sig)) - newSigs.insert(sig); + for (auto & sig : info2->sigs) + if (!info->sigs.count(sig)) + newSigs.insert(sig); + } catch (InvalidPath &) { + } } if (!newSigs.empty()) { @@ -122,8 +124,8 @@ struct CmdQueryPathSigs : StorePathsCommand for (auto & storePath : storePaths) { auto info = store->queryPathInfo(storePath); std::cout << storePath << " "; - if (info.ultimate) std::cout << "ultimate "; - for (auto & sig : info.sigs) + if (info->ultimate) std::cout << "ultimate "; + for (auto & sig : info->sigs) std::cout << sig << " "; std::cout << "\n"; } @@ -163,12 +165,12 @@ struct CmdSignPaths : StorePathsCommand for (auto & storePath : storePaths) { auto info = store->queryPathInfo(storePath); - auto info2(info); + auto info2(*info); info2.sigs.clear(); info2.sign(secretKey); assert(!info2.sigs.empty()); - if (!info.sigs.count(*info2.sigs.begin())) { + if (!info->sigs.count(*info2.sigs.begin())) { store->addSignatures(storePath, info2.sigs); added++; } diff --git a/src/nix/verify.cc b/src/nix/verify.cc index 39a4395c..da156780 100644 --- a/src/nix/verify.cc +++ b/src/nix/verify.cc @@ -83,16 +83,16 @@ struct CmdVerify : StorePathsCommand if (!noContents) { - HashSink sink(info.narHash.type); + HashSink sink(info->narHash.type); store->narFromPath(storePath, sink); auto hash = sink.finish(); - if (hash.first != info.narHash) { + if (hash.first != info->narHash) { corrupted = 1; printMsg(lvlError, format("path ‘%s’ was modified! expected hash ‘%s’, got ‘%s’") - % storePath % printHash(info.narHash) % printHash(hash.first)); + % storePath % printHash(info->narHash) % printHash(hash.first)); } } @@ -101,7 +101,7 @@ struct CmdVerify : StorePathsCommand bool good = false; - if (info.ultimate && !sigsNeeded) + if (info->ultimate && !sigsNeeded) good = true; else { @@ -114,18 +114,18 @@ struct CmdVerify : StorePathsCommand for (auto sig : sigs) { if (sigsSeen.count(sig)) continue; sigsSeen.insert(sig); - if (info.checkSignature(publicKeys, sig)) + if (info->checkSignature(publicKeys, sig)) validSigs++; } }; - doSigs(info.sigs); + doSigs(info->sigs); for (auto & store2 : substituters) { if (validSigs >= actualSigsNeeded) break; try { - if (!store2->isValidPath(storePath)) continue; - doSigs(store2->queryPathInfo(storePath).sigs); + doSigs(store2->queryPathInfo(storePath)->sigs); + } catch (InvalidPath &) { } catch (Error & e) { printMsg(lvlError, format(ANSI_RED "error:" ANSI_NORMAL " %s") % e.what()); }