Factor out NarCache from RemoteFSAccessor

src/libstore/binary-cache-store.cc

@@ -156,7 +156,7 @@ ref<const ValidPathInfo> BinaryCacheStore::addToStoreCommon(
             config.compression, teeSinkCompressed, config.parallelCompression, config.compressionLevel);
         TeeSink teeSinkUncompressed{*compressionSink, narHashSink};
         TeeSource teeSource{narSource, teeSinkUncompressed};
-        narAccessor = makeNarAccessor(teeSource);
+        narAccessor = makeNarAccessor(parseNarListing(teeSource));
         compressionSink->finish();
         fileSink.flush();
     }

src/libstore/include/nix/store/remote-fs-accessor.hh

@@ -3,6 +3,7 @@
 
 #include "nix/util/source-accessor.hh"
 #include "nix/util/ref.hh"
+#include "nix/util/nar-cache.hh"
 #include "nix/store/store-api.hh"
 
 namespace nix {
@@ -13,20 +14,15 @@ class RemoteFSAccessor : public SourceAccessor
 
     /**
      * Map from store path hash part to NAR hash. Used to then look up
-     * in `nars`. The indirection allows avoiding opening multiple
+     * in the NAR cache. The indirection allows avoiding opening multiple
      * redundant NAR accessors for the same NAR.
      */
     std::map<std::string, Hash, std::less<>> narHashes;
 
-    /**
-     * Map from NAR hash to NAR accessor.
-     */
-    std::map<Hash, ref<SourceAccessor>> nars;
+    NarCache narCache;
 
     bool requireValidPath;
 
-    std::optional<std::filesystem::path> cacheDir;
-
     std::pair<ref<SourceAccessor>, CanonPath> fetch(const CanonPath & path);
 
     friend struct BinaryCacheStore;

src/libstore/remote-fs-accessor.cc

@@ -1,21 +1,13 @@
-#include <nlohmann/json.hpp>
 #include "nix/store/remote-fs-accessor.hh"
-#include "nix/util/nar-accessor.hh"
-
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
 
 namespace nix {
 
 RemoteFSAccessor::RemoteFSAccessor(
-    ref<Store> store, bool requireValidPath, std::optional<std::filesystem::path> cacheDir_)
+    ref<Store> store, bool requireValidPath, std::optional<std::filesystem::path> cacheDir)
     : store(store)
+    , narCache(cacheDir)
     , requireValidPath(requireValidPath)
-    , cacheDir(std::move(cacheDir_))
 {
-    if (cacheDir)
-        createDirs(*cacheDir);
 }
 
 std::pair<ref<SourceAccessor>, CanonPath> RemoteFSAccessor::fetch(const CanonPath & path)
@@ -28,72 +20,18 @@ std::pair<ref<SourceAccessor>, CanonPath> RemoteFSAccessor::fetch(const CanonPat
 
 std::shared_ptr<SourceAccessor> RemoteFSAccessor::accessObject(const StorePath & storePath)
 {
-    if (auto * narHash = get(narHashes, storePath.hashPart())) {
-        if (auto * accessor = get(nars, *narHash))
-            return *accessor;
-    }
+    // Check if we already have the NAR hash for this store path
+    if (auto * narHash = get(narHashes, storePath.hashPart()))
+        return narCache.getOrInsert(*narHash, [&](Sink & sink) { store->narFromPath(storePath, sink); });
 
+    // Query the path info to get the NAR hash
     auto info = store->queryPathInfo(storePath);
 
-    auto cacheAccessor = [&](ref<SourceAccessor> accessor) {
-        narHashes.emplace(storePath.hashPart(), info->narHash);
-        nars.emplace(info->narHash, accessor);
-        return accessor;
-    };
-
-    auto getNar = [&]() {
-        StringSink sink;
-        store->narFromPath(storePath, sink);
-        return std::move(sink.s);
-    };
-
-    if (cacheDir) {
-        auto makeCacheFile = [&](const std::string & ext) {
-            auto res = *cacheDir / info->narHash.to_string(HashFormat::Nix32, false);
-            res += ".";
-            res += ext;
-            return res;
-        };
-
-        auto cacheFile = makeCacheFile("nar");
-        auto listingFile = makeCacheFile("ls");
-
-        if (nix::pathExists(cacheFile)) {
-            try {
-                return cacheAccessor(makeLazyNarAccessor(
-                    nlohmann::json::parse(nix::readFile(listingFile)).template get<NarListing>(),
-                    seekableGetNarBytes(cacheFile)));
-            } catch (SystemError &) {
-            }
-
-            try {
-                return cacheAccessor(makeNarAccessor(nix::readFile(cacheFile)));
-            } catch (SystemError &) {
-            }
-        }
-
-        auto nar = getNar();
-
-        try {
-            /* FIXME: do this asynchronously. */
-            writeFile(cacheFile, nar);
-        } catch (...) {
-            ignoreExceptionExceptInterrupt();
-        }
-
-        auto narAccessor = makeNarAccessor(std::move(nar));
-
-        try {
-            nlohmann::json j = narAccessor->getListing();
-            writeFile(listingFile, j.dump());
-        } catch (...) {
-            ignoreExceptionExceptInterrupt();
-        }
-
-        return cacheAccessor(narAccessor);
-    }
+    // Cache the mapping from store path to NAR hash
+    narHashes.emplace(storePath.hashPart(), info->narHash);
 
-    return cacheAccessor(makeNarAccessor(getNar()));
+    // Get or create the NAR accessor
+    return narCache.getOrInsert(info->narHash, [&](Sink & sink) { store->narFromPath(storePath, sink); });
 }
 
 std::optional<SourceAccessor::Stat> RemoteFSAccessor::maybeLstat(const CanonPath & path)

src/libutil/include/nix/util/meson.build

@@ -53,6 +53,7 @@ headers = files(
   'mounted-source-accessor.hh',
   'muxable-pipe.hh',
   'nar-accessor.hh',
+  'nar-cache.hh',
   'nar-listing.hh',
   'os-string.hh',
   'pool.hh',

src/libutil/include/nix/util/nar-accessor.hh

@@ -27,7 +27,12 @@ struct NarAccessor : SourceAccessor
  */
 ref<NarAccessor> makeNarAccessor(std::string && nar);
 
-ref<NarAccessor> makeNarAccessor(Source & source);
+/**
+ * This NAR accessor doesn't actually access a NAR, and thus cannot read
+ * the contents of files. It just conveys the information which is
+ * gotten from `listing`.
+ */
+ref<NarAccessor> makeNarAccessor(NarListing listing);
 
 /**
  * Create a NAR accessor from a NAR listing (in the format produced by
@@ -44,12 +49,9 @@ GetNarBytes seekableGetNarBytes(const std::filesystem::path & path);
 
 GetNarBytes seekableGetNarBytes(Descriptor fd);
 
-ref<NarAccessor> makeLazyNarAccessor(NarListing listing, GetNarBytes getNarBytes);
-
 /**
- * Creates a NAR accessor from a given stream and a GetNarBytes getter.
- * @param source Consumed eagerly. References to it are not persisted in the resulting SourceAccessor.
+ * Creates a NAR accessor from a given listing and a `GetNarBytes` getter.
  */
-ref<NarAccessor> makeLazyNarAccessor(Source & source, GetNarBytes getNarBytes);
+ref<NarAccessor> makeLazyNarAccessor(NarListing listing, GetNarBytes getNarBytes);
 
 } // namespace nix

src/libutil/include/nix/util/nar-cache.hh

@@ -0,0 +1,47 @@
+#pragma once
+
+#include "nix/util/hash.hh"
+#include "nix/util/nar-accessor.hh"
+#include "nix/util/ref.hh"
+#include "nix/util/source-accessor.hh"
+
+#include <filesystem>
+#include <functional>
+#include <map>
+#include <optional>
+
+namespace nix {
+
+/**
+ * A cache for NAR accessors with optional disk caching.
+ */
+class NarCache
+{
+    /**
+     * Optional directory for caching NARs and listings on disk.
+     */
+    std::optional<std::filesystem::path> cacheDir;
+
+    /**
+     * Map from NAR hash to NAR accessor.
+     */
+    std::map<Hash, ref<SourceAccessor>> nars;
+
+public:
+
+    /**
+     * Create a NAR cache with an optional cache directory for disk storage.
+     */
+    NarCache(std::optional<std::filesystem::path> cacheDir = {});
+
+    /**
+     * Lookup or create a NAR accessor, optionally using disk cache.
+     *
+     * @param narHash The NAR hash to use as cache key
+     * @param populate Function called with a Sink to populate the NAR if not cached
+     * @return The cached or newly created accessor
+     */
+    ref<SourceAccessor> getOrInsert(const Hash & narHash, std::function<void(Sink &)> populate);
+};
+
+} // namespace nix

src/libutil/meson.build

@@ -151,6 +151,7 @@ sources = [ config_priv_h ] + files(
   'memory-source-accessor/json.cc',
   'mounted-source-accessor.cc',
   'nar-accessor.cc',
+  'nar-cache.cc',
   'nar-listing.cc',
   'pos-table.cc',
   'position.cc',

src/libutil/nar-accessor.cc

@@ -34,14 +34,8 @@ struct NarAccessorImpl : NarAccessor
     {
     }
 
-    NarAccessorImpl(Source & source)
-        : root{parseNarListing(source)}
-    {
-    }
-
-    NarAccessorImpl(Source & source, GetNarBytes getNarBytes)
-        : root{parseNarListing(source)}
-        , getNarBytes{std::move(getNarBytes)}
+    NarAccessorImpl(NarListing && listing)
+        : root{std::move(listing)}
     {
     }
 
@@ -147,21 +141,16 @@ ref<NarAccessor> makeNarAccessor(std::string && nar)
     return make_ref<NarAccessorImpl>(std::move(nar));
 }
 
-ref<NarAccessor> makeNarAccessor(Source & source)
+ref<NarAccessor> makeNarAccessor(NarListing listing)
 {
-    return make_ref<NarAccessorImpl>(source);
+    return make_ref<NarAccessorImpl>(std::move(listing));
 }
 
 ref<NarAccessor> makeLazyNarAccessor(NarListing listing, GetNarBytes getNarBytes)
 {
     return make_ref<NarAccessorImpl>(std::move(listing), getNarBytes);
 }
 
-ref<NarAccessor> makeLazyNarAccessor(Source & source, GetNarBytes getNarBytes)
-{
-    return make_ref<NarAccessorImpl>(source, getNarBytes);
-}
-
 GetNarBytes seekableGetNarBytes(const std::filesystem::path & path)
 {
     AutoCloseFD fd = openFileReadonly(path);

src/libutil/nar-cache.cc

@@ -0,0 +1,84 @@
+#include "nix/util/nar-cache.hh"
+#include "nix/util/file-system.hh"
+
+#include <nlohmann/json.hpp>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+namespace nix {
+
+NarCache::NarCache(std::optional<std::filesystem::path> cacheDir_)
+    : cacheDir(std::move(cacheDir_))
+{
+    if (cacheDir)
+        createDirs(*cacheDir);
+}
+
+ref<SourceAccessor> NarCache::getOrInsert(const Hash & narHash, std::function<void(Sink &)> populate)
+{
+    // Check in-memory cache first
+    if (auto * accessor = get(nars, narHash))
+        return *accessor;
+
+    auto cacheAccessor = [&](ref<SourceAccessor> accessor) {
+        nars.emplace(narHash, accessor);
+        return accessor;
+    };
+
+    auto getNar = [&]() {
+        StringSink sink;
+        populate(sink);
+        return std::move(sink.s);
+    };
+
+    if (cacheDir) {
+        auto makeCacheFile = [&](const std::string & ext) {
+            auto res = *cacheDir / narHash.to_string(HashFormat::Nix32, false);
+            res += ".";
+            res += ext;
+            return res;
+        };
+
+        auto cacheFile = makeCacheFile("nar");
+        auto listingFile = makeCacheFile("ls");
+
+        if (nix::pathExists(cacheFile)) {
+            try {
+                return cacheAccessor(makeLazyNarAccessor(
+                    nlohmann::json::parse(nix::readFile(listingFile)).template get<NarListing>(),
+                    seekableGetNarBytes(cacheFile)));
+            } catch (SystemError &) {
+            }
+
+            try {
+                return cacheAccessor(makeNarAccessor(nix::readFile(cacheFile)));
+            } catch (SystemError &) {
+            }
+        }
+
+        auto nar = getNar();
+
+        try {
+            /* FIXME: do this asynchronously. */
+            writeFile(cacheFile, nar);
+        } catch (...) {
+            ignoreExceptionExceptInterrupt();
+        }
+
+        auto narAccessor = makeNarAccessor(std::move(nar));
+
+        try {
+            nlohmann::json j = narAccessor->getListing();
+            writeFile(listingFile, j.dump());
+        } catch (...) {
+            ignoreExceptionExceptInterrupt();
+        }
+
+        return cacheAccessor(narAccessor);
+    }
+
+    return cacheAccessor(makeNarAccessor(getNar()));
+}
+
+} // namespace nix

src/nix/ls.cc

@@ -154,7 +154,7 @@ struct CmdLsNar : Command, MixLs
         if (!fd)
             throw NativeSysError("opening NAR file %s", PathFmt(narPath));
         auto source = FdSource{fd.get()};
-        list(makeLazyNarAccessor(source, seekableGetNarBytes(fd.get())), CanonPath{path});
+        list(makeLazyNarAccessor(parseNarListing(source), seekableGetNarBytes(fd.get())), CanonPath{path});
     }
 };