From 7c650ea2418222e2794e94584fe0ec4d0061d9e3 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Wed, 27 Nov 2024 02:09:08 +0100 Subject: [PATCH] libexpr: remove eval caches from EvalState eval caches are not used by actual eval at all, only by the flake-shaped wrappers around evaluation. moving caches into a subclass both clarifies that eval caches and eval states are coupled and separates concerns that should not have been intermixed as they were here. in the future we will want to split up and decouple things even further. that'll have to wait. Change-Id: I7b69510c0f8b212f05fae62e7b992d9475b4841f --- lix/libcmd/command.cc | 8 ++++---- lix/libcmd/command.hh | 7 ++++--- lix/libcmd/installable-attr-path.cc | 4 ++-- lix/libcmd/installable-attr-path.hh | 5 +++-- lix/libcmd/installable-flake.cc | 2 +- lix/libcmd/installable-flake.hh | 4 ++-- lix/libcmd/installable-value.hh | 7 +++---- lix/libcmd/installables.cc | 10 +++------- lix/libexpr/eval-cache.cc | 10 ++++++++++ lix/libexpr/eval-cache.hh | 23 ++++++++++++++++++++++- lix/libexpr/eval.hh | 5 ----- 11 files changed, 54 insertions(+), 31 deletions(-) diff --git a/lix/libcmd/command.cc b/lix/libcmd/command.cc index 10b5a6472..914042248 100644 --- a/lix/libcmd/command.cc +++ b/lix/libcmd/command.cc @@ -102,15 +102,15 @@ ref EvalCommand::getEvalStore() return ref(evalStore); } -ref EvalCommand::getEvalState() +ref EvalCommand::getEvalState() { if (!evalState) { evalState = #if HAVE_BOEHMGC - std::allocate_shared(traceable_allocator(), + std::allocate_shared(traceable_allocator(), searchPath, getEvalStore(), getStore()) #else - std::make_shared( + std::make_shared( searchPath, getEvalStore(), getStore()) #endif ; @@ -121,7 +121,7 @@ ref EvalCommand::getEvalState() evalState->debug = std::make_unique(&AbstractNixRepl::runSimple); }; } - return ref(evalState); + return ref(evalState); } MixOperateOnOptions::MixOperateOnOptions() diff --git a/lix/libcmd/command.hh b/lix/libcmd/command.hh index 1f49952bd..0f9ec66e7 100644 --- a/lix/libcmd/command.hh +++ b/lix/libcmd/command.hh @@ -2,6 +2,7 @@ ///@file #include "lix/libcmd/installable-value.hh" +#include "lix/libexpr/eval-cache.hh" #include "lix/libutil/args.hh" #include "lix/libcmd/common-eval-args.hh" #include "lix/libstore/path.hh" @@ -76,12 +77,12 @@ struct EvalCommand : virtual StoreCommand, MixEvalArgs ref getEvalStore(); - ref getEvalState(); + ref getEvalState(); private: std::shared_ptr evalStore; - std::shared_ptr evalState; + std::shared_ptr evalState; }; /** @@ -331,7 +332,7 @@ void completeFlakeRef(AddCompletions & completions, ref store, std::strin void completeFlakeRefWithFragment( AddCompletions & completions, - ref evalState, + ref evalState, flake::LockFlags lockFlags, Strings attrPathPrefixes, const Strings & defaultFlakeAttrPaths, diff --git a/lix/libcmd/installable-attr-path.cc b/lix/libcmd/installable-attr-path.cc index 6a105bf59..e5a5c6238 100644 --- a/lix/libcmd/installable-attr-path.cc +++ b/lix/libcmd/installable-attr-path.cc @@ -12,7 +12,7 @@ namespace nix { InstallableAttrPath::InstallableAttrPath( - ref state, + ref state, SourceExprCommand & cmd, Value * v, const std::string & attrPath, @@ -93,7 +93,7 @@ DerivedPathsWithInfo InstallableAttrPath::toDerivedPaths() } InstallableAttrPath InstallableAttrPath::parse( - ref state, + ref state, SourceExprCommand & cmd, Value * v, std::string_view prefix, diff --git a/lix/libcmd/installable-attr-path.hh b/lix/libcmd/installable-attr-path.hh index 7af397b2c..623af4e05 100644 --- a/lix/libcmd/installable-attr-path.hh +++ b/lix/libcmd/installable-attr-path.hh @@ -2,6 +2,7 @@ ///@file #include "lix/libcmd/installable-value.hh" +#include "lix/libexpr/eval-cache.hh" #include "lix/libstore/outputs-spec.hh" #include "lix/libcmd/command.hh" #include "lix/libcmd/common-eval-args.hh" @@ -19,7 +20,7 @@ class InstallableAttrPath : public InstallableValue ExtendedOutputsSpec extendedOutputsSpec; InstallableAttrPath( - ref state, + ref state, SourceExprCommand & cmd, Value * v, const std::string & attrPath, @@ -34,7 +35,7 @@ class InstallableAttrPath : public InstallableValue public: static InstallableAttrPath parse( - ref state, + ref state, SourceExprCommand & cmd, Value * v, std::string_view prefix, diff --git a/lix/libcmd/installable-flake.cc b/lix/libcmd/installable-flake.cc index dacf7229a..c739cd548 100644 --- a/lix/libcmd/installable-flake.cc +++ b/lix/libcmd/installable-flake.cc @@ -67,7 +67,7 @@ static std::string showAttrPaths(const std::vector & paths) InstallableFlake::InstallableFlake( SourceExprCommand * cmd, - ref state, + ref state, FlakeRef && flakeRef, std::string_view fragment, ExtendedOutputsSpec extendedOutputsSpec, diff --git a/lix/libcmd/installable-flake.hh b/lix/libcmd/installable-flake.hh index ecb78f0e3..8e74d1533 100644 --- a/lix/libcmd/installable-flake.hh +++ b/lix/libcmd/installable-flake.hh @@ -40,7 +40,7 @@ struct InstallableFlake : InstallableValue InstallableFlake( SourceExprCommand * cmd, - ref state, + ref state, FlakeRef && flakeRef, std::string_view fragment, ExtendedOutputsSpec extendedOutputsSpec, @@ -83,7 +83,7 @@ static inline FlakeRef defaultNixpkgsFlakeRef() } ref openEvalCache( - EvalState & state, + eval_cache::CachingEvalState & state, std::shared_ptr lockedFlake); } diff --git a/lix/libcmd/installable-value.hh b/lix/libcmd/installable-value.hh index 2e5b1311b..df4935cd6 100644 --- a/lix/libcmd/installable-value.hh +++ b/lix/libcmd/installable-value.hh @@ -2,6 +2,7 @@ ///@file #include "lix/libcmd/installables.hh" +#include "lix/libexpr/eval-cache.hh" #include "lix/libexpr/flake/flake.hh" namespace nix { @@ -9,8 +10,6 @@ namespace nix { struct DrvInfo; struct SourceExprCommand; -namespace eval_cache { class EvalCache; class AttrCursor; } - struct App { std::vector context; @@ -71,9 +70,9 @@ struct ExtraPathInfoValue : ExtraPathInfo */ struct InstallableValue : Installable { - ref state; + ref state; - InstallableValue(ref state) : state(state) {} + InstallableValue(ref state) : state(state) {} virtual ~InstallableValue() { } diff --git a/lix/libcmd/installables.cc b/lix/libcmd/installables.cc index ab55c7cca..1a51a36d3 100644 --- a/lix/libcmd/installables.cc +++ b/lix/libcmd/installables.cc @@ -267,7 +267,7 @@ void SourceExprCommand::completeInstallable(AddCompletions & completions, std::s void completeFlakeRefWithFragment( AddCompletions & completions, - ref evalState, + ref evalState, flake::LockFlags lockFlags, Strings attrPathPrefixes, const Strings & defaultFlakeAttrPaths, @@ -390,7 +390,7 @@ static StorePath getDeriver( } ref openEvalCache( - EvalState & state, + eval_cache::CachingEvalState & state, std::shared_ptr lockedFlake) { auto fingerprint = evalSettings.useEvalCache && evalSettings.pureEval @@ -415,11 +415,7 @@ ref openEvalCache( }; if (fingerprint) { - auto search = state.evalCaches.find(fingerprint.value()); - if (search == state.evalCaches.end()) { - search = state.evalCaches.emplace(fingerprint.value(), make_ref(fingerprint, state, rootLoader)).first; - } - return search->second; + return state.getCacheFor(fingerprint.value(), rootLoader); } else { return make_ref(std::nullopt, state, rootLoader); } diff --git a/lix/libexpr/eval-cache.cc b/lix/libexpr/eval-cache.cc index 51deeacee..1c6dfd18c 100644 --- a/lix/libexpr/eval-cache.cc +++ b/lix/libexpr/eval-cache.cc @@ -334,6 +334,16 @@ static std::shared_ptr makeAttrDb( } } +ref CachingEvalState::getCacheFor(Hash hash, RootLoader rootLoader) +{ + if (auto it = caches.find(hash); it != caches.end()) { + return it->second; + } + auto cache = make_ref(hash, *this, rootLoader); + caches.emplace(hash, cache); + return cache; +} + EvalCache::EvalCache( std::optional> useCache, EvalState & state, diff --git a/lix/libexpr/eval-cache.hh b/lix/libexpr/eval-cache.hh index 75d293fe3..255cb2255 100644 --- a/lix/libexpr/eval-cache.hh +++ b/lix/libexpr/eval-cache.hh @@ -13,13 +13,34 @@ namespace nix::eval_cache { struct AttrDb; class AttrCursor; +typedef std::function RootLoader; + +/** + * EvalState with caching support. Historically this was part of EvalState, + * but it was split out to make maintenance easier. This could've been just + * a pair of EvalState and the cache map, but doing so would currently hide + * the rather strong connection between EvalState and these caches. At some + * future time the cache interface should be changed to hide its EvalState. + */ +class CachingEvalState : public EvalState +{ + /** + * A cache for evaluation caches, so as to reuse the same root value if possible + */ + std::map> caches; + +public: + using EvalState::EvalState; + + ref getCacheFor(Hash hash, RootLoader rootLoader); +}; + class EvalCache : public std::enable_shared_from_this { friend class AttrCursor; std::shared_ptr db; EvalState & state; - typedef std::function RootLoader; RootLoader rootLoader; RootValue value; diff --git a/lix/libexpr/eval.hh b/lix/libexpr/eval.hh index 55d3aa44f..6ad0a2a6b 100644 --- a/lix/libexpr/eval.hh +++ b/lix/libexpr/eval.hh @@ -321,11 +321,6 @@ public: return *new EvalErrorBuilder(*this, args...); } - /** - * A cache for evaluation caches, so as to reuse the same root value if possible - */ - std::map> evalCaches; - private: /* Cache for calls to addToStore(); maps source paths to the store