* Memoize the substitution function.

* Print some substitution statistics.
* Option to turn off the closed term optimization.

src/libexpr/eval.cc

@@ -20,6 +20,8 @@ namespace nix {
 int cacheTerms;
 
 bool shortCircuit;
+bool closedTerms; // don't substitute under terms known to be closed
+bool substCache; // memoization of the term substitution function 
 
 #define maxActiveCalls 4096
 
@@ -39,6 +41,8 @@ EvalState::EvalState()
     if (!string2Int(getEnv("NIX_TERM_CACHE"), cacheTerms)) cacheTerms = 1;
     shortCircuit = getEnv("NIX_SHORT_CIRCUIT", "0") == "1";
     strictMode = getEnv("NIX_STRICT", "0") == "1";
+    closedTerms = getEnv("NIX_CLOSED_TERMS", "1") == "1";
+    substCache = getEnv("NIX_SUBST_CACHE", "1") == "1";
 
     ATprotectMemory(activeCalls, maxActiveCalls);
 }
@@ -882,7 +886,7 @@ Expr evalExpr(EvalState & state, Expr e)
             throw;
         }
         state.normalForms.set(e, nf);
-        maybeShortCircuit(state, e, nf);
+        if (shortCircuit) maybeShortCircuit(state, e, nf);
         return nf;
 
     }
@@ -981,17 +985,24 @@ extern "C" {
     unsigned long AT_calcAllocatedSize();
 }
 
+
+unsigned int substs = 0;
+unsigned int substsCached = 0;
+
+
 void printEvalStats(EvalState & state)
 {
     char x;
     bool showStats = getEnv("NIX_SHOW_STATS", "0") != "0";
     printMsg(lvlError, format("FNORD %1%") % fnord);
     printMsg(showStats ? lvlInfo : lvlDebug,
-        format("evaluated %1% expressions, %2% cache hits, %3%%% efficiency, used %4% ATerm bytes, used %5% bytes of stack space")
+        format("evaluated %1% expressions, %2% cache hits, %3%%% efficiency, used %4% ATerm bytes, used %5% bytes of stack space, %6% substitutions (%7% cached)")
         % state.nrEvaluated % state.nrCached
         % ((float) state.nrCached / (float) state.nrEvaluated * 100)
         % AT_calcAllocatedSize()
-        % (&x - deepestStack));
+        % (&x - deepestStack)
+        % substs
+        % substsCached);
     if (showStats)
         printATermMapStats();
 }

src/libexpr/nixexpr.cc

@@ -109,7 +109,16 @@ Expr makeAttrs(const ATermMap & attrs)
 }
 
 
-Expr substitute(const Substitution & subs, Expr e)
+extern unsigned int substs;
+extern unsigned int substsCached;
+extern bool closedTerms;
+extern bool substCache;
+
+
+static Expr substitute(ATermMap & done, const Substitution & subs, Expr e);
+
+
+static Expr substitute2(ATermMap & done, const Substitution & subs, Expr e)
 {
     checkInterrupt();
 
@@ -117,19 +126,20 @@ Expr substitute(const Substitution & subs, Expr e)
 
     ATerm name, pos, e2;
 
+    substs++;
+
     /* As an optimisation, don't substitute in subterms known to be
        closed. */
-    if (matchClosed(e, e2)) return e;
+    if (closedTerms && matchClosed(e, e2)) return e;
 
     if (matchVar(e, name)) {
         Expr sub = subs.lookup(name);
-        if (sub == makeRemoved()) sub = 0;
         Expr wrapped;
         /* Add a "closed" wrapper around terms that aren't already
            closed.  The check is necessary to prevent repeated
            wrapping, e.g., closed(closed(closed(...))), which kills
            caching. */
-        return sub ? (matchClosed(sub, wrapped) ? sub : makeClosed(sub)) : e;
+        return sub ? ((!closedTerms || matchClosed(sub, wrapped)) ? sub : makeClosed(sub)) : e;
     }
 
     /* In case of a function, filter out all variables bound by this
@@ -141,18 +151,30 @@ Expr substitute(const Substitution & subs, Expr e)
         for (ATermIterator i(formals); i; ++i) {
             ATerm d1, d2;
             if (!matchFormal(*i, name, d1, d2)) abort();
-            map.set(name, makeRemoved());
+            if (subs.lookup(name))
+                map.set(name, constRemoved);
         }
-        Substitution subs2(&subs, &map);
+        if (map.size() == 0)
             return makeFunction(
-            (ATermList) substitute(subs2, (ATerm) formals),
+                (ATermList) substitute(done, subs, (ATerm) formals),
-            substitute(subs2, body), pos);
+                substitute(done, subs, body), pos);
+        else {
+            Substitution subs2(&subs, &map);
+            ATermMap done2(128);
+            return makeFunction(
+                (ATermList) substitute(done2, subs2, (ATerm) formals),
+                substitute(done2, subs2, body), pos);
+        }
     }
 
     if (matchFunction1(e, name, body, pos)) {
+        if (subs.lookup(name)) {
             ATermMap map(1);
-        map.set(name, makeRemoved());
+            map.set(name, constRemoved);
-        return makeFunction1(name, substitute(Substitution(&subs, &map), body), pos);
+            ATermMap done2(128);
+            return makeFunction1(name, substitute(done2, Substitution(&subs, &map), body), pos);
+        } else
+            return makeFunction1(name, substitute(done, subs, body), pos);
     }
         
     /* Idem for a mutually recursive attribute set. */
@@ -160,14 +182,21 @@ Expr substitute(const Substitution & subs, Expr e)
     if (matchRec(e, rbnds, nrbnds)) {
         ATermMap map(ATgetLength(rbnds) + ATgetLength(nrbnds));
         for (ATermIterator i(rbnds); i; ++i)
-            if (matchBind(*i, name, e2, pos)) map.set(name, makeRemoved());
+            if (matchBind(*i, name, e2, pos) && subs.lookup(name))
-            else abort(); /* can't happen */
+                map.set(name, constRemoved);
         for (ATermIterator i(nrbnds); i; ++i)
-            if (matchBind(*i, name, e2, pos)) map.set(name, makeRemoved());
+            if (matchBind(*i, name, e2, pos) && subs.lookup(name))
-            else abort(); /* can't happen */
+                map.set(name, constRemoved);
+        if (map.size() == 0)
             return makeRec(
-            (ATermList) substitute(Substitution(&subs, &map), (ATerm) rbnds),
+                (ATermList) substitute(done, subs, (ATerm) rbnds),
-            (ATermList) substitute(subs, (ATerm) nrbnds));
+                (ATermList) substitute(done, subs, (ATerm) nrbnds));
+        else {
+            ATermMap done2(128);
+            return makeRec(
+                (ATermList) substitute(done2, Substitution(&subs, &map), (ATerm) rbnds),
+                (ATermList) substitute(done, subs, (ATerm) nrbnds));
+        }
     }
 
     if (ATgetType(e) == AT_APPL) {
@@ -178,7 +207,7 @@ Expr substitute(const Substitution & subs, Expr e)
 
         for (int i = 0; i < arity; ++i) {
             ATerm arg = ATgetArgument(e, i);
-            args[i] = substitute(subs, arg);
+            args[i] = substitute(done, subs, arg);
             if (args[i] != arg) changed = true;
         }
         
@@ -189,8 +218,12 @@ Expr substitute(const Substitution & subs, Expr e)
         unsigned int len = ATgetLength((ATermList) e);
         ATerm es[len];
         ATermIterator i((ATermList) e);
-        for (unsigned int j = 0; i; ++i, ++j)
+        bool changed = false;
-            es[j] = substitute(subs, *i);
+        for (unsigned int j = 0; i; ++i, ++j) {
+            es[j] = substitute(done, subs, *i);
+            if (es[j] != *i) changed = true;
+        }
+        if (!changed) return e;
         ATermList out = ATempty;
         for (unsigned int j = len; j; --j)
             out = ATinsert(out, es[j - 1]);
@@ -201,6 +234,26 @@ Expr substitute(const Substitution & subs, Expr e)
 }
 
 
+static Expr substitute(ATermMap & done, const Substitution & subs, Expr e)
+{
+    Expr res = done[e];
+    if (substCache && res) {
+        substsCached++;
+        return res;
+    }
+    res = substitute2(done, subs, e);
+    done.set(e, res);
+    return res;
+}
+
+
+Expr substitute(const Substitution & subs, Expr e)
+{
+    ATermMap done(256);
+    return substitute(done, subs, e);
+}
+
+
 Expr allocCells(Expr e)
 {
     checkInterrupt();

src/libexpr/nixexpr.hh

@@ -34,6 +34,9 @@ typedef ATerm Pos;
 typedef vector<ATerm> ATermVector;
 
 
+extern Expr constRemoved;
+
+
 /* A substitution is a linked list of ATermMaps that map names to
    identifiers.  We use a list of ATermMaps rather than a single to
    make it easy to grow or shrink a substitution when entering a
@@ -53,7 +56,8 @@ struct Substitution
     {
         Expr x;
         for (const Substitution * s(this); s; s = s->prev)
-            if ((x = s->map->get(name))) return x;
+            if ((x = s->map->get(name)))
+                return x == constRemoved ? 0 : x;
         return 0;
     }
 };