From ff1320b85072b5c2ab28143a45a49f06d3545a8f Mon Sep 17 00:00:00 2001 From: Matthew Kenigsberg Date: Mon, 1 Jun 2020 01:39:15 -0600 Subject: [PATCH 1/3] fetchOrSubstituteTree improvements Caches tree in addition to lockedRef, and explicitly writes out the logic for different combinations of cached/uncached flakes and indirect/resolved/locked flakes. This eliminates uneccessary calls to lookupInFlakeCache, fetchTree, maybeLookupFlake, and flakeCache.push_back --- src/libexpr/flake/flake.cc | 52 +++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 54282d40f..c94924371 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -12,25 +12,10 @@ using namespace flake; namespace flake { -/* If 'allowLookup' is true, then resolve 'flakeRef' using the - registries. */ -static FlakeRef maybeLookupFlake( - ref store, - const FlakeRef & flakeRef, - bool allowLookup) -{ - if (!flakeRef.input.isDirect()) { - if (allowLookup) - return flakeRef.resolve(store); - else - throw Error("'%s' is an indirect flake reference, but registry lookups are not allowed", flakeRef); - } else - return flakeRef; -} +typedef std::pair FetchedFlake; +typedef std::vector> FlakeCache; -typedef std::vector> FlakeCache; - -static FlakeRef lookupInFlakeCache( +static std::optional lookupInFlakeCache( const FlakeCache & flakeCache, const FlakeRef & flakeRef) { @@ -38,12 +23,12 @@ static FlakeRef lookupInFlakeCache( for (auto & i : flakeCache) { if (flakeRef == i.first) { debug("mapping '%s' to previously seen input '%s' -> '%s", - flakeRef, i.first, i.second); + flakeRef, i.first, i.second.second); return i.second; } } - return flakeRef; + return std::nullopt; } static std::tuple fetchOrSubstituteTree( @@ -52,17 +37,32 @@ static std::tuple fetchOrSubstituteTree( bool allowLookup, FlakeCache & flakeCache) { - auto resolvedRef = lookupInFlakeCache(flakeCache, - maybeLookupFlake(state.store, - lookupInFlakeCache(flakeCache, originalRef), allowLookup)); + auto fetched = lookupInFlakeCache(flakeCache, originalRef); + FlakeRef resolvedRef = originalRef; - auto [tree, lockedRef] = resolvedRef.fetchTree(state.store); + if (!fetched) { + if (originalRef.input.isDirect()) { + fetched.emplace(originalRef.fetchTree(state.store)); + } else { + if (allowLookup) { + resolvedRef = originalRef.resolve(state.store); + auto fetchedResolved = lookupInFlakeCache(flakeCache, originalRef); + if (!fetchedResolved) fetchedResolved.emplace(resolvedRef.fetchTree(state.store)); + flakeCache.push_back({resolvedRef, fetchedResolved.value()}); + fetched.emplace(fetchedResolved.value()); + } + else { + throw Error("'%s' is an indirect flake reference, but registry lookups are not allowed", originalRef); + } + } + flakeCache.push_back({originalRef, fetched.value()}); + } + + auto [tree, lockedRef] = fetched.value(); debug("got tree '%s' from '%s'", state.store->printStorePath(tree.storePath), lockedRef); - flakeCache.push_back({originalRef, lockedRef}); - flakeCache.push_back({resolvedRef, lockedRef}); if (state.allowedPaths) state.allowedPaths->insert(tree.actualPath); From 768099350666f103131c59853cc7d70c0a6e19cd Mon Sep 17 00:00:00 2001 From: Matthew Kenigsberg Date: Mon, 1 Jun 2020 09:01:37 -0600 Subject: [PATCH 2/3] Tree ctors --- src/libfetchers/fetchers.hh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh index c43cfe50c..2e8c534b0 100644 --- a/src/libfetchers/fetchers.hh +++ b/src/libfetchers/fetchers.hh @@ -16,6 +16,8 @@ struct Tree { Path actualPath; StorePath storePath; + Tree(Path && actualPath, StorePath && storePath) : actualPath(actualPath), storePath(std::move(storePath)) {} + Tree (const Tree & rhs) : actualPath(rhs.actualPath), storePath(rhs.storePath.clone()) {} }; struct InputScheme; From c254254a8088ca303e97b9430484cf25cf8f7806 Mon Sep 17 00:00:00 2001 From: Matthew Kenigsberg Date: Mon, 1 Jun 2020 08:59:26 -0600 Subject: [PATCH 3/3] use Tree ctor --- src/libfetchers/fetchers.cc | 2 +- src/libfetchers/git.cc | 11 +++-------- src/libfetchers/github.cc | 5 +---- src/libfetchers/mercurial.cc | 13 +++++-------- src/libfetchers/path.cc | 5 +---- src/libfetchers/tarball.cc | 10 ++-------- 6 files changed, 13 insertions(+), 33 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index f0d8f72c8..dae8b9fb2 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -117,7 +117,7 @@ std::pair Input::fetch(ref store) const auto actualPath = store->toRealPath(storePath); - return {fetchers::Tree { .actualPath = actualPath, .storePath = std::move(storePath) }, *this}; + return {fetchers::Tree(std::move(actualPath), std::move(storePath)), *this}; } catch (Error & e) { debug("substitution of input '%s' failed: %s", to_string(), e.what()); } diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 4fcf3f542..af4efc9b1 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -189,10 +189,7 @@ struct GitInputScheme : InputScheme input.attrs.insert_or_assign("revCount", getIntAttr(infoAttrs, "revCount")); input.attrs.insert_or_assign("lastModified", getIntAttr(infoAttrs, "lastModified")); return { - Tree { - .actualPath = store->toRealPath(storePath), - .storePath = std::move(storePath), - }, + Tree(store->toRealPath(storePath), std::move(storePath)), input }; }; @@ -273,10 +270,8 @@ struct GitInputScheme : InputScheme haveCommits ? std::stoull(runProgram("git", true, { "-C", actualUrl, "log", "-1", "--format=%ct", "HEAD" })) : 0); return { - Tree { - .actualPath = store->printStorePath(storePath), - .storePath = std::move(storePath), - }, input + Tree(store->printStorePath(storePath), std::move(storePath)), + input }; } } diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index d20b5d00c..cf2554a50 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -140,10 +140,7 @@ struct GitArchiveInputScheme : InputScheme if (auto res = getCache()->lookup(store, immutableAttrs)) { input.attrs.insert_or_assign("lastModified", getIntAttr(res->first, "lastModified")); return { - Tree{ - .actualPath = store->toRealPath(res->second), - .storePath = std::move(res->second), - }, + Tree(store->toRealPath(res->second), std::move(res->second)), input }; } diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 49ed63243..9fae32b83 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -164,10 +164,10 @@ struct MercurialInputScheme : InputScheme auto storePath = store->addToStore("source", actualUrl, true, htSHA256, filter); - return {Tree { - .actualPath = store->printStorePath(storePath), - .storePath = std::move(storePath), - }, input}; + return { + Tree(store->printStorePath(storePath), std::move(storePath)), + input + }; } } @@ -189,10 +189,7 @@ struct MercurialInputScheme : InputScheme assert(!_input.getRev() || _input.getRev() == input.getRev()); input.attrs.insert_or_assign("revCount", getIntAttr(infoAttrs, "revCount")); return { - Tree{ - .actualPath = store->toRealPath(storePath), - .storePath = std::move(storePath), - }, + Tree(store->toRealPath(storePath), std::move(storePath)), input }; }; diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc index cbbb0fa02..99d4b4e8f 100644 --- a/src/libfetchers/path.cc +++ b/src/libfetchers/path.cc @@ -96,10 +96,7 @@ struct PathInputScheme : InputScheme storePath = store->addToStore("source", path); return { - Tree { - .actualPath = store->toRealPath(*storePath), - .storePath = std::move(*storePath), - }, + Tree(store->toRealPath(*storePath), std::move(*storePath)), input }; } diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index 624f8b3fb..9479bb1b3 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -117,10 +117,7 @@ std::pair downloadTarball( if (cached && !cached->expired) return { - Tree { - .actualPath = store->toRealPath(cached->storePath), - .storePath = std::move(cached->storePath), - }, + Tree(store->toRealPath(cached->storePath), std::move(cached->storePath)), getIntAttr(cached->infoAttrs, "lastModified") }; @@ -157,10 +154,7 @@ std::pair downloadTarball( immutable); return { - Tree { - .actualPath = store->toRealPath(*unpackedStorePath), - .storePath = std::move(*unpackedStorePath), - }, + Tree(store->toRealPath(*unpackedStorePath), std::move(*unpackedStorePath)), lastModified, }; }