Merge pull request #12038 from DeterminateSystems/flake-cache

lookupInFlakeCache(): Fix O(n) time lookup

src/libfetchers/fetchers.hh

@@ -104,6 +104,11 @@ public:
 
     bool operator ==(const Input & other) const noexcept;
 
+    bool operator <(const Input & other) const
+    {
+        return attrs < other.attrs;
+    }
+
     bool contains(const Input & other) const;
 
     /**

src/libflake/flake/flake.cc

@@ -21,23 +21,23 @@ using namespace flake;
 
 namespace flake {
 
-typedef std::pair<StorePath, FlakeRef> FetchedFlake;
-typedef std::vector<std::pair<FlakeRef, FetchedFlake>> FlakeCache;
+struct FetchedFlake
+{
+    FlakeRef lockedRef;
+    StorePath storePath;
+};
+
+typedef std::map<FlakeRef, FetchedFlake> FlakeCache;
 
 static std::optional<FetchedFlake> lookupInFlakeCache(
     const FlakeCache & flakeCache,
     const FlakeRef & flakeRef)
 {
-    // FIXME: inefficient.
-    for (auto & i : flakeCache) {
-        if (flakeRef == i.first) {
-            debug("mapping '%s' to previously seen input '%s' -> '%s",
-                flakeRef, i.first, i.second.second);
-            return i.second;
-        }
-    }
-
-    return std::nullopt;
+    auto i = flakeCache.find(flakeRef);
+    if (i == flakeCache.end()) return std::nullopt;
+    debug("mapping '%s' to previously seen input '%s' -> '%s",
+        flakeRef, i->first, i->second.lockedRef);
+    return i->second;
 }
 
 static std::tuple<StorePath, FlakeRef, FlakeRef> fetchOrSubstituteTree(
@@ -51,7 +51,8 @@ static std::tuple<StorePath, FlakeRef, FlakeRef> fetchOrSubstituteTree(
 
     if (!fetched) {
         if (originalRef.input.isDirect()) {
-            fetched.emplace(originalRef.fetchTree(state.store));
+            auto [storePath, lockedRef] = originalRef.fetchTree(state.store);
+            fetched.emplace(FetchedFlake{.lockedRef = lockedRef, .storePath = storePath});
         } else {
             if (allowLookup) {
                 resolvedRef = originalRef.resolve(
@@ -61,28 +62,28 @@ static std::tuple<StorePath, FlakeRef, FlakeRef> fetchOrSubstituteTree(
                            to resolve indirect flakerefs. */
                         return type == fetchers::Registry::Flag || type == fetchers::Registry::Global;
                     });
-                auto fetchedResolved = lookupInFlakeCache(flakeCache, originalRef);
-                if (!fetchedResolved) fetchedResolved.emplace(resolvedRef.fetchTree(state.store));
-                flakeCache.push_back({resolvedRef, *fetchedResolved});
-                fetched.emplace(*fetchedResolved);
+                fetched = lookupInFlakeCache(flakeCache, originalRef);
+                if (!fetched) {
+                    auto [storePath, lockedRef] = resolvedRef.fetchTree(state.store);
+                    fetched.emplace(FetchedFlake{.lockedRef = lockedRef, .storePath = storePath});
+                }
+                flakeCache.insert_or_assign(resolvedRef, *fetched);
             }
             else {
                 throw Error("'%s' is an indirect flake reference, but registry lookups are not allowed", originalRef);
             }
         }
-        flakeCache.push_back({originalRef, *fetched});
+        flakeCache.insert_or_assign(originalRef, *fetched);
     }
 
-    auto [storePath, lockedRef] = *fetched;
-
     debug("got tree '%s' from '%s'",
-        state.store->printStorePath(storePath), lockedRef);
+        state.store->printStorePath(fetched->storePath), fetched->lockedRef);
 
-    state.allowPath(storePath);
+    state.allowPath(fetched->storePath);
 
-    assert(!originalRef.input.getNarHash() || storePath == originalRef.input.computeStorePath(*state.store));
+    assert(!originalRef.input.getNarHash() || fetched->storePath == originalRef.input.computeStorePath(*state.store));
 
-    return {std::move(storePath), resolvedRef, lockedRef};
+    return {fetched->storePath, resolvedRef, fetched->lockedRef};
 }
 
 static void forceTrivialValue(EvalState & state, Value & value, const PosIdx pos)

src/libflake/flake/flakeref.hh

@@ -49,6 +49,11 @@ struct FlakeRef
 
     bool operator ==(const FlakeRef & other) const = default;
 
+    bool operator <(const FlakeRef & other) const
+    {
+        return std::tie(input, subdir) < std::tie(other.input, other.subdir);
+    }
+
     FlakeRef(fetchers::Input && input, const Path & subdir)
         : input(std::move(input)), subdir(subdir)
     { }

src/libutil/types.hh

@@ -43,9 +43,11 @@ template<typename T>
 struct Explicit {
     T t;
 
-    bool operator ==(const Explicit<T> & other) const
+    bool operator ==(const Explicit<T> & other) const = default;
+
+    bool operator <(const Explicit<T> & other) const
     {
-        return t == other.t;
+        return t < other.t;
     }
 };