diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 0a44b0cf0..f4e06305a 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -125,11 +125,8 @@ void BinaryCacheStore::writeNarInfo(ref narInfo) upsertFile(narInfoFile, narInfo->to_string(*this), "text/x-nix-narinfo"); - { - auto state_(state.lock()); - state_->pathInfoCache.upsert( - std::string(narInfo->path.to_string()), PathInfoCacheValue{.value = std::shared_ptr(narInfo)}); - } + pathInfoCache->lock()->upsert( + std::string(narInfo->path.to_string()), PathInfoCacheValue{.value = std::shared_ptr(narInfo)}); if (diskCache) diskCache->upsertNarInfo( diff --git a/src/libstore/include/nix/store/store-api.hh b/src/libstore/include/nix/store/store-api.hh index 7d019ea21..dad5c9e8d 100644 --- a/src/libstore/include/nix/store/store-api.hh +++ b/src/libstore/include/nix/store/store-api.hh @@ -310,14 +310,11 @@ protected: } }; - struct State - { - LRUCache pathInfoCache; - }; - void invalidatePathInfoCacheFor(const StorePath & path); - SharedSync state; + // Note: this is a `ref` to avoid false sharing with immutable + // bits of `Store`. + ref>> pathInfoCache; std::shared_ptr diskCache; @@ -860,7 +857,7 @@ public: */ void clearPathInfoCache() { - state.lock()->pathInfoCache.clear(); + pathInfoCache->lock()->clear(); } /** diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 058814f33..f848ddc70 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -722,12 +722,8 @@ uint64_t LocalStore::addValidPath(State & state, const ValidPathInfo & info, boo } } - { - auto state_(Store::state.lock()); - state_->pathInfoCache.upsert( - std::string(info.path.to_string()), - PathInfoCacheValue{.value = std::make_shared(info)}); - } + pathInfoCache->lock()->upsert( + std::string(info.path.to_string()), PathInfoCacheValue{.value = std::make_shared(info)}); return id; } @@ -1022,10 +1018,7 @@ void LocalStore::invalidatePath(State & state, const StorePath & 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(std::string(path.to_string())); - } + pathInfoCache->lock()->erase(std::string(path.to_string())); } const PublicKeys & LocalStore::getPublicKeys() diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 5694fa466..8c0a815d8 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -764,10 +764,7 @@ void RemoteStore::collectGarbage(const GCOptions & options, GCResults & results) results.bytesFreed = readLongLong(conn->from); readLongLong(conn->from); // obsolete - { - auto state_(Store::state.lock()); - state_->pathInfoCache.clear(); - } + pathInfoCache->lock()->clear(); } void RemoteStore::optimiseStore() diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 78d2bbd54..275b8c84b 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -306,7 +306,7 @@ StringSet Store::Config::getDefaultSystemFeatures() Store::Store(const Store::Config & config) : StoreDirConfig{config} , config{config} - , state({(size_t) config.pathInfoCacheSize}) + , pathInfoCache(make_ref((size_t) config.pathInfoCacheSize)) { assertLibStoreInitialized(); } @@ -326,7 +326,7 @@ bool Store::PathInfoCacheValue::isKnownNow() void Store::invalidatePathInfoCacheFor(const StorePath & path) { - state.lock()->pathInfoCache.erase(path.to_string()); + pathInfoCache->lock()->erase(path.to_string()); } std::map> Store::queryStaticPartialDerivationOutputMap(const StorePath & path) @@ -448,13 +448,10 @@ void Store::querySubstitutablePathInfos(const StorePathCAMap & paths, Substituta bool Store::isValidPath(const StorePath & storePath) { - { - auto state_(state.lock()); - auto res = state_->pathInfoCache.get(storePath.to_string()); - if (res && res->isKnownNow()) { - stats.narInfoReadAverted++; - return res->didExist(); - } + auto res = pathInfoCache->lock()->get(storePath.to_string()); + if (res && res->isKnownNow()) { + stats.narInfoReadAverted++; + return res->didExist(); } if (diskCache) { @@ -462,8 +459,7 @@ bool Store::isValidPath(const StorePath & storePath) config.getReference().render(/*FIXME withParams=*/false), std::string(storePath.hashPart())); if (res.first != NarInfoDiskCache::oUnknown) { stats.narInfoReadAverted++; - auto state_(state.lock()); - state_->pathInfoCache.upsert( + pathInfoCache->lock()->upsert( storePath.to_string(), res.first == NarInfoDiskCache::oInvalid ? PathInfoCacheValue{} : PathInfoCacheValue{.value = res.second}); @@ -518,30 +514,25 @@ std::optional> Store::queryPathInfoFromClie { auto hashPart = std::string(storePath.hashPart()); - { - auto res = state.lock()->pathInfoCache.get(storePath.to_string()); - if (res && res->isKnownNow()) { - stats.narInfoReadAverted++; - if (res->didExist()) - return std::make_optional(res->value); - else - return std::make_optional(nullptr); - } + auto res = pathInfoCache->lock()->get(storePath.to_string()); + if (res && res->isKnownNow()) { + stats.narInfoReadAverted++; + if (res->didExist()) + return std::make_optional(res->value); + else + return std::make_optional(nullptr); } if (diskCache) { auto res = diskCache->lookupNarInfo(config.getReference().render(/*FIXME withParams=*/false), hashPart); if (res.first != NarInfoDiskCache::oUnknown) { stats.narInfoReadAverted++; - { - auto state_(state.lock()); - state_->pathInfoCache.upsert( - storePath.to_string(), - res.first == NarInfoDiskCache::oInvalid ? PathInfoCacheValue{} - : PathInfoCacheValue{.value = res.second}); - if (res.first == NarInfoDiskCache::oInvalid || !goodStorePath(storePath, res.second->path)) - return std::make_optional(nullptr); - } + pathInfoCache->lock()->upsert( + storePath.to_string(), + res.first == NarInfoDiskCache::oInvalid ? PathInfoCacheValue{} + : PathInfoCacheValue{.value = res.second}); + if (res.first == NarInfoDiskCache::oInvalid || !goodStorePath(storePath, res.second->path)) + return std::make_optional(nullptr); assert(res.second); return std::make_optional(res.second); } @@ -577,10 +568,7 @@ void Store::queryPathInfo(const StorePath & storePath, CallbackupsertNarInfo(config.getReference().render(/*FIXME withParams=*/false), hashPart, info); - { - auto state_(state.lock()); - state_->pathInfoCache.upsert(storePath.to_string(), PathInfoCacheValue{.value = info}); - } + pathInfoCache->lock()->upsert(storePath.to_string(), PathInfoCacheValue{.value = info}); if (!info || !goodStorePath(storePath, info->path)) { stats.narInfoMissing++; @@ -803,10 +791,7 @@ StorePathSet Store::exportReferences(const StorePathSet & storePaths, const Stor const Store::Stats & Store::getStats() { - { - auto state_(state.readLock()); - stats.pathInfoCacheSize = state_->pathInfoCache.size(); - } + stats.pathInfoCacheSize = pathInfoCache->readLock()->size(); return stats; } diff --git a/src/libutil/include/nix/util/ref.hh b/src/libutil/include/nix/util/ref.hh index fb27949c0..7cf5ef25e 100644 --- a/src/libutil/include/nix/util/ref.hh +++ b/src/libutil/include/nix/util/ref.hh @@ -18,6 +18,9 @@ private: std::shared_ptr p; public: + + using element_type = T; + explicit ref(const std::shared_ptr & p) : p(p) { diff --git a/src/libutil/include/nix/util/sync.hh b/src/libutil/include/nix/util/sync.hh index 262fc328b..3a41d1bd8 100644 --- a/src/libutil/include/nix/util/sync.hh +++ b/src/libutil/include/nix/util/sync.hh @@ -36,6 +36,8 @@ private: public: + using element_type = T; + SyncBase() {} SyncBase(const T & data)