From 66d7128512e60998d2632cf7493a5133cfd2d03c Mon Sep 17 00:00:00 2001
From: eldritch horrors <pennae@lix.systems>
Date: Wed, 27 Nov 2024 02:09:08 +0100
Subject: [PATCH] libexpr: remove EvalState parse caches

nothing needs these any more. the CLI calls evalFile, but only in places
where it'll only be called *once* per process lifetime. __import does so
too, but import doesn't care about the parse tree, only the eval result.

interestingly this improves eval performance by 6% on system eval unless
GC never occurs. no idea why this makes a GCing eval faster, but it does

Change-Id: I8289528550244e0a8b5ebc7284d8fb9aaac59e20
---
 lix/libexpr/eval.cc | 43 +++++++++++--------------------------------
 lix/libexpr/eval.hh | 16 ----------------
 2 files changed, 11 insertions(+), 48 deletions(-)

diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc
index 8539510a1..26021a509 100644
--- a/lix/libexpr/eval.cc
+++ b/lix/libexpr/eval.cc
@@ -941,51 +941,24 @@ void EvalState::evalFile(const SourcePath & path_, Value & v, bool mustBeTrivial
     }
 
     debug("evaluating file '%1%'", resolvedPath);
-    Expr * e = nullptr;
-
-    auto j = fileParseCache.find(resolvedPath);
-    if (j != fileParseCache.end())
-        e = j->second;
-
-    if (!e)
-        e = &parseExprFromFile(checkSourcePath(resolvedPath));
-
-    cacheFile(path, resolvedPath, e, v, mustBeTrivial);
-}
-
-
-void EvalState::resetFileCache()
-{
-    fileEvalCache.clear();
-    fileParseCache.clear();
-}
-
-
-void EvalState::cacheFile(
-    const SourcePath & path,
-    const SourcePath & resolvedPath,
-    Expr * e,
-    Value & v,
-    bool mustBeTrivial)
-{
-    fileParseCache[resolvedPath] = e;
+    Expr & e = parseExprFromFile(checkSourcePath(resolvedPath));
 
     try {
         auto dts = debug
             ? makeDebugTraceStacker(
                 *this,
-                *e,
+                e,
                 this->baseEnv,
-                e->getPos() ? std::make_shared<Pos>(positions[e->getPos()]) : nullptr,
+                e.getPos() ? std::make_shared<Pos>(positions[e.getPos()]) : nullptr,
                 "while evaluating the file '%1%':", resolvedPath.to_string())
             : nullptr;
 
         // Enforce that 'flake.nix' is a direct attrset, not a
         // computation.
         if (mustBeTrivial &&
-            !(dynamic_cast<ExprAttrs *>(e)))
+            !(dynamic_cast<ExprAttrs *>(&e)))
             error<EvalError>("file '%s' must be an attribute set", path).debugThrow();
-        eval(*e, v);
+        eval(e, v);
     } catch (Error & e) {
         addErrorTrace(e, "while evaluating the file '%1%':", resolvedPath.to_string());
         throw;
@@ -996,6 +969,12 @@ void EvalState::cacheFile(
 }
 
 
+void EvalState::resetFileCache()
+{
+    fileEvalCache.clear();
+}
+
+
 void EvalState::eval(Expr & e, Value & v)
 {
     e.eval(*this, baseEnv, v);
diff --git a/lix/libexpr/eval.hh b/lix/libexpr/eval.hh
index 6ad0a2a6b..27dad80bc 100644
--- a/lix/libexpr/eval.hh
+++ b/lix/libexpr/eval.hh
@@ -327,12 +327,6 @@ private:
        paths. */
     std::map<SourcePath, StorePath> srcToStore;
 
-    /**
-     * A cache from path names to parse trees.
-     */
-    using FileParseCache = GcMap<SourcePath, Expr *>;
-    FileParseCache fileParseCache;
-
     /**
      * A cache from path names to values.
      */
@@ -426,16 +420,6 @@ public:
      */
     void evalFile(const SourcePath & path, Value & v, bool mustBeTrivial = false);
 
-    /**
-     * Like `evalFile`, but with an already parsed expression.
-     */
-    void cacheFile(
-        const SourcePath & path,
-        const SourcePath & resolvedPath,
-        Expr * e,
-        Value & v,
-        bool mustBeTrivial = false);
-
     void resetFileCache();
 
     /**