From 4d6a3806d24b54f06ddc0cf234ac993db028cf29 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 12 Mar 2022 00:28:00 +0000 Subject: [PATCH] Decode string context straight to using `StorePath`s I gather decoding happens on demand, so I hope don't think this should have any perf implications one way or the other. --- src/libexpr/eval-cache.cc | 19 +++++++++++-------- src/libexpr/eval.cc | 19 ++++++++++++++----- src/libexpr/eval.hh | 4 ++-- src/libexpr/primops.cc | 8 ++++---- src/libexpr/primops/context.cc | 4 ++-- src/libexpr/value.hh | 6 ++++-- src/nix/app.cc | 2 +- 7 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 7e5b5c9c4..54fa9b741 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -21,6 +21,8 @@ struct AttrDb { std::atomic_bool failed{false}; + const Store & cfg; + struct State { SQLite db; @@ -33,8 +35,9 @@ struct AttrDb std::unique_ptr> _state; - AttrDb(const Hash & fingerprint) - : _state(std::make_unique>()) + AttrDb(const Store & cfg, const Hash & fingerprint) + : cfg(cfg) + , _state(std::make_unique>()) { auto state(_state->lock()); @@ -257,7 +260,7 @@ struct AttrDb NixStringContext context; if (!queryAttribute.isNull(3)) for (auto & s : tokenizeString>(queryAttribute.getStr(3), ";")) - context.push_back(decodeContext(s)); + context.push_back(decodeContext(cfg, s)); return {{rowId, string_t{queryAttribute.getStr(2), context}}}; } case AttrType::Bool: @@ -274,10 +277,10 @@ struct AttrDb } }; -static std::shared_ptr makeAttrDb(const Hash & fingerprint) +static std::shared_ptr makeAttrDb(const Store & cfg, const Hash & fingerprint) { try { - return std::make_shared(fingerprint); + return std::make_shared(cfg, fingerprint); } catch (SQLiteError &) { ignoreException(); return nullptr; @@ -288,7 +291,7 @@ EvalCache::EvalCache( std::optional> useCache, EvalState & state, RootLoader rootLoader) - : db(useCache ? makeAttrDb(*useCache) : nullptr) + : db(useCache ? makeAttrDb(*state.store, *useCache) : nullptr) , state(state) , rootLoader(rootLoader) { @@ -546,7 +549,7 @@ string_t AttrCursor::getStringWithContext() if (auto s = std::get_if(&cachedValue->second)) { bool valid = true; for (auto & c : s->second) { - if (!root->state.store->isValidPath(root->state.store->parseStorePath(c.first))) { + if (!root->state.store->isValidPath(c.first)) { valid = false; break; } @@ -563,7 +566,7 @@ string_t AttrCursor::getStringWithContext() auto & v = forceValue(); if (v.type() == nString) - return {v.string.s, v.getContext()}; + return {v.string.s, v.getContext(*root->state.store)}; else if (v.type() == nPath) return {v.path, {}}; else diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 126e0af8c..f7911e32b 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1903,13 +1903,22 @@ std::string_view EvalState::forceString(Value & v, const Pos & pos) /* Decode a context string ‘!!’ into a pair . */ -NixStringContextElem decodeContext(std::string_view s) +NixStringContextElem decodeContext(const Store & store, std::string_view s) { if (s.at(0) == '!') { size_t index = s.find("!", 1); - return {std::string(s.substr(index + 1)), std::string(s.substr(1, index - 1))}; + return { + store.parseStorePath(s.substr(index + 1)), + std::string(s.substr(1, index - 1)), + }; } else - return {s.at(0) == '/' ? std::string(s) : std::string(s.substr(1)), ""}; + return { + store.parseStorePath( + s.at(0) == '/' + ? s + : s.substr(1)), + "", + }; } @@ -1921,13 +1930,13 @@ void copyContext(const Value & v, PathSet & context) } -NixStringContext Value::getContext() +NixStringContext Value::getContext(const Store & store) { NixStringContext res; assert(internalType == tString); if (string.context) for (const char * * p = string.context; *p; ++p) - res.push_back(decodeContext(*p)); + res.push_back(decodeContext(store, *p)); return res; } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 18480f290..198a62ad2 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -17,7 +17,7 @@ namespace nix { -class Store; +struct Store; class EvalState; class StorePath; enum RepairFlag : bool; @@ -430,7 +430,7 @@ std::string showType(const Value & v); /* Decode a context string ‘!!’ into a pair . */ -NixStringContextElem decodeContext(std::string_view s); +NixStringContextElem decodeContext(const Store & store, std::string_view s); /* If `path' refers to a directory, then append "/default.nix". */ Path resolveExprPath(Path path); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 3124025aa..566cbdfdb 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -43,8 +43,8 @@ StringMap EvalState::realiseContext(const PathSet & context) StringMap res; for (auto & i : context) { - auto [ctxS, outputName] = decodeContext(i); - auto ctx = store->parseStorePath(ctxS); + auto [ctx, outputName] = decodeContext(*store, i); + auto ctxS = store->printStorePath(ctx); if (!store->isValidPath(ctx)) throw InvalidPathError(store->printStorePath(ctx)); if (!outputName.empty() && ctx.isDerivation()) { @@ -1118,8 +1118,8 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * /* Handle derivation outputs of the form ‘!!’. */ else if (path.at(0) == '!') { - auto ctx = decodeContext(path); - drv.inputDrvs[state.store->parseStorePath(ctx.first)].insert(ctx.second); + auto ctx = decodeContext(*state.store, path); + drv.inputDrvs[ctx.first].insert(ctx.second); } /* Otherwise it's a source file. */ diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index ad4de3840..6ec5e4a74 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -82,8 +82,8 @@ static void prim_getContext(EvalState & state, const Pos & pos, Value * * args, drv = std::string(p, 1); path = &drv; } else if (p.at(0) == '!') { - NixStringContextElem ctx = decodeContext(p); - drv = ctx.first; + NixStringContextElem ctx = decodeContext(*state.store, p); + drv = state.store->printStorePath(ctx.first); output = ctx.second; path = &drv; } diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index ee9cb832a..d413060f9 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -57,6 +57,8 @@ struct ExprLambda; struct PrimOp; class Symbol; struct Pos; +class StorePath; +class Store; class EvalState; class XMLWriter; class JSONPlaceholder; @@ -64,7 +66,7 @@ class JSONPlaceholder; typedef int64_t NixInt; typedef double NixFloat; -typedef std::pair NixStringContextElem; +typedef std::pair NixStringContextElem; typedef std::vector NixStringContext; /* External values must descend from ExternalValueBase, so that @@ -370,7 +372,7 @@ public: non-trivial. */ bool isTrivial() const; - NixStringContext getContext(); + NixStringContext getContext(const Store &); auto listItems() { diff --git a/src/nix/app.cc b/src/nix/app.cc index 2563180fb..a4e0df06a 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -70,7 +70,7 @@ UnresolvedApp Installable::toApp(EvalState & state) std::vector context2; for (auto & [path, name] : context) - context2.push_back({state.store->parseStorePath(path), {name}}); + context2.push_back({path, {name}}); return UnresolvedApp{App { .context = std::move(context2),