Skip to content

Commit

Permalink
Move path info caching from BinaryCacheStore to Store
Browse files Browse the repository at this point in the history 
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().
  • Loading branch information
@edolstra
edolstra committed Apr 19, 2016
1 parent 608b026 commit e0204f8
Show file tree
Hide file tree
Showing 21 changed files with 320 additions and 355 deletions.
21 changes: 10 additions & 11 deletions perl/lib/Nix/Store.xs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
118 changes: 38 additions & 80 deletions src/libstore/binary-cache-store.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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>(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<NarInfo>(*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.
Expand All @@ -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<const NarInfo>();

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);
Expand All @@ -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,
Expand Down Expand Up @@ -244,9 +187,24 @@ struct NopSink : ParseSink
{
};

ValidPathInfo BinaryCacheStore::queryPathInfo(const Path & storePath)
std::shared_ptr<ValidPathInfo> 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<NarInfo>(*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>(narInfo);
}

void BinaryCacheStore::querySubstitutablePathInfos(const PathSet & paths,
Expand All @@ -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)
Expand Down Expand Up @@ -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);
}
}

Expand Down
44 changes: 2 additions & 42 deletions src/libstore/binary-cache-store.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
#include "crypto.hh"
#include "store-api.hh"

#include "lru-cache.hh"
#include "sync.hh"
#include "pool.hh"

#include <atomic>
Expand All @@ -22,13 +20,6 @@ private:

std::shared_ptr<Store> localStore;

struct State
{
LRUCache<Path, std::shared_ptr<NarInfo>> narInfoCache{64 * 1024};
};

Sync<State> state;

protected:

BinaryCacheStore(std::shared_ptr<Store> localStore, const Path & secretKeyFile);
Expand All @@ -47,61 +38,30 @@ public:

virtual void init();

struct Stats
{
std::atomic<uint64_t> narInfoRead{0};
std::atomic<uint64_t> narInfoReadAverted{0};
std::atomic<uint64_t> narInfoMissing{0};
std::atomic<uint64_t> narInfoWrite{0};
std::atomic<uint64_t> narInfoCacheSize{0};
std::atomic<uint64_t> narRead{0};
std::atomic<uint64_t> narReadBytes{0};
std::atomic<uint64_t> narReadCompressedBytes{0};
std::atomic<uint64_t> narWrite{0};
std::atomic<uint64_t> narWriteAverted{0};
std::atomic<uint64_t> narWriteBytes{0};
std::atomic<uint64_t> narWriteCompressedBytes{0};
std::atomic<uint64_t> 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(); }

PathSet queryAllValidPaths() override
{ notImpl(); }

ValidPathInfo queryPathInfo(const Path & path) override;

Hash queryPathHash(const Path & path) override
{ notImpl(); }
std::shared_ptr<ValidPathInfo> 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 {}; }

Expand Down
10 changes: 5 additions & 5 deletions src/libstore/build.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -3778,14 +3778,14 @@ bool Worker::pathContentsGood(const Path & path)
std::map<Path, bool>::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);
Expand Down Expand Up @@ -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));
Expand Down
4 changes: 2 additions & 2 deletions src/libstore/gc.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down
Loading

0 comments on commit e0204f8

Please sign in to comment.