From 8ac06726b92fff66714ceee8af89068ac876875a Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Sat, 23 Oct 2010 20:07:47 +0000
Subject: [PATCH] * Make Value smaller by not storing redundant PrimOp info. *
 Clear pointers in Values after overwriting them to make sure that no  
 objects are kept alive unnecessarily.

---
 src/libexpr/eval.cc | 37 +++++++++++++++++++++----------------
 src/libexpr/eval.hh | 32 +++++++++++++++++++++++---------
 2 files changed, 44 insertions(+), 25 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index faf18cb7f4..72a3bf9b9c 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -153,15 +153,14 @@ void EvalState::addConstant(const string & name, Value & v)
 
 
 void EvalState::addPrimOp(const string & name,
-    unsigned int arity, PrimOp primOp)
+    unsigned int arity, PrimOpFun primOp)
 {
     Value * v = allocValue();
     string name2 = string(name, 0, 2) == "__" ? string(name, 2) : name;
+    Symbol sym = symbols.create(name);
     v->type = tPrimOp;
-    v->primOp.arity = arity;
-    v->primOp.fun = primOp;
-    v->primOp.name = GC_STRDUP(name2.c_str());
-    staticBaseEnv.vars[symbols.create(name)] = baseEnvDispl;
+    v->primOp = new (UseGC) PrimOp(primOp, arity, sym);
+    staticBaseEnv.vars[sym] = baseEnvDispl;
     baseEnv.values[baseEnvDispl++] = v;
     (*baseEnv.values[0]->attrs)[symbols.create(name2)].value = v;
 }
@@ -252,6 +251,7 @@ void mkString(Value & v, const string & s, const PathSet & context)
 
 void mkPath(Value & v, const char * s)
 {
+    clearValue(v);
     v.type = tPath;
     v.path = GC_STRDUP(s);
 }
@@ -310,6 +310,7 @@ void EvalState::mkList(Value & v, unsigned int length)
 
 void EvalState::mkAttrs(Value & v)
 {
+    clearValue(v);
     v.type = tAttrs;
 #if HAVE_BOEHMGC
     v.attrs = new (UseGC) Bindings;
@@ -595,15 +596,20 @@ void ExprApp::eval(EvalState & state, Env & env, Value & v)
 void EvalState::callFunction(Value & fun, Value & arg, Value & v)
 {
     if (fun.type == tPrimOp || fun.type == tPrimOpApp) {
-        unsigned int argsLeft =
-            fun.type == tPrimOp ? fun.primOp.arity : fun.primOpApp.argsLeft;
+
+        /* Figure out the number of arguments still needed. */
+        unsigned int argsDone = 0;
+        Value * primOp = &fun;
+        while (primOp->type == tPrimOpApp) {
+            argsDone++;
+            primOp = primOp->primOpApp.left;
+        }
+        assert(primOp->type == tPrimOp);
+        unsigned int arity = primOp->primOp->arity;
+        unsigned int argsLeft = arity - argsDone;
+        
         if (argsLeft == 1) {
-            /* We have all the arguments, so call the primop.  First
-               find the primop. */
-            Value * primOp = &fun;
-            while (primOp->type == tPrimOpApp) primOp = primOp->primOpApp.left;
-            assert(primOp->type == tPrimOp);
-            unsigned int arity = primOp->primOp.arity;
+            /* We have all the arguments, so call the primop. */
                 
             /* Put all the arguments in an array. */
             Value * vArgs[arity];
@@ -614,9 +620,9 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v)
 
             /* And call the primop. */
             try {
-                primOp->primOp.fun(*this, vArgs, v);
+                primOp->primOp->fun(*this, vArgs, v);
             } catch (Error & e) {
-                addErrorPrefix(e, "while evaluating the builtin function `%1%':\n", primOp->primOp.name);
+                addErrorPrefix(e, "while evaluating the builtin function `%1%':\n", primOp->primOp->name);
                 throw;
             }
         } else {
@@ -624,7 +630,6 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v)
             v.primOpApp.left = allocValue();
             *v.primOpApp.left = fun;
             v.primOpApp.right = &arg;
-            v.primOpApp.argsLeft = argsLeft - 1;
         }
         return;
     }
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 5f5966930f..ec49394425 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -44,7 +44,17 @@ typedef enum {
 } ValueType;
 
 
-typedef void (* PrimOp) (EvalState & state, Value * * args, Value & v);
+typedef void (* PrimOpFun) (EvalState & state, Value * * args, Value & v);
+
+
+struct PrimOp
+{
+    PrimOpFun fun;
+    unsigned int arity;
+    Symbol name;
+    PrimOp(PrimOpFun fun, unsigned int arity, Symbol name)
+        : fun(fun), arity(arity), name(name) { }
+};
 
 
 struct Value
@@ -97,15 +107,9 @@ struct Value
             Env * env;
             ExprLambda * fun;
         } lambda;
-        Value * val;
-        struct {
-            PrimOp fun;
-            char * name;
-            unsigned int arity;
-        } primOp;
+        PrimOp * primOp;
         struct {
             Value * left, * right;
-            unsigned int argsLeft;
         } primOpApp;
     };
 };
@@ -127,8 +131,17 @@ struct Attr
 };
 
 
+/* After overwriting an app node, be sure to clear pointers in the
+   Value to ensure that the target isn't kept alive unnecessarily. */
+static inline void clearValue(Value & v)
+{
+    v.app.right = 0;
+}
+
+
 static inline void mkInt(Value & v, int n)
 {
+    clearValue(v);
     v.type = tInt;
     v.integer = n;
 }
@@ -136,6 +149,7 @@ static inline void mkInt(Value & v, int n)
 
 static inline void mkBool(Value & v, bool b)
 {
+    clearValue(v);
     v.type = tBool;
     v.boolean = b;
 }
@@ -268,7 +282,7 @@ private:
     void addConstant(const string & name, Value & v);
 
     void addPrimOp(const string & name,
-        unsigned int arity, PrimOp primOp);
+        unsigned int arity, PrimOpFun primOp);
 
     Value * lookupVar(Env * env, const VarRef & var);