From b0c11cda7ef9eaef46f9868e3f3736aa72fa641c Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Wed, 20 Oct 2010 09:08:39 +0000
Subject: [PATCH 01/23] * Evaluator garbage collection branch.


From e879a0371ba7ef99da2d82547823c3f96b445fdb Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Wed, 20 Oct 2010 11:38:30 +0000
Subject: [PATCH 02/23] * Use the Boehm garbage collector to reclaim unused
 memory in the Nix   expression evaluator.

---
 src/libexpr/Makefile.am         |  3 ++-
 src/libexpr/eval.cc             | 23 ++++++++++++++---------
 src/libexpr/eval.hh             |  5 ++++-
 src/nix-env/Makefile.am         |  6 ++++--
 src/nix-instantiate/Makefile.am |  7 +++++--
 5 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/src/libexpr/Makefile.am b/src/libexpr/Makefile.am
index e7228e183..24aa2968b 100644
--- a/src/libexpr/Makefile.am
+++ b/src/libexpr/Makefile.am
@@ -20,7 +20,8 @@ EXTRA_DIST = lexer.l parser.y
 
 AM_CXXFLAGS = \
  -I$(srcdir)/.. \
- -I$(srcdir)/../libutil -I$(srcdir)/../libstore
+ -I$(srcdir)/../libutil -I$(srcdir)/../libstore \
+ -I/home/eelco/Dev/nix/boehmgc/include
 
 
 # Parser generation.
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index e55f28822..ba331749f 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -8,6 +8,9 @@
 
 #include <cstring>
 
+#include <gc/gc.h>
+#include <gc/gc_cpp.h>
+
 
 #define LocalNoInline(f) static f __attribute__((noinline)); f
 #define LocalNoInlineNoReturn(f) static f __attribute__((noinline, noreturn)); f
@@ -147,7 +150,7 @@ void EvalState::addPrimOp(const string & name,
     v.type = tPrimOp;
     v.primOp.arity = arity;
     v.primOp.fun = primOp;
-    v.primOp.name = strdup(name2.c_str());
+    v.primOp.name = GC_strdup(name2.c_str());
     staticBaseEnv.vars[symbols.create(name)] = baseEnvDispl;
     baseEnv.values[baseEnvDispl++] = v;
     (*baseEnv.values[0].attrs)[symbols.create(name2)].value = v;
@@ -218,7 +221,7 @@ LocalNoInline(void addErrorPrefix(Error & e, const char * s, const string & s2,
 void mkString(Value & v, const char * s)
 {
     v.type = tString;
-    v.string.s = strdup(s);
+    v.string.s = GC_strdup(s);
     v.string.context = 0;
 }
 
@@ -228,9 +231,10 @@ void mkString(Value & v, const string & s, const PathSet & context)
     mkString(v, s.c_str());
     if (!context.empty()) {
         unsigned int n = 0;
-        v.string.context = new const char *[context.size() + 1];
+        v.string.context = (const char * *)
+            GC_malloc((context.size() + 1) * sizeof(char *));
         foreach (PathSet::const_iterator, i, context) 
-            v.string.context[n++] = strdup(i->c_str());
+            v.string.context[n++] = GC_strdup(i->c_str());
         v.string.context[n] = 0;
     }
 }
@@ -239,7 +243,7 @@ void mkString(Value & v, const string & s, const PathSet & context)
 void mkPath(Value & v, const char * s)
 {
     v.type = tPath;
-    v.path = strdup(s);
+    v.path = GC_strdup(s);
 }
 
 
@@ -264,7 +268,7 @@ Value * EvalState::lookupVar(Env * env, const VarRef & var)
 Value * EvalState::allocValues(unsigned int count)
 {
     nrValues += count;
-    return new Value[count]; // !!! check destructor
+    return (Value *) GC_MALLOC(count * sizeof(Value));
 }
 
 
@@ -272,7 +276,7 @@ Env & EvalState::allocEnv(unsigned int size)
 {
     nrEnvs++;
     nrValuesInEnvs += size;
-    Env * env = (Env *) malloc(sizeof(Env) + size * sizeof(Value));
+    Env * env = (Env *) GC_MALLOC(sizeof(Env) + size * sizeof(Value));
     return *env;
 }
 
@@ -281,7 +285,7 @@ void EvalState::mkList(Value & v, unsigned int length)
 {
     v.type = tList;
     v.list.length = length;
-    v.list.elems = new Value *[length];
+    v.list.elems = (Value * *) GC_MALLOC(length * sizeof(Value *));
     nrListElems += length;
 }
 
@@ -289,7 +293,7 @@ void EvalState::mkList(Value & v, unsigned int length)
 void EvalState::mkAttrs(Value & v)
 {
     v.type = tAttrs;
-    v.attrs = new Bindings;
+    v.attrs = new (UseGC) Bindings;
 }
 
 
@@ -1089,6 +1093,7 @@ void EvalState::printStats()
     bool showStats = getEnv("NIX_SHOW_STATS", "0") != "0";
     Verbosity v = showStats ? lvlInfo : lvlDebug;
     printMsg(v, "evaluation statistics:");
+    printMsg(v, format("  size of a value: %1%") % sizeof(Value));
     printMsg(v, format("  expressions evaluated: %1%") % nrEvaluated);
     printMsg(v, format("  stack space used: %1% bytes") % (&x - deepestStack));
     printMsg(v, format("  max eval() nesting depth: %1%") % maxRecursionDepth);
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 501ac93d0..d74a03506 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -7,6 +7,8 @@
 
 #include <map>
 
+#include <gc/gc_allocator.h>
+
 
 namespace nix {
 
@@ -16,7 +18,7 @@ struct Env;
 struct Value;
 struct Attr;
 
-typedef std::map<Symbol, Attr> Bindings;
+typedef std::map<Symbol, Attr, std::less<Symbol>, gc_allocator<std::pair<const Symbol, Attr> > > Bindings;
 
 
 typedef enum {
@@ -313,6 +315,7 @@ private:
     char * deepestStack; /* for measuring stack usage */
     
     friend class RecursionCounter;
+    friend class ExprOpUpdate;
 };
 
 
diff --git a/src/nix-env/Makefile.am b/src/nix-env/Makefile.am
index 7dfa7425a..169c85fcb 100644
--- a/src/nix-env/Makefile.am
+++ b/src/nix-env/Makefile.am
@@ -4,7 +4,8 @@ nix_env_SOURCES = nix-env.cc profiles.cc profiles.hh user-env.cc user-env.hh hel
 
 nix_env_LDADD = ../libmain/libmain.la ../libexpr/libexpr.la \
  ../libstore/libstore.la ../libutil/libutil.la \
- ../boost/format/libformat.la @ADDITIONAL_NETWORK_LIBS@
+ ../boost/format/libformat.la @ADDITIONAL_NETWORK_LIBS@ \
+ -L/home/eelco/Dev/nix/boehmgc/lib -lgc
 
 nix-env.o: help.txt.hh
 
@@ -14,4 +15,5 @@ nix-env.o: help.txt.hh
 AM_CXXFLAGS = \
  -I$(srcdir)/.. \
  -I$(srcdir)/../libutil -I$(srcdir)/../libstore \
- -I$(srcdir)/../libexpr -I$(srcdir)/../libmain -I../libexpr
+ -I$(srcdir)/../libexpr -I$(srcdir)/../libmain -I../libexpr \
+ -I/home/eelco/Dev/nix/boehmgc/include
diff --git a/src/nix-instantiate/Makefile.am b/src/nix-instantiate/Makefile.am
index a65907a8d..c6455b116 100644
--- a/src/nix-instantiate/Makefile.am
+++ b/src/nix-instantiate/Makefile.am
@@ -3,7 +3,8 @@ bin_PROGRAMS = nix-instantiate
 nix_instantiate_SOURCES = nix-instantiate.cc help.txt
 nix_instantiate_LDADD = ../libmain/libmain.la ../libexpr/libexpr.la \
  ../libstore/libstore.la ../libutil/libutil.la \
- ../boost/format/libformat.la @ADDITIONAL_NETWORK_LIBS@
+ ../boost/format/libformat.la @ADDITIONAL_NETWORK_LIBS@ \
+ -L/home/eelco/Dev/nix/boehmgc/lib -lgc
 
 nix-instantiate.o: help.txt.hh
 
@@ -12,4 +13,6 @@ nix-instantiate.o: help.txt.hh
 
 AM_CXXFLAGS = \
  -I$(srcdir)/.. -I$(srcdir)/../libutil -I$(srcdir)/../libstore \
- -I$(srcdir)/../libexpr -I$(srcdir)/../libmain -I../libexpr
+ -I$(srcdir)/../libexpr -I$(srcdir)/../libmain -I../libexpr \
+ -I/home/eelco/Dev/nix/boehmgc/include
+

From 76feaf016a7e9a9b019148df5ff84a63e48dbda7 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Wed, 20 Oct 2010 15:48:00 +0000
Subject: [PATCH 03/23] * Keep some more stats.

---
 src/libexpr/eval.cc | 9 +++++++++
 src/libexpr/eval.hh | 3 +++
 2 files changed, 12 insertions(+)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index ba331749f..46978f6d0 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -119,6 +119,7 @@ EvalState::EvalState()
 {
     nrEnvs = nrValuesInEnvs = nrValues = nrListElems = 0;
     nrEvaluated = recursionDepth = maxRecursionDepth = 0;
+    nrAttrsets = nrOpUpdates = nrOpUpdateValuesCopied = 0;
     deepestStack = (char *) -1;
 
     createBaseEnv();
@@ -294,6 +295,7 @@ void EvalState::mkAttrs(Value & v)
 {
     v.type = tAttrs;
     v.attrs = new (UseGC) Bindings;
+    nrAttrsets++;
 }
 
 
@@ -758,6 +760,8 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v)
     state.evalAttrs(env, e1, v1);
     state.evalAttrs(env, e2, v2);
 
+    state.nrOpUpdates++;
+
     if (v1.attrs->size() == 0) { v = v2; return; }
     if (v2.attrs->size() == 0) { v = v1; return; }
 
@@ -768,6 +772,8 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v)
         mkCopy(a.value, i->second.value);
         a.pos = i->second.pos;
     }
+
+    state.nrOpUpdateValuesCopied += v.attrs->size();
 }
 
 
@@ -1107,6 +1113,9 @@ void EvalState::printStats()
         % nrListElems % (nrListElems * sizeof(Value *)));
     printMsg(v, format("  misc. values allocated: %1% (%2% bytes)")
         % nrValues % (nrValues * sizeof(Value)));
+    printMsg(v, format("  attribute sets allocated: %1%") % nrAttrsets);
+    printMsg(v, format("  right-biased unions: %1%") % nrOpUpdates);
+    printMsg(v, format("  values copied in right-biased unions: %1%") % nrOpUpdateValuesCopied);
     printMsg(v, format("  symbols in symbol table: %1%") % symbols.size());
 }
 
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index d74a03506..9144d99a4 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -310,6 +310,9 @@ private:
     unsigned long nrValues;
     unsigned long nrListElems;
     unsigned long nrEvaluated;
+    unsigned long nrAttrsets;
+    unsigned long nrOpUpdates;
+    unsigned long nrOpUpdateValuesCopied;
     unsigned int recursionDepth;
     unsigned int maxRecursionDepth;
     char * deepestStack; /* for measuring stack usage */

From 64c3325b0bef8c0234bf797033e129323b36ad1e Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Fri, 22 Oct 2010 13:39:15 +0000
Subject: [PATCH 04/23] * Make building against the Boehm GC a configure
 option.

---
 configure.ac                    | 13 +++++++++++++
 src/libexpr/Makefile.am         |  3 +--
 src/libexpr/eval.cc             | 23 ++++++++++++++++++-----
 src/libexpr/eval.hh             |  6 ++++++
 src/nix-env/Makefile.am         |  5 ++---
 src/nix-instantiate/Makefile.am |  6 ++----
 6 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/configure.ac b/configure.ac
index 1caa9be5c..02eebf692 100644
--- a/configure.ac
+++ b/configure.ac
@@ -250,6 +250,19 @@ AC_SUBST(bzip2_bin)
 AC_SUBST(bzip2_bin_test)
 
 
+# Whether to use the Boehm garbage collector.
+AC_ARG_WITH(boehm-gc, AC_HELP_STRING([--with-boehm-gc=PATH],
+  [prefix of the Boehm GC package to enable garbage collection in the Nix expression evaluator]),
+  boehmgc=$withval, boehmgc=)
+if test -n "$boehmgc"; then
+  boehmgc_lib="-L$boehmgc/lib -lgc"
+  CXXFLAGS="-I$boehmgc/include $CXXFLAGS"
+  AC_DEFINE(HAVE_BOEHMGC, 1, [Whether to use the Boehm garbage collector.])
+fi
+AC_SUBST(boehmgc_lib)
+AC_SUBST(boehmgc_include)
+  
+
 AC_ARG_ENABLE(init-state, AC_HELP_STRING([--disable-init-state],
   [do not initialise DB etc. in `make install']),
   init_state=$enableval, init_state=yes)
diff --git a/src/libexpr/Makefile.am b/src/libexpr/Makefile.am
index 24aa2968b..e7228e183 100644
--- a/src/libexpr/Makefile.am
+++ b/src/libexpr/Makefile.am
@@ -20,8 +20,7 @@ EXTRA_DIST = lexer.l parser.y
 
 AM_CXXFLAGS = \
  -I$(srcdir)/.. \
- -I$(srcdir)/../libutil -I$(srcdir)/../libstore \
- -I/home/eelco/Dev/nix/boehmgc/include
+ -I$(srcdir)/../libutil -I$(srcdir)/../libstore
 
 
 # Parser generation.
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 46978f6d0..c8a031ac6 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -8,9 +8,18 @@
 
 #include <cstring>
 
+#if HAVE_BOEHMGC
+
 #include <gc/gc.h>
 #include <gc/gc_cpp.h>
 
+#else
+
+#define GC_STRDUP strdup
+#define GC_MALLOC malloc
+
+#endif
+
 
 #define LocalNoInline(f) static f __attribute__((noinline)); f
 #define LocalNoInlineNoReturn(f) static f __attribute__((noinline, noreturn)); f
@@ -151,7 +160,7 @@ void EvalState::addPrimOp(const string & name,
     v.type = tPrimOp;
     v.primOp.arity = arity;
     v.primOp.fun = primOp;
-    v.primOp.name = GC_strdup(name2.c_str());
+    v.primOp.name = GC_STRDUP(name2.c_str());
     staticBaseEnv.vars[symbols.create(name)] = baseEnvDispl;
     baseEnv.values[baseEnvDispl++] = v;
     (*baseEnv.values[0].attrs)[symbols.create(name2)].value = v;
@@ -222,7 +231,7 @@ LocalNoInline(void addErrorPrefix(Error & e, const char * s, const string & s2,
 void mkString(Value & v, const char * s)
 {
     v.type = tString;
-    v.string.s = GC_strdup(s);
+    v.string.s = GC_STRDUP(s);
     v.string.context = 0;
 }
 
@@ -233,9 +242,9 @@ void mkString(Value & v, const string & s, const PathSet & context)
     if (!context.empty()) {
         unsigned int n = 0;
         v.string.context = (const char * *)
-            GC_malloc((context.size() + 1) * sizeof(char *));
+            GC_MALLOC((context.size() + 1) * sizeof(char *));
         foreach (PathSet::const_iterator, i, context) 
-            v.string.context[n++] = GC_strdup(i->c_str());
+            v.string.context[n++] = GC_STRDUP(i->c_str());
         v.string.context[n] = 0;
     }
 }
@@ -244,7 +253,7 @@ void mkString(Value & v, const string & s, const PathSet & context)
 void mkPath(Value & v, const char * s)
 {
     v.type = tPath;
-    v.path = GC_strdup(s);
+    v.path = GC_STRDUP(s);
 }
 
 
@@ -294,7 +303,11 @@ void EvalState::mkList(Value & v, unsigned int length)
 void EvalState::mkAttrs(Value & v)
 {
     v.type = tAttrs;
+#if HAVE_BOEHMGC
     v.attrs = new (UseGC) Bindings;
+#else
+    v.attrs = new Bindings;
+#endif
     nrAttrsets++;
 }
 
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 9144d99a4..738ca9439 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -7,7 +7,9 @@
 
 #include <map>
 
+#if HAVE_BOEHMGC
 #include <gc/gc_allocator.h>
+#endif
 
 
 namespace nix {
@@ -18,7 +20,11 @@ struct Env;
 struct Value;
 struct Attr;
 
+#if HAVE_BOEHMGC
 typedef std::map<Symbol, Attr, std::less<Symbol>, gc_allocator<std::pair<const Symbol, Attr> > > Bindings;
+#else
+typedef std::map<Symbol, Attr> Bindings;
+#endif
 
 
 typedef enum {
diff --git a/src/nix-env/Makefile.am b/src/nix-env/Makefile.am
index 169c85fcb..a876b3eb3 100644
--- a/src/nix-env/Makefile.am
+++ b/src/nix-env/Makefile.am
@@ -5,7 +5,7 @@ nix_env_SOURCES = nix-env.cc profiles.cc profiles.hh user-env.cc user-env.hh hel
 nix_env_LDADD = ../libmain/libmain.la ../libexpr/libexpr.la \
  ../libstore/libstore.la ../libutil/libutil.la \
  ../boost/format/libformat.la @ADDITIONAL_NETWORK_LIBS@ \
- -L/home/eelco/Dev/nix/boehmgc/lib -lgc
+ @boehmgc_lib@
 
 nix-env.o: help.txt.hh
 
@@ -15,5 +15,4 @@ nix-env.o: help.txt.hh
 AM_CXXFLAGS = \
  -I$(srcdir)/.. \
  -I$(srcdir)/../libutil -I$(srcdir)/../libstore \
- -I$(srcdir)/../libexpr -I$(srcdir)/../libmain -I../libexpr \
- -I/home/eelco/Dev/nix/boehmgc/include
+ -I$(srcdir)/../libexpr -I$(srcdir)/../libmain -I../libexpr
diff --git a/src/nix-instantiate/Makefile.am b/src/nix-instantiate/Makefile.am
index c6455b116..bc9792818 100644
--- a/src/nix-instantiate/Makefile.am
+++ b/src/nix-instantiate/Makefile.am
@@ -4,7 +4,7 @@ nix_instantiate_SOURCES = nix-instantiate.cc help.txt
 nix_instantiate_LDADD = ../libmain/libmain.la ../libexpr/libexpr.la \
  ../libstore/libstore.la ../libutil/libutil.la \
  ../boost/format/libformat.la @ADDITIONAL_NETWORK_LIBS@ \
- -L/home/eelco/Dev/nix/boehmgc/lib -lgc
+ @boehmgc_lib@
 
 nix-instantiate.o: help.txt.hh
 
@@ -13,6 +13,4 @@ nix-instantiate.o: help.txt.hh
 
 AM_CXXFLAGS = \
  -I$(srcdir)/.. -I$(srcdir)/../libutil -I$(srcdir)/../libstore \
- -I$(srcdir)/../libexpr -I$(srcdir)/../libmain -I../libexpr \
- -I/home/eelco/Dev/nix/boehmgc/include
-
+ -I$(srcdir)/../libexpr -I$(srcdir)/../libmain -I../libexpr

From 41c45a9b319a5578e2731505ca3de2b9c50b4988 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Fri, 22 Oct 2010 14:47:42 +0000
Subject: [PATCH 05/23] * Store Value nodes outside of attribute sets.  I.e.,
 Attr now stores   a pointer to a Value, rather than the Value directly.  This
 improves   the effectiveness of garbage collection a lot: if the Value is  
 stored inside the set directly, then any live pointer to the Value   causes
 all other attributes in the set to be live as well.

---
 src/libexpr/Makefile.am                |  2 +-
 src/libexpr/attr-path.cc               |  5 +-
 src/libexpr/attr-path.hh               |  2 +-
 src/libexpr/common-opts.cc             |  7 +-
 src/libexpr/eval.cc                    | 97 +++++++++++++++-----------
 src/libexpr/eval.hh                    |  7 +-
 src/libexpr/get-drvs.cc                | 40 +++++------
 src/libexpr/get-drvs.hh                |  2 +-
 src/libexpr/nixexpr.cc                 |  4 +-
 src/libexpr/nixexpr.hh                 |  1 +
 src/libexpr/primops.cc                 | 70 +++++++++----------
 src/libexpr/value-to-xml.cc            | 14 ++--
 src/nix-env/nix-env.cc                 |  4 +-
 src/nix-env/user-env.cc                | 23 +++---
 src/nix-instantiate/nix-instantiate.cc |  2 +-
 15 files changed, 150 insertions(+), 130 deletions(-)

diff --git a/src/libexpr/Makefile.am b/src/libexpr/Makefile.am
index e7228e183..6c38ecdd5 100644
--- a/src/libexpr/Makefile.am
+++ b/src/libexpr/Makefile.am
@@ -11,7 +11,7 @@ pkginclude_HEADERS = \
  names.hh symbol-table.hh
 
 libexpr_la_LIBADD = ../libutil/libutil.la ../libstore/libstore.la \
- ../boost/format/libformat.la
+ ../boost/format/libformat.la @boehmgc_lib@
 
 BUILT_SOURCES = \
  parser-tab.hh lexer-tab.hh parser-tab.cc lexer-tab.cc
diff --git a/src/libexpr/attr-path.cc b/src/libexpr/attr-path.cc
index 0660ba1c1..365b03c0b 100644
--- a/src/libexpr/attr-path.cc
+++ b/src/libexpr/attr-path.cc
@@ -5,8 +5,9 @@
 namespace nix {
 
 
+// !!! Shouldn't we return a pointer to a Value?
 void findAlongAttrPath(EvalState & state, const string & attrPath,
-    const Bindings & autoArgs, Expr * e, Value & v)
+    Bindings & autoArgs, Expr * e, Value & v)
 {
     Strings tokens = tokenizeString(attrPath, ".");
 
@@ -48,7 +49,7 @@ void findAlongAttrPath(EvalState & state, const string & attrPath,
             Bindings::iterator a = v.attrs->find(state.symbols.create(attr));
             if (a == v.attrs->end())
                 throw Error(format("attribute `%1%' in selection path `%2%' not found") % attr % curPath);
-            v = a->second.value;
+            v = *a->second.value;
         }
 
         else if (apType == apIndex) {
diff --git a/src/libexpr/attr-path.hh b/src/libexpr/attr-path.hh
index b4f5c29d2..b106da5ef 100644
--- a/src/libexpr/attr-path.hh
+++ b/src/libexpr/attr-path.hh
@@ -11,7 +11,7 @@ namespace nix {
 
     
 void findAlongAttrPath(EvalState & state, const string & attrPath,
-    const Bindings & autoArgs, Expr * e, Value & v);
+    Bindings & autoArgs, Expr * e, Value & v);
 
     
 }
diff --git a/src/libexpr/common-opts.cc b/src/libexpr/common-opts.cc
index 9131951e3..6b393c07d 100644
--- a/src/libexpr/common-opts.cc
+++ b/src/libexpr/common-opts.cc
@@ -20,12 +20,13 @@ bool parseOptionArg(const string & arg, Strings::iterator & i,
     if (i == argsEnd) throw error;
     string value = *i++;
 
-    Value & v(autoArgs[state.symbols.create(name)].value);
+    Value * v = state.allocValue();
+    autoArgs[state.symbols.create(name)].value = v;
 
     if (arg == "--arg")
-        state.mkThunk_(v, parseExprFromString(state, value, absPath(".")));
+        state.mkThunk_(*v, parseExprFromString(state, value, absPath(".")));
     else
-        mkString(v, value);
+        mkString(*v, value);
     
     return true;
 }
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index c8a031ac6..ea4894725 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -58,7 +58,7 @@ std::ostream & operator << (std::ostream & str, const Value & v)
         typedef std::map<string, Value *> Sorted;
         Sorted sorted;
         foreach (Bindings::iterator, i, *v.attrs)
-            sorted[i->first] = &i->second.value;
+            sorted[i->first] = i->second.value;
         foreach (Sorted::iterator, i, sorted)
             str << i->first << " = " << *i->second << "; ";
         str << "}";
@@ -145,24 +145,26 @@ EvalState::~EvalState()
 
 void EvalState::addConstant(const string & name, Value & v)
 {
+    Value * v2 = allocValue();
+    *v2 = v;
     staticBaseEnv.vars[symbols.create(name)] = baseEnvDispl;
     baseEnv.values[baseEnvDispl++] = v;
     string name2 = string(name, 0, 2) == "__" ? string(name, 2) : name;
-    (*baseEnv.values[0].attrs)[symbols.create(name2)].value = v;
+    (*baseEnv.values[0].attrs)[symbols.create(name2)].value = v2;
 }
 
 
 void EvalState::addPrimOp(const string & name,
     unsigned int arity, PrimOp primOp)
 {
-    Value v;
+    Value * v = allocValue();
     string name2 = string(name, 0, 2) == "__" ? string(name, 2) : name;
-    v.type = tPrimOp;
-    v.primOp.arity = arity;
-    v.primOp.fun = primOp;
-    v.primOp.name = GC_STRDUP(name2.c_str());
+    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;
-    baseEnv.values[baseEnvDispl++] = v;
+    baseEnv.values[baseEnvDispl++] = *v;
     (*baseEnv.values[0].attrs)[symbols.create(name2)].value = v;
 }
 
@@ -265,7 +267,7 @@ Value * EvalState::lookupVar(Env * env, const VarRef & var)
         while (1) {
             Bindings::iterator j = env->values[0].attrs->find(var.name);
             if (j != env->values[0].attrs->end())
-                return &j->second.value;
+                return j->second.value;
             if (env->prevWith == 0)
                 throwEvalError("undefined variable `%1%'", var.name);
             for (unsigned int l = env->prevWith; l; --l, env = env->up) ;
@@ -275,6 +277,13 @@ Value * EvalState::lookupVar(Env * env, const VarRef & var)
 }
 
 
+Value * EvalState::allocValue()
+{
+    nrValues++;
+    return (Value *) GC_MALLOC(sizeof(Value));
+}
+
+
 Value * EvalState::allocValues(unsigned int count)
 {
     nrValues += count;
@@ -291,6 +300,14 @@ Env & EvalState::allocEnv(unsigned int size)
 }
 
 
+Value * EvalState::allocAttr(Value & vAttrs, const Symbol & name)
+{
+    Attr & a = (*vAttrs.attrs)[name];
+    a.value = allocValue();
+    return a.value;
+}
+
+    
 void EvalState::mkList(Value & v, unsigned int length)
 {
     v.type = tList;
@@ -321,11 +338,8 @@ void EvalState::mkThunk_(Value & v, Expr * expr)
 void EvalState::cloneAttrs(Value & src, Value & dst)
 {
     mkAttrs(dst);
-    foreach (Bindings::iterator, i, *src.attrs) {
-        Attr & a = (*dst.attrs)[i->first];
-        mkCopy(a.value, i->second.value);
-        a.pos = i->second.pos;
-    }
+    foreach (Bindings::iterator, i, *src.attrs)
+        (*dst.attrs)[i->first] = i->second;
 }
 
 
@@ -449,8 +463,9 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
            environment. */
         foreach (Attrs::iterator, i, attrs) {
             nix::Attr & a = (*v.attrs)[i->first];
-            mkThunk(a.value, env2, i->second.first);
-            mkCopy(env2.values[displ++], a.value);
+            a.value = state.allocValue();
+            mkThunk(*a.value, env2, i->second.first);
+            mkCopy(env2.values[displ++], *a.value);
             a.pos = &i->second.second;
         }
 
@@ -459,7 +474,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
         foreach (list<Inherited>::iterator, i, inherited) {
             nix::Attr & a = (*v.attrs)[i->first.name];
             Value * v2 = state.lookupVar(&env, i->first);
-            mkCopy(a.value, *v2);
+            a.value = v2;
             mkCopy(env2.values[displ++], *v2);
             a.pos = &i->second;
         }
@@ -474,10 +489,12 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
            Hence we need __overrides.) */
         Bindings::iterator overrides = v.attrs->find(state.sOverrides);
         if (overrides != v.attrs->end()) {
-            state.forceAttrs(overrides->second.value);
-            foreach (Bindings::iterator, i, *overrides->second.value.attrs) {
+            state.forceAttrs(*overrides->second.value);
+            foreach (Bindings::iterator, i, *overrides->second.value->attrs) {
                 nix::Attr & a = (*v.attrs)[i->first];
-                mkCopy(a.value, i->second.value);
+                if (a.value)
+                    mkCopy(env2.values[displs[i->first]], *i->second.value);
+                a = i->second;
             }
         }
     }
@@ -485,13 +502,14 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
     else {
         foreach (Attrs::iterator, i, attrs) {
             nix::Attr & a = (*v.attrs)[i->first];
-            mkThunk(a.value, env, i->second.first);
+            a.value = state.allocValue();
+            mkThunk(*a.value, env, i->second.first);
             a.pos = &i->second.second;
         }
 
         foreach (list<Inherited>::iterator, i, inherited) {
             nix::Attr & a = (*v.attrs)[i->first.name];
-            mkCopy(a.value, *state.lookupVar(&env, i->first));
+            a.value = state.lookupVar(&env, i->first);
             a.pos = &i->second;
         }
     }
@@ -548,13 +566,13 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v)
     if (i == v2.attrs->end())
         throwEvalError("attribute `%1%' missing", name);
     try {            
-        state.forceValue(i->second.value);
+        state.forceValue(*i->second.value);
     } catch (Error & e) {
         addErrorPrefix(e, "while evaluating the attribute `%1%' at %2%:\n",
             name, *i->second.pos);
         throw;
     }
-    v = i->second.value;
+    v = *i->second.value;
 }
 
 
@@ -578,9 +596,9 @@ void ExprApp::eval(EvalState & state, Env & env, Value & v)
 {
     Value vFun;
     state.eval(env, e1, vFun);
-    Value vArg;
-    mkThunk(vArg, env, e2); // !!! should this be on the heap?
-    state.callFunction(vFun, vArg, v);
+    Value * vArg = state.allocValue();
+    mkThunk(*vArg, env, e2);
+    state.callFunction(vFun, *vArg, v);
 }
 
 
@@ -656,7 +674,7 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v)
                 mkThunk(env2.values[displ++], env2, i->def);
             } else {
                 attrsUsed++;
-                mkCopy(env2.values[displ++], j->second.value);
+                mkCopy(env2.values[displ++], *j->second.value);
             }
         }
 
@@ -677,7 +695,7 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v)
 }
 
 
-void EvalState::autoCallFunction(const Bindings & args, Value & fun, Value & res)
+void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res)
 {
     forceValue(fun);
 
@@ -690,7 +708,7 @@ void EvalState::autoCallFunction(const Bindings & args, Value & fun, Value & res
     mkAttrs(actualArgs);
 
     foreach (Formals::Formals_::iterator, i, fun.lambda.fun->formals->formals) {
-        Bindings::const_iterator j = args.find(i->name);
+        Bindings::iterator j = args.find(i->name);
         if (j != args.end())
             (*actualArgs.attrs)[i->name] = j->second;
         else if (!i->def)
@@ -780,11 +798,8 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v)
 
     state.cloneAttrs(v1, v);
 
-    foreach (Bindings::iterator, i, *v2.attrs) {
-        Attr & a = (*v.attrs)[i->first];
-        mkCopy(a.value, i->second.value);
-        a.pos = i->second.pos;
-    }
+    foreach (Bindings::iterator, i, *v2.attrs)
+        (*v.attrs)[i->first] = i->second;
 
     state.nrOpUpdateValuesCopied += v.attrs->size();
 }
@@ -866,7 +881,7 @@ void EvalState::strictForceValue(Value & v)
     
     if (v.type == tAttrs) {
         foreach (Bindings::iterator, i, *v.attrs)
-            strictForceValue(i->second.value);
+            strictForceValue(*i->second.value);
     }
     
     else if (v.type == tList) {
@@ -957,7 +972,7 @@ bool EvalState::isDerivation(Value & v)
 {
     if (v.type != tAttrs) return false;
     Bindings::iterator i = v.attrs->find(sType);
-    return i != v.attrs->end() && forceStringNoCtx(i->second.value) == "derivation";
+    return i != v.attrs->end() && forceStringNoCtx(*i->second.value) == "derivation";
 }
 
 
@@ -1001,7 +1016,7 @@ string EvalState::coerceToString(Value & v, PathSet & context,
         Bindings::iterator i = v.attrs->find(sOutPath);
         if (i == v.attrs->end())
             throwTypeError("cannot coerce an attribute set (except a derivation) to a string");
-        return coerceToString(i->second.value, context, coerceMore, copyToStore);
+        return coerceToString(*i->second.value, context, coerceMore, copyToStore);
     }
 
     if (coerceMore) {
@@ -1087,9 +1102,9 @@ bool EvalState::eqValues(Value & v1, Value & v2)
 
         case tAttrs: {
             if (v1.attrs->size() != v2.attrs->size()) return false;
-            Bindings::iterator i, j;
-            for (i = v1.attrs->begin(), j = v2.attrs->begin(); i != v1.attrs->end(); ++i, ++j)
-                if (i->first != j->first || !eqValues(i->second.value, j->second.value))
+            Bindings::iterator i = v1.attrs->begin(), j = v2.attrs->begin();
+            for ( ; i != v1.attrs->end(); ++i, ++j)
+                if (i->first != j->first || !eqValues(*i->second.value, *j->second.value))
                     return false;
             return true;
         }
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 738ca9439..ee7db91a0 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -122,7 +122,7 @@ struct Env
 
 struct Attr
 {
-    Value value;
+    Value * value;
     Pos * pos;
     Attr() : pos(&noPos) { };
 };
@@ -294,12 +294,15 @@ public:
 
     /* Automatically call a function for which each argument has a
        default value or has a binding in the `args' map. */
-    void autoCallFunction(const Bindings & args, Value & fun, Value & res);
+    void autoCallFunction(Bindings & args, Value & fun, Value & res);
     
     /* Allocation primitives. */
+    Value * allocValue();
     Value * allocValues(unsigned int count);
     Env & allocEnv(unsigned int size);
 
+    Value * allocAttr(Value & vAttrs, const Symbol & name);
+
     void mkList(Value & v, unsigned int length);
     void mkAttrs(Value & v);
     void mkThunk_(Value & v, Expr * expr);
diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc
index 82a92416b..63106b87b 100644
--- a/src/libexpr/get-drvs.cc
+++ b/src/libexpr/get-drvs.cc
@@ -10,7 +10,7 @@ string DrvInfo::queryDrvPath(EvalState & state) const
     if (drvPath == "" && attrs) {
         Bindings::iterator i = attrs->find(state.sDrvPath);
         PathSet context;
-        (string &) drvPath = i != attrs->end() ? state.coerceToPath(i->second.value, context) : "";
+        (string &) drvPath = i != attrs->end() ? state.coerceToPath(*i->second.value, context) : "";
     }
     return drvPath;
 }
@@ -21,7 +21,7 @@ string DrvInfo::queryOutPath(EvalState & state) const
     if (outPath == "" && attrs) {
         Bindings::iterator i = attrs->find(state.sOutPath);
         PathSet context;
-        (string &) outPath = i != attrs->end() ? state.coerceToPath(i->second.value, context) : "";
+        (string &) outPath = i != attrs->end() ? state.coerceToPath(*i->second.value, context) : "";
     }
     return outPath;
 }
@@ -36,21 +36,21 @@ MetaInfo DrvInfo::queryMetaInfo(EvalState & state) const
     Bindings::iterator a = attrs->find(state.sMeta);
     if (a == attrs->end()) return meta; /* fine, empty meta information */
 
-    state.forceAttrs(a->second.value);
+    state.forceAttrs(*a->second.value);
 
-    foreach (Bindings::iterator, i, *a->second.value.attrs) {
+    foreach (Bindings::iterator, i, *a->second.value->attrs) {
         MetaValue value;
-        state.forceValue(i->second.value);
-        if (i->second.value.type == tString) {
+        state.forceValue(*i->second.value);
+        if (i->second.value->type == tString) {
             value.type = MetaValue::tpString;
-            value.stringValue = i->second.value.string.s;
-        } else if (i->second.value.type == tInt) {
+            value.stringValue = i->second.value->string.s;
+        } else if (i->second.value->type == tInt) {
             value.type = MetaValue::tpInt;
-            value.intValue = i->second.value.integer;
-        } else if (i->second.value.type == tList) {
+            value.intValue = i->second.value->integer;
+        } else if (i->second.value->type == tList) {
             value.type = MetaValue::tpStrings;
-            for (unsigned int j = 0; j < i->second.value.list.length; ++j)
-                value.stringValues.push_back(state.forceStringNoCtx(*i->second.value.list.elems[j]));
+            for (unsigned int j = 0; j < i->second.value->list.length; ++j)
+                value.stringValues.push_back(state.forceStringNoCtx(*i->second.value->list.elems[j]));
         } else continue;
         ((MetaInfo &) meta)[i->first] = value;
     }
@@ -99,13 +99,13 @@ static bool getDerivation(EvalState & state, Value & v,
         Bindings::iterator i = v.attrs->find(state.sName);
         /* !!! We really would like to have a decent back trace here. */
         if (i == v.attrs->end()) throw TypeError("derivation name missing");
-        drv.name = state.forceStringNoCtx(i->second.value);
+        drv.name = state.forceStringNoCtx(*i->second.value);
 
-        i = v.attrs->find(state.sSystem);
-        if (i == v.attrs->end())
+        Bindings::iterator i2 = v.attrs->find(state.sSystem);
+        if (i2 == v.attrs->end())
             drv.system = "unknown";
         else
-            drv.system = state.forceStringNoCtx(i->second.value);
+            drv.system = state.forceStringNoCtx(*i2->second.value);
 
         drv.attrs = v.attrs;
 
@@ -138,7 +138,7 @@ static string addToPath(const string & s1, const string & s2)
 
 
 static void getDerivations(EvalState & state, Value & vIn,
-    const string & pathPrefix, const Bindings & autoArgs,
+    const string & pathPrefix, Bindings & autoArgs,
     DrvInfos & drvs, Done & done)
 {
     Value v;
@@ -168,7 +168,7 @@ static void getDerivations(EvalState & state, Value & vIn,
         foreach (SortedSymbols::iterator, i, attrs) {
             startNest(nest, lvlDebug, format("evaluating attribute `%1%'") % i->first);
             string pathPrefix2 = addToPath(pathPrefix, i->first);
-            Value & v2((*v.attrs)[i->second].value);
+            Value & v2(*(*v.attrs)[i->second].value);
             if (combineChannels)
                 getDerivations(state, v2, pathPrefix2, autoArgs, drvs, done);
             else if (getDerivation(state, v2, pathPrefix2, drvs, done)) {
@@ -178,7 +178,7 @@ static void getDerivations(EvalState & state, Value & vIn,
                    attribute. */
                 if (v2.type == tAttrs) {
                     Bindings::iterator j = v2.attrs->find(state.symbols.create("recurseForDerivations"));
-                    if (j != v2.attrs->end() && state.forceBool(j->second.value))
+                    if (j != v2.attrs->end() && state.forceBool(*j->second.value))
                         getDerivations(state, v2, pathPrefix2, autoArgs, drvs, done);
                 }
             }
@@ -200,7 +200,7 @@ static void getDerivations(EvalState & state, Value & vIn,
 
 
 void getDerivations(EvalState & state, Value & v, const string & pathPrefix,
-    const Bindings & autoArgs, DrvInfos & drvs)
+    Bindings & autoArgs, DrvInfos & drvs)
 {
     Done done;
     getDerivations(state, v, pathPrefix, autoArgs, drvs, done);
diff --git a/src/libexpr/get-drvs.hh b/src/libexpr/get-drvs.hh
index 7b96decf4..7c014b7e4 100644
--- a/src/libexpr/get-drvs.hh
+++ b/src/libexpr/get-drvs.hh
@@ -70,7 +70,7 @@ typedef list<DrvInfo> DrvInfos;
 bool getDerivation(EvalState & state, Value & v, DrvInfo & drv);
 
 void getDerivations(EvalState & state, Value & v, const string & pathPrefix,
-    const Bindings & autoArgs, DrvInfos & drvs);
+    Bindings & autoArgs, DrvInfos & drvs);
 
  
 }
diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc
index 898fdb609..9f2ea7883 100644
--- a/src/libexpr/nixexpr.cc
+++ b/src/libexpr/nixexpr.cc
@@ -213,10 +213,10 @@ void ExprAttrs::bindVars(const StaticEnv & env)
         unsigned int displ = 0;
 
         foreach (ExprAttrs::Attrs::iterator, i, attrs)
-            newEnv.vars[i->first] = displ++;
+            displs[i->first] = newEnv.vars[i->first] = displ++;
 
         foreach (list<Inherited>::iterator, i, inherited) {
-            newEnv.vars[i->first.name] = displ++;
+            displs[i->first.name] = newEnv.vars[i->first.name] = displ++;
             i->first.bind(env);
         }
 
diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh
index 1d03220f6..5c0071dca 100644
--- a/src/libexpr/nixexpr.hh
+++ b/src/libexpr/nixexpr.hh
@@ -137,6 +137,7 @@ struct ExprAttrs : Expr
     Attrs attrs;
     list<Inherited> inherited;
     std::map<Symbol, Pos> attrNames; // used during parsing
+    std::map<Symbol, unsigned int> displs;
     ExprAttrs() : recursive(false) { };
     COMMON_METHODS
 };
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 973670e68..c7709ca9e 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -119,24 +119,24 @@ static void prim_genericClosure(EvalState & state, Value * * args, Value & v)
         args[0]->attrs->find(state.symbols.create("startSet"));
     if (startSet == args[0]->attrs->end())
         throw EvalError("attribute `startSet' required");
-    state.forceList(startSet->second.value);
+    state.forceList(*startSet->second.value);
 
     list<Value *> workSet;
-    for (unsigned int n = 0; n < startSet->second.value.list.length; ++n)
-        workSet.push_back(startSet->second.value.list.elems[n]);
+    for (unsigned int n = 0; n < startSet->second.value->list.length; ++n)
+        workSet.push_back(startSet->second.value->list.elems[n]);
 
     /* Get the operator. */
     Bindings::iterator op =
         args[0]->attrs->find(state.symbols.create("operator"));
     if (op == args[0]->attrs->end())
         throw EvalError("attribute `operator' required");
-    state.forceValue(op->second.value);
+    state.forceValue(*op->second.value);
 
     /* Construct the closure by applying the operator to element of
        `workSet', adding the result to `workSet', continuing until
        no new elements are found. */
     list<Value> res;
-    set<Value, CompareValues> doneKeys;
+    set<Value, CompareValues> doneKeys; // !!! use Value *?
     while (!workSet.empty()) {
 	Value * e = *(workSet.begin());
 	workSet.pop_front();
@@ -147,15 +147,15 @@ static void prim_genericClosure(EvalState & state, Value * * args, Value & v)
             e->attrs->find(state.symbols.create("key"));
         if (key == e->attrs->end())
             throw EvalError("attribute `key' required");
-        state.forceValue(key->second.value);
+        state.forceValue(*key->second.value);
 
-        if (doneKeys.find(key->second.value) != doneKeys.end()) continue;
-        doneKeys.insert(key->second.value);
+        if (doneKeys.find(*key->second.value) != doneKeys.end()) continue;
+        doneKeys.insert(*key->second.value);
         res.push_back(*e);
         
         /* Call the `operator' function with `e' as argument. */
         Value call;
-        mkApp(call, op->second.value, *e);
+        mkApp(call, *op->second.value, *e);
         state.forceList(call);
 
         /* Add the values returned by the operator to the work set. */
@@ -213,11 +213,13 @@ static void prim_tryEval(EvalState & state, Value * * args, Value & v)
     state.mkAttrs(v);
     try {
         state.forceValue(*args[0]);
-        (*v.attrs)[state.symbols.create("value")].value = *args[0];
-        mkBool((*v.attrs)[state.symbols.create("success")].value, true);
+        printMsg(lvlError, format("%1%") % *args[0]);
+        (*v.attrs)[state.symbols.create("value")].value = args[0];
+        mkBool(*state.allocAttr(v, state.symbols.create("success")), true);
+        printMsg(lvlError, format("%1%") % v);
     } catch (AssertionError & e) {
-        mkBool((*v.attrs)[state.symbols.create("value")].value, false);
-        mkBool((*v.attrs)[state.symbols.create("success")].value, false);
+        mkBool(*state.allocAttr(v, state.symbols.create("value")), false);
+        mkBool(*state.allocAttr(v, state.symbols.create("success")), false);
     }
 }
 
@@ -326,7 +328,7 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
     string drvName;
     Pos & posDrvName(*attr->second.pos);
     try {        
-        drvName = state.forceStringNoCtx(attr->second.value);
+        drvName = state.forceStringNoCtx(*attr->second.value);
     } catch (Error & e) {
         e.addPrefix(format("while evaluating the derivation attribute `name' at %1%:\n") % posDrvName);
         throw;
@@ -349,9 +351,9 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
             /* The `args' attribute is special: it supplies the
                command-line arguments to the builder. */
             if (key == "args") {
-                state.forceList(i->second.value);
-                for (unsigned int n = 0; n < i->second.value.list.length; ++n) {
-                    string s = state.coerceToString(*i->second.value.list.elems[n], context, true);
+                state.forceList(*i->second.value);
+                for (unsigned int n = 0; n < i->second.value->list.length; ++n) {
+                    string s = state.coerceToString(*i->second.value->list.elems[n], context, true);
                     drv.args.push_back(s);
                 }
             }
@@ -359,7 +361,7 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
             /* All other attributes are passed to the builder through
                the environment. */
             else {
-                string s = state.coerceToString(i->second.value, context, true);
+                string s = state.coerceToString(*i->second.value, context, true);
                 drv.env[key] = s;
                 if (key == "builder") drv.builder = s;
                 else if (i->first == state.sSystem) drv.platform = s;
@@ -488,8 +490,8 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
 
     /* !!! assumes a single output */
     state.mkAttrs(v);
-    mkString((*v.attrs)[state.sOutPath].value, outPath, singleton<PathSet>(drvPath));
-    mkString((*v.attrs)[state.sDrvPath].value, drvPath, singleton<PathSet>("=" + drvPath));
+    mkString(*state.allocAttr(v, state.sOutPath), outPath, singleton<PathSet>(drvPath));
+    mkString(*state.allocAttr(v, state.sDrvPath), drvPath, singleton<PathSet>("=" + drvPath));
 }
 
 
@@ -713,8 +715,8 @@ static void prim_getAttr(EvalState & state, Value * * args, Value & v)
     if (i == args[1]->attrs->end())
         throw EvalError(format("attribute `%1%' missing") % attr);
     // !!! add to stack trace?
-    state.forceValue(i->second.value);
-    v = i->second.value;
+    state.forceValue(*i->second.value);
+    v = *i->second.value;
 }
 
 
@@ -766,15 +768,13 @@ static void prim_listToAttrs(EvalState & state, Value * * args, Value & v)
         Bindings::iterator j = v2.attrs->find(state.sName);
         if (j == v2.attrs->end())
             throw TypeError("`name' attribute missing in a call to `listToAttrs'");
-        string name = state.forceStringNoCtx(j->second.value);
+        string name = state.forceStringNoCtx(*j->second.value);
         
-        j = v2.attrs->find(state.symbols.create("value"));
-        if (j == v2.attrs->end())
+        Bindings::iterator j2 = v2.attrs->find(state.symbols.create("value"));
+        if (j2 == v2.attrs->end())
             throw TypeError("`value' attribute missing in a call to `listToAttrs'");
 
-        Attr & a = (*v.attrs)[state.symbols.create(name)];
-        mkCopy(a.value, j->second.value);
-        a.pos = j->second.pos;
+        (*v.attrs)[state.symbols.create(name)] = j2->second;
     }
 }
 
@@ -791,11 +791,8 @@ static void prim_intersectAttrs(EvalState & state, Value * * args, Value & v)
 
     foreach (Bindings::iterator, i, *args[0]->attrs) {
         Bindings::iterator j = args[1]->attrs->find(i->first);
-        if (j != args[1]->attrs->end()) {
-            Attr & a = (*v.attrs)[j->first];
-            mkCopy(a.value, j->second.value);
-            a.pos = j->second.pos;
-        }
+        if (j != args[1]->attrs->end())
+            (*v.attrs)[j->first] = j->second;
     }
 }
 
@@ -824,7 +821,8 @@ static void prim_functionArgs(EvalState & state, Value * * args, Value & v)
     if (!args[0]->lambda.fun->matchAttrs) return;
 
     foreach (Formals::Formals_::iterator, i, args[0]->lambda.fun->formals->formals)
-        mkBool((*v.attrs)[i->name].value, i->def);
+        // !!! should optimise booleans (allocate only once)
+        mkBool(*state.allocAttr(v, i->name), i->def);
 }
 
 
@@ -1007,8 +1005,8 @@ static void prim_parseDrvName(EvalState & state, Value * * args, Value & v)
     string name = state.forceStringNoCtx(*args[0]);
     DrvName parsed(name);
     state.mkAttrs(v);
-    mkString((*v.attrs)[state.sName].value, parsed.name);
-    mkString((*v.attrs)[state.symbols.create("version")].value, parsed.version);
+    mkString(*state.allocAttr(v, state.sName), parsed.name);
+    mkString(*state.allocAttr(v, state.symbols.create("version")), parsed.version);
 }
 
 
diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc
index 8955a8a33..1bb200fb8 100644
--- a/src/libexpr/value-to-xml.cc
+++ b/src/libexpr/value-to-xml.cc
@@ -45,7 +45,7 @@ static void showAttrs(EvalState & state, bool strict, bool location,
         
         XMLOpenElement _(doc, "attr", xmlAttrs);
         printValueAsXML(state, strict, location,
-            a.value, doc, context, drvsSeen);
+            *a.value, doc, context, drvsSeen);
     }
 }
 
@@ -90,16 +90,16 @@ static void printValueAsXML(EvalState & state, bool strict, bool location,
                 Path drvPath;
                 a = v.attrs->find(state.sDrvPath);
                 if (a != v.attrs->end()) {
-                    if (strict) state.forceValue(a->second.value);
-                    if (a->second.value.type == tString)
-                        xmlAttrs["drvPath"] = drvPath = a->second.value.string.s;
+                    if (strict) state.forceValue(*a->second.value);
+                    if (a->second.value->type == tString)
+                        xmlAttrs["drvPath"] = drvPath = a->second.value->string.s;
                 }
         
                 a = v.attrs->find(state.sOutPath);
                 if (a != v.attrs->end()) {
-                    if (strict) state.forceValue(a->second.value);
-                    if (a->second.value.type == tString)
-                        xmlAttrs["outPath"] = a->second.value.string.s;
+                    if (strict) state.forceValue(*a->second.value);
+                    if (a->second.value->type == tString)
+                        xmlAttrs["outPath"] = a->second.value->string.s;
                 }
 
                 XMLOpenElement _(doc, "derivation", xmlAttrs);
diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc
index df32b9f60..4895c68e9 100644
--- a/src/nix-env/nix-env.cc
+++ b/src/nix-env/nix-env.cc
@@ -161,7 +161,7 @@ static Expr * loadSourceExpr(EvalState & state, const Path & path)
 
 
 static void loadDerivations(EvalState & state, Path nixExprPath,
-    string systemFilter, const Bindings & autoArgs,
+    string systemFilter, Bindings & autoArgs,
     const string & pathPrefix, DrvInfos & elems)
 {
     Value v;
@@ -321,7 +321,7 @@ static bool isPath(const string & s)
 
 
 static void queryInstSources(EvalState & state,
-    const InstallSourceInfo & instSource, const Strings & args,
+    InstallSourceInfo & instSource, const Strings & args,
     DrvInfos & elems, bool newestOnly)
 {
     InstallSourceType type = instSource.type;
diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc
index 0a619f698..07c94fe4d 100644
--- a/src/nix-env/user-env.cc
+++ b/src/nix-env/user-env.cc
@@ -25,7 +25,8 @@ DrvInfos queryInstalled(EvalState & state, const Path & userEnv)
     if (pathExists(manifestFile)) {
         Value v;
         state.eval(parseExprFromFile(state, manifestFile), v);
-        getDerivations(state, v, "", Bindings(), elems);
+        Bindings bindings;
+        getDerivations(state, v, "", bindings, elems);
     } else if (pathExists(oldManifestFile))
         readLegacyManifest(oldManifestFile, elems);
 
@@ -62,19 +63,19 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
         manifest.list.elems[n++] = &v;
         state.mkAttrs(v);
 
-        mkString((*v.attrs)[state.sType].value, "derivation");
-        mkString((*v.attrs)[state.sName].value, i->name);
-        mkString((*v.attrs)[state.sSystem].value, i->system);
-        mkString((*v.attrs)[state.sOutPath].value, i->queryOutPath(state));
+        mkString(*state.allocAttr(v, state.sType), "derivation");
+        mkString(*state.allocAttr(v, state.sName), i->name);
+        mkString(*state.allocAttr(v, state.sSystem), i->system);
+        mkString(*state.allocAttr(v, state.sOutPath), i->queryOutPath(state));
         if (drvPath != "")
-            mkString((*v.attrs)[state.sDrvPath].value, i->queryDrvPath(state));
+            mkString(*state.allocAttr(v, state.sDrvPath), i->queryDrvPath(state));
         
-        state.mkAttrs((*v.attrs)[state.sMeta].value);
+        state.mkAttrs(*state.allocAttr(v, state.sMeta));
         
         MetaInfo meta = i->queryMetaInfo(state);
 
         foreach (MetaInfo::const_iterator, j, meta) {
-            Value & v2((*(*v.attrs)[state.sMeta].value.attrs)[state.symbols.create(j->first)].value);
+            Value & v2(*state.allocAttr(*(*v.attrs)[state.sMeta].value, state.symbols.create(j->first)));
             switch (j->second.type) {
                 case MetaValue::tpInt: mkInt(v2, j->second.intValue); break;
                 case MetaValue::tpString: mkString(v2, j->second.stringValue); break;
@@ -114,10 +115,10 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
        builder with the manifest as argument. */
     Value args, topLevel;
     state.mkAttrs(args);
-    mkString((*args.attrs)[state.sSystem].value, thisSystem);
-    mkString((*args.attrs)[state.symbols.create("manifest")].value,
+    mkString(*state.allocAttr(args, state.sSystem), thisSystem);
+    mkString(*state.allocAttr(args, state.symbols.create("manifest")),
         manifestFile, singleton<PathSet>(manifestFile));
-    (*args.attrs)[state.symbols.create("derivations")].value = manifest;
+    (*args.attrs)[state.symbols.create("derivations")].value = &manifest;
     mkApp(topLevel, envBuilder, args);
         
     /* Evaluate it. */
diff --git a/src/nix-instantiate/nix-instantiate.cc b/src/nix-instantiate/nix-instantiate.cc
index 2925f9c5e..b330f0cf1 100644
--- a/src/nix-instantiate/nix-instantiate.cc
+++ b/src/nix-instantiate/nix-instantiate.cc
@@ -38,7 +38,7 @@ static bool indirectRoot = false;
 
 
 void processExpr(EvalState & state, const Strings & attrPaths,
-    bool parseOnly, bool strict, const Bindings & autoArgs,
+    bool parseOnly, bool strict, Bindings & autoArgs,
     bool evalOnly, bool xmlOutput, bool location, Expr * e)
 {
     if (parseOnly)

From cf7e645a48a31e04428778a8d39924a3da8a30f8 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Fri, 22 Oct 2010 15:15:12 +0000
Subject: [PATCH 06/23] * Regression test for __overrides.

---
 tests/lang/eval-okay-overrides.exp | 1 +
 tests/lang/eval-okay-overrides.nix | 9 +++++++++
 2 files changed, 10 insertions(+)
 create mode 100644 tests/lang/eval-okay-overrides.exp
 create mode 100644 tests/lang/eval-okay-overrides.nix

diff --git a/tests/lang/eval-okay-overrides.exp b/tests/lang/eval-okay-overrides.exp
new file mode 100644
index 000000000..0cfbf0888
--- /dev/null
+++ b/tests/lang/eval-okay-overrides.exp
@@ -0,0 +1 @@
+2
diff --git a/tests/lang/eval-okay-overrides.nix b/tests/lang/eval-okay-overrides.nix
new file mode 100644
index 000000000..358742b36
--- /dev/null
+++ b/tests/lang/eval-okay-overrides.nix
@@ -0,0 +1,9 @@
+let
+
+  overrides = { a = 2; };
+
+in (rec {
+  __overrides = overrides;
+  x = a;
+  a = 1;
+}).x

From 4dee289550d11950d6d17482484061a4792b2eef Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Fri, 22 Oct 2010 15:51:52 +0000
Subject: [PATCH 07/23] * In environments, store pointers to values rather than
 values.  This   improves GC effectiveness a bit more (because a live value
 doesn't   keep other values in the environment plus the parent environments  
 alive), and removes the need for copy nodes.

---
 src/libexpr/eval.cc | 50 ++++++++++++++++++++++-----------------------
 src/libexpr/eval.hh | 10 +--------
 2 files changed, 25 insertions(+), 35 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index ea4894725..3f2371d79 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -72,7 +72,6 @@ std::ostream & operator << (std::ostream & str, const Value & v)
         break;
     case tThunk:
     case tApp:
-    case tCopy:
         str << "<CODE>";
         break;
     case tLambda:
@@ -104,7 +103,6 @@ string showType(const Value & v)
         case tThunk: return "a thunk";
         case tApp: return "a function application";
         case tLambda: return "a function";
-        case tCopy: return "a copy";
         case tBlackhole: return "a black hole";
         case tPrimOp: return "a built-in function";
         case tPrimOpApp: return "a partially applied built-in function";
@@ -148,9 +146,9 @@ void EvalState::addConstant(const string & name, Value & v)
     Value * v2 = allocValue();
     *v2 = v;
     staticBaseEnv.vars[symbols.create(name)] = baseEnvDispl;
-    baseEnv.values[baseEnvDispl++] = v;
+    baseEnv.values[baseEnvDispl++] = v2;
     string name2 = string(name, 0, 2) == "__" ? string(name, 2) : name;
-    (*baseEnv.values[0].attrs)[symbols.create(name2)].value = v2;
+    (*baseEnv.values[0]->attrs)[symbols.create(name2)].value = v2;
 }
 
 
@@ -164,8 +162,8 @@ void EvalState::addPrimOp(const string & name,
     v->primOp.fun = primOp;
     v->primOp.name = GC_STRDUP(name2.c_str());
     staticBaseEnv.vars[symbols.create(name)] = baseEnvDispl;
-    baseEnv.values[baseEnvDispl++] = *v;
-    (*baseEnv.values[0].attrs)[symbols.create(name2)].value = v;
+    baseEnv.values[baseEnvDispl++] = v;
+    (*baseEnv.values[0]->attrs)[symbols.create(name2)].value = v;
 }
 
 
@@ -265,15 +263,15 @@ Value * EvalState::lookupVar(Env * env, const VarRef & var)
     
     if (var.fromWith) {
         while (1) {
-            Bindings::iterator j = env->values[0].attrs->find(var.name);
-            if (j != env->values[0].attrs->end())
+            Bindings::iterator j = env->values[0]->attrs->find(var.name);
+            if (j != env->values[0]->attrs->end())
                 return j->second.value;
             if (env->prevWith == 0)
                 throwEvalError("undefined variable `%1%'", var.name);
             for (unsigned int l = env->prevWith; l; --l, env = env->up) ;
         }
     } else
-        return &env->values[var.displ];
+        return env->values[var.displ];
 }
 
 
@@ -295,7 +293,7 @@ Env & EvalState::allocEnv(unsigned int size)
 {
     nrEnvs++;
     nrValuesInEnvs += size;
-    Env * env = (Env *) GC_MALLOC(sizeof(Env) + size * sizeof(Value));
+    Env * env = (Env *) GC_MALLOC(sizeof(Env) + size * sizeof(Value *));
     return *env;
 }
 
@@ -465,7 +463,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
             nix::Attr & a = (*v.attrs)[i->first];
             a.value = state.allocValue();
             mkThunk(*a.value, env2, i->second.first);
-            mkCopy(env2.values[displ++], *a.value);
+            env2.values[displ++] = a.value;
             a.pos = &i->second.second;
         }
 
@@ -475,7 +473,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
             nix::Attr & a = (*v.attrs)[i->first.name];
             Value * v2 = state.lookupVar(&env, i->first);
             a.value = v2;
-            mkCopy(env2.values[displ++], *v2);
+            env2.values[displ++] = v2;
             a.pos = &i->second;
         }
 
@@ -493,7 +491,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
             foreach (Bindings::iterator, i, *overrides->second.value->attrs) {
                 nix::Attr & a = (*v.attrs)[i->first];
                 if (a.value)
-                    mkCopy(env2.values[displs[i->first]], *i->second.value);
+                    env2.values[displs[i->first]] = i->second.value;
                 a = i->second;
             }
         }
@@ -527,13 +525,15 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)
 
     /* The recursive attributes are evaluated in the new
        environment. */
-    foreach (ExprAttrs::Attrs::iterator, i, attrs->attrs)
-        mkThunk(env2.values[displ++], env2, i->second.first);
+    foreach (ExprAttrs::Attrs::iterator, i, attrs->attrs) {
+        env2.values[displ] = state.allocValue();
+        mkThunk(*env2.values[displ++], env2, i->second.first);
+    }
 
     /* The inherited attributes, on the other hand, are evaluated in
        the original environment. */
     foreach (list<ExprAttrs::Inherited>::iterator, i, attrs->inherited)
-        mkCopy(env2.values[displ++], *state.lookupVar(&env, i->first));
+        env2.values[displ++] = state.lookupVar(&env, i->first);
 
     state.eval(env2, body, v);
 }
@@ -654,13 +654,13 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v)
     unsigned int displ = 0;
 
     if (!fun.lambda.fun->matchAttrs)
-        env2.values[displ++] = arg;
+        env2.values[displ++] = &arg;
 
     else {
         forceAttrs(arg);
         
         if (!fun.lambda.fun->arg.empty())
-            env2.values[displ++] = arg;
+            env2.values[displ++] = &arg;
 
         /* For each formal argument, get the actual argument.  If
            there is no matching actual argument but the formal
@@ -670,11 +670,12 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v)
             Bindings::iterator j = arg.attrs->find(i->name);
             if (j == arg.attrs->end()) {
                 if (!i->def) throwTypeError("function at %1% called without required argument `%2%'",
-                    fun.lambda.fun->pos, i->name);   
-                mkThunk(env2.values[displ++], env2, i->def);
+                    fun.lambda.fun->pos, i->name);
+                env2.values[displ] = allocValue();
+                mkThunk(*env2.values[displ++], env2, i->def);
             } else {
                 attrsUsed++;
-                mkCopy(env2.values[displ++], *j->second.value);
+                env2.values[displ++] = j->second.value;
             }
         }
 
@@ -725,7 +726,8 @@ void ExprWith::eval(EvalState & state, Env & env, Value & v)
     env2.up = &env;
     env2.prevWith = prevWith;
 
-    state.evalAttrs(env, attrs, env2.values[0]);
+    env2.values[0] = state.allocValue();
+    state.evalAttrs(env, attrs, *env2.values[0]);
 
     state.eval(env2, body, v);
 }
@@ -864,10 +866,6 @@ void EvalState::forceValue(Value & v)
             throw;
         }
     }
-    else if (v.type == tCopy) {
-        forceValue(*v.val);
-        v = *v.val;
-    }
     else if (v.type == tApp)
         callFunction(*v.app.left, *v.app.right, v);
     else if (v.type == tBlackhole)
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index ee7db91a0..b6ec927f0 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -38,7 +38,6 @@ typedef enum {
     tThunk,
     tApp,
     tLambda,
-    tCopy,
     tBlackhole,
     tPrimOp,
     tPrimOpApp,
@@ -116,7 +115,7 @@ struct Env
 {
     Env * up;
     unsigned int prevWith; // nr of levels up to next `with' environment
-    Value values[0];
+    Value * values[0];
 };
 
 
@@ -150,13 +149,6 @@ static inline void mkThunk(Value & v, Env & env, Expr * expr)
 }
 
 
-static inline void mkCopy(Value & v, Value & src)
-{
-    v.type = tCopy;
-    v.val = &src;
-}
-
-
 static inline void mkApp(Value & v, Value & left, Value & right)
 {
     v.type = tApp;

From 3f66cfb96b6f4bbddc8bf3b15e364fd522e028bc Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Sat, 23 Oct 2010 18:18:07 +0000
Subject: [PATCH 08/23] * Remove allocValues().

---
 src/libexpr/eval.cc     | 22 +++++-----------------
 src/libexpr/eval.hh     |  1 -
 src/libexpr/primops.cc  | 23 +++++++----------------
 src/nix-env/user-env.cc |  4 ++--
 4 files changed, 14 insertions(+), 36 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 3f2371d79..faf18cb7f 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -282,13 +282,6 @@ Value * EvalState::allocValue()
 }
 
 
-Value * EvalState::allocValues(unsigned int count)
-{
-    nrValues += count;
-    return (Value *) GC_MALLOC(count * sizeof(Value));
-}
-
-
 Env & EvalState::allocEnv(unsigned int size)
 {
     nrEnvs++;
@@ -542,11 +535,8 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)
 void ExprList::eval(EvalState & state, Env & env, Value & v)
 {
     state.mkList(v, elems.size());
-    Value * vs = state.allocValues(v.list.length);
-    for (unsigned int n = 0; n < v.list.length; ++n) {
-        v.list.elems[n] = &vs[n];
-        mkThunk(vs[n], env, elems[n]);
-    }
+    for (unsigned int n = 0; n < v.list.length; ++n)
+        mkThunk(*(v.list.elems[n] = state.allocValue()), env, elems[n]);
 }
 
 
@@ -630,12 +620,10 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v)
                 throw;
             }
         } else {
-            Value * v2 = allocValues(2);
-            v2[0] = fun;
-            v2[1] = arg;
             v.type = tPrimOpApp;
-            v.primOpApp.left = &v2[0];
-            v.primOpApp.right = &v2[1];
+            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 b6ec927f0..5f5966930 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -290,7 +290,6 @@ public:
     
     /* Allocation primitives. */
     Value * allocValue();
-    Value * allocValues(unsigned int count);
     Env & allocEnv(unsigned int size);
 
     Value * allocAttr(Value & vAttrs, const Symbol & name);
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index c7709ca9e..01cbf7a7c 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -167,13 +167,9 @@ static void prim_genericClosure(EvalState & state, Value * * args, Value & v)
 
     /* Create the result list. */
     state.mkList(v, res.size());
-    Value * vs = state.allocValues(res.size());
-
     unsigned int n = 0;
-    foreach (list<Value>::iterator, i, res) {
-        v.list.elems[n] = &vs[n];
-        vs[n++] = *i;
-    }
+    foreach (list<Value>::iterator, i, res)
+        *(v.list.elems[n++] = state.allocValue()) = *i;
 }
 
 
@@ -691,17 +687,14 @@ static void prim_attrNames(EvalState & state, Value * * args, Value & v)
     state.forceAttrs(*args[0]);
 
     state.mkList(v, args[0]->attrs->size());
-    Value * vs = state.allocValues(v.list.length);
 
     StringSet names;
     foreach (Bindings::iterator, i, *args[0]->attrs)
         names.insert(i->first);
 
     unsigned int n = 0;
-    foreach (StringSet::iterator, i, names) {
-        v.list.elems[n] = &vs[n];
-        mkString(vs[n++], *i);
-    }
+    foreach (StringSet::iterator, i, names)
+        mkString(*(v.list.elems[n++] = state.allocValue()), *i);
 }
 
 
@@ -870,12 +863,10 @@ static void prim_map(EvalState & state, Value * * args, Value & v)
     state.forceList(*args[1]);
 
     state.mkList(v, args[1]->list.length);
-    Value * vs = state.allocValues(v.list.length);
 
-    for (unsigned int n = 0; n < v.list.length; ++n) {
-        v.list.elems[n] = &vs[n];
-        mkApp(vs[n], *args[0], *args[1]->list.elems[n]);
-    }
+    for (unsigned int n = 0; n < v.list.length; ++n)
+        mkApp(*(v.list.elems[n] = state.allocValue()), 
+            *args[0], *args[1]->list.elems[n]);
 }
 
 
diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc
index 07c94fe4d..34e00b7c2 100644
--- a/src/nix-env/user-env.cc
+++ b/src/nix-env/user-env.cc
@@ -59,7 +59,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
            the meta attributes. */
         Path drvPath = keepDerivations ? i->queryDrvPath(state) : "";
 
-        Value & v(*state.allocValues(1));
+        Value & v(*state.allocValue());
         manifest.list.elems[n++] = &v;
         state.mkAttrs(v);
 
@@ -83,7 +83,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
                     state.mkList(v2, j->second.stringValues.size());
                     unsigned int m = 0;
                     foreach (Strings::const_iterator, k, j->second.stringValues) {
-                        v2.list.elems[m] = state.allocValues(1);
+                        v2.list.elems[m] = state.allocValue();
                         mkString(*v2.list.elems[m++], *k);
                     }
                     break;

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 09/23] * 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 faf18cb7f..72a3bf9b9 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 5f5966930..ec4939442 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);
     

From b2ba62170cc8359d2f8bbbd9dbacf331b98151fe Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Sat, 23 Oct 2010 21:11:59 +0000
Subject: [PATCH 10/23] * Optimise string constants by putting them in the
 symbol table.

---
 src/libexpr/eval.cc    | 10 +++++-
 src/libexpr/lexer.l    |  8 ++---
 src/libexpr/nixexpr.hh |  4 +--
 src/libexpr/parser.y   | 72 +++++++++++++++++++++++-------------------
 4 files changed, 55 insertions(+), 39 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 72a3bf9b9..8c33fd224 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -249,6 +249,14 @@ void mkString(Value & v, const string & s, const PathSet & context)
 }
 
 
+void mkString(Value & v, const Symbol & s)
+{
+    v.type = tString;
+    v.string.s = ((string) s).c_str();
+    v.string.context = 0;
+}
+
+
 void mkPath(Value & v, const char * s)
 {
     clearValue(v);
@@ -429,7 +437,7 @@ void ExprInt::eval(EvalState & state, Env & env, Value & v)
 
 void ExprString::eval(EvalState & state, Env & env, Value & v)
 {
-    mkString(v, s.c_str());
+    mkString(v, s);
 }
 
 
diff --git a/src/libexpr/lexer.l b/src/libexpr/lexer.l
index f29f9b684..5b27e2582 100644
--- a/src/libexpr/lexer.l
+++ b/src/libexpr/lexer.l
@@ -46,7 +46,7 @@ static void adjustLoc(YYLTYPE * loc, const char * s, size_t len)
 }
 
 
-static Expr * unescapeStr(const char * s)
+static Expr * unescapeStr(SymbolTable & symbols, const char * s)
 {
     string t;
     char c;
@@ -66,7 +66,7 @@ static Expr * unescapeStr(const char * s)
         }
         else t += c;
     }
-    return new ExprString(t);
+    return new ExprString(symbols.create(t));
 }
 
  
@@ -119,7 +119,7 @@ inherit     { return INHERIT; }
                  "$\"" will be consumed as part of a string, rather
                  than a "$" followed by the string terminator.
                  Disallow "$\"" for now. */
-              yylval->e = unescapeStr(yytext);
+              yylval->e = unescapeStr(data->symbols, yytext);
               return STR;
             }
 <STRING>\$\{  { BEGIN(INITIAL); return DOLLAR_CURLY; }
@@ -140,7 +140,7 @@ inherit     { return INHERIT; }
                    return IND_STR;
                  }
 <IND_STRING>\'\'\\. {
-                   yylval->e = unescapeStr(yytext + 2);
+                   yylval->e = unescapeStr(data->symbols, yytext + 2);
                    return IND_STR;
                  }
 <IND_STRING>\$\{ { BEGIN(INITIAL); return DOLLAR_CURLY; }
diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh
index 5c0071dca..1aa3df368 100644
--- a/src/libexpr/nixexpr.hh
+++ b/src/libexpr/nixexpr.hh
@@ -65,8 +65,8 @@ struct ExprInt : Expr
 
 struct ExprString : Expr
 {
-    string s;
-    ExprString(const string & s) : s(s) { };
+    Symbol s;
+    ExprString(const Symbol & s) : s(s) { };
     COMMON_METHODS
 };
 
diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y
index b0c54339b..3a72a4ade 100644
--- a/src/libexpr/parser.y
+++ b/src/libexpr/parser.y
@@ -7,19 +7,44 @@
 %parse-param { yyscan_t scanner }
 %parse-param { ParseData * data }
 %lex-param { yyscan_t scanner }
+%lex-param { ParseData * data }
 
-
-%{
-/* Newer versions of Bison copy the declarations below to
-   parser-tab.hh, which sucks bigtime since lexer.l doesn't want that
-   stuff.  So allow it to be excluded. */
-#ifndef BISON_HEADER_HACK
-#define BISON_HEADER_HACK
-
+%code requires {
+    
+#ifndef BISON_HEADER
+#define BISON_HEADER
+    
 #include "util.hh"
     
 #include "nixexpr.hh"
 
+namespace nix {
+
+    struct ParseData 
+    {
+        SymbolTable & symbols;
+        Expr * result;
+        Path basePath;
+        Path path;
+        string error;
+        Symbol sLetBody;
+        ParseData(SymbolTable & symbols)
+            : symbols(symbols)
+            , sLetBody(symbols.create("<let-body>"))
+            { };
+    };
+    
+}
+
+#define YY_DECL int yylex \
+    (YYSTYPE * yylval_param, YYLTYPE * yylloc_param, yyscan_t yyscanner, nix::ParseData * data)
+
+#endif
+
+}
+
+%{
+
 #include "parser-tab.hh"
 #include "lexer-tab.hh"
 #define YYSTYPE YYSTYPE // workaround a bug in Bison 2.4
@@ -28,27 +53,13 @@
 #include <stdlib.h>
 #include <string.h>
 
+YY_DECL;
 
 using namespace nix;
 
 
 namespace nix {
-
     
-struct ParseData 
-{
-    SymbolTable & symbols;
-    Expr * result;
-    Path basePath;
-    Path path;
-    string error;
-    Symbol sLetBody;
-    ParseData(SymbolTable & symbols)
-        : symbols(symbols)
-        , sLetBody(symbols.create("<let-body>"))
-    { };
-};
-
 
 static string showAttrPath(const vector<Symbol> & attrPath)
 {
@@ -113,9 +124,9 @@ static void addFormal(const Pos & pos, Formals * formals, const Formal & formal)
 }
 
 
-static Expr * stripIndentation(vector<Expr *> & es)
+static Expr * stripIndentation(SymbolTable & symbols, vector<Expr *> & es)
 {
-    if (es.empty()) return new ExprString("");
+    if (es.empty()) return new ExprString(symbols.create(""));
     
     /* Figure out the minimum indentation.  Note that by design
        whitespace-only final lines are not taken into account.  (So
@@ -195,7 +206,7 @@ static Expr * stripIndentation(vector<Expr *> & es)
                 s2 = string(s2, 0, p + 1);
         }
 
-        es2->push_back(new ExprString(s2));
+        es2->push_back(new ExprString(symbols.create(s2)));
     }
 
     return new ExprConcatStrings(es2);
@@ -224,9 +235,6 @@ void yyerror(YYLTYPE * loc, yyscan_t scanner, ParseData * data, const char * err
 }
 
 
-#endif
-
-
 %}
 
 %union {
@@ -337,15 +345,15 @@ expr_simple
   | INT { $$ = new ExprInt($1); }
   | '"' string_parts '"' {
       /* For efficiency, and to simplify parse trees a bit. */
-      if ($2->empty()) $$ = new ExprString("");
+      if ($2->empty()) $$ = new ExprString(data->symbols.create(""));
       else if ($2->size() == 1) $$ = $2->front();
       else $$ = new ExprConcatStrings($2);
   }
   | IND_STRING_OPEN ind_string_parts IND_STRING_CLOSE {
-      $$ = stripIndentation(*$2);
+      $$ = stripIndentation(data->symbols, *$2);
   }
   | PATH { $$ = new ExprPath(absPath($1, data->basePath)); }
-  | URI { $$ = new ExprString($1); }
+  | URI { $$ = new ExprString(data->symbols.create($1)); }
   | '(' expr ')' { $$ = $2; }
   /* Let expressions `let {..., body = ...}' are just desugared
      into `(rec {..., body = ...}).body'. */

From 02934b12000d5a837ef4cac78025cb531330c2b3 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Sat, 23 Oct 2010 22:55:30 +0000
Subject: [PATCH 11/23] * Regression test for listToAttr's behaviour if an
 attribute name   occurs multiple times.

---
 tests/lang/eval-okay-listtoattrs.exp | 2 +-
 tests/lang/eval-okay-listtoattrs.nix | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/lang/eval-okay-listtoattrs.exp b/tests/lang/eval-okay-listtoattrs.exp
index 11d29b588..74abef7bc 100644
--- a/tests/lang/eval-okay-listtoattrs.exp
+++ b/tests/lang/eval-okay-listtoattrs.exp
@@ -1 +1 @@
-"AA"
+"AAbar"
diff --git a/tests/lang/eval-okay-listtoattrs.nix b/tests/lang/eval-okay-listtoattrs.nix
index d5cd726b0..023c1d84e 100644
--- a/tests/lang/eval-okay-listtoattrs.nix
+++ b/tests/lang/eval-okay-listtoattrs.nix
@@ -7,4 +7,5 @@ let
   a = builtins.listToAttrs list;
   b = builtins.listToAttrs ( list ++ list );
   r = builtins.listToAttrs [ (asi "result" [ a b ]) ( asi "throw" (throw "this should not be thrown")) ];
-in concat (map (x: x.a) r.result)
+  x = builtins.listToAttrs [ (asi "foo" "bla") (asi "foo" "bar") ];
+in concat (map (x: x.a) r.result) + x.foo

From a247d20604a97ff6e84b87f66e3338714e7964f0 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Sat, 23 Oct 2010 22:58:24 +0000
Subject: [PATCH 12/23] * Fix compiling without Boehm. * Fix the stats.

---
 src/libexpr/eval.cc | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 8c33fd224..d73f99a15 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -13,11 +13,15 @@
 #include <gc/gc.h>
 #include <gc/gc_cpp.h>
 
+#define NEW (UseGC)
+
 #else
 
 #define GC_STRDUP strdup
 #define GC_MALLOC malloc
 
+#define NEW new
+
 #endif
 
 
@@ -159,7 +163,7 @@ void EvalState::addPrimOp(const string & name,
     string name2 = string(name, 0, 2) == "__" ? string(name, 2) : name;
     Symbol sym = symbols.create(name);
     v->type = tPrimOp;
-    v->primOp = new (UseGC) PrimOp(primOp, arity, sym);
+    v->primOp = NEW PrimOp(primOp, arity, sym);
     staticBaseEnv.vars[sym] = baseEnvDispl;
     baseEnv.values[baseEnvDispl++] = v;
     (*baseEnv.values[0]->attrs)[symbols.create(name2)].value = v;
@@ -320,11 +324,7 @@ void EvalState::mkAttrs(Value & v)
 {
     clearValue(v);
     v.type = tAttrs;
-#if HAVE_BOEHMGC
-    v.attrs = new (UseGC) Bindings;
-#else
-    v.attrs = new Bindings;
-#endif
+    v.attrs = NEW Bindings;
     nrAttrsets++;
 }
 
@@ -1133,12 +1133,10 @@ void EvalState::printStats()
     printMsg(v, format("  stack space per eval() level: %1% bytes")
         % ((&x - deepestStack) / (float) maxRecursionDepth));
     printMsg(v, format("  environments allocated: %1% (%2% bytes)")
-        % nrEnvs % (nrEnvs * sizeof(Env)));
-    printMsg(v, format("  values allocated in environments: %1% (%2% bytes)")
-        % nrValuesInEnvs % (nrValuesInEnvs * sizeof(Value)));
+        % nrEnvs % (nrEnvs * sizeof(Env) + nrValuesInEnvs * sizeof(Value *)));
     printMsg(v, format("  list elements: %1% (%2% bytes)")
         % nrListElems % (nrListElems * sizeof(Value *)));
-    printMsg(v, format("  misc. values allocated: %1% (%2% bytes)")
+    printMsg(v, format("  values allocated: %1% (%2% bytes)")
         % nrValues % (nrValues * sizeof(Value)));
     printMsg(v, format("  attribute sets allocated: %1%") % nrAttrsets);
     printMsg(v, format("  right-biased unions: %1%") % nrOpUpdates);

From 0b305c534f989dbc3645ff03e070b0e4665fdeb7 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Sun, 24 Oct 2010 00:41:29 +0000
Subject: [PATCH 13/23] * Store attribute sets as a vector instead of a map
 (i.e. a red-black   tree).  This saves a lot of memory.  The vector should be
 sorted so   that names can be looked up using binary search, but this is not
 the   case yet.  (Surprisingly, looking up attributes using linear search  
 doesn't have a big impact on performance.)

  Memory consumption for

    $ nix-instantiate /etc/nixos/nixos/tests -A bittorrent.test --readonly-mode

  on x86_64-linux with GC enabled is now 185 MiB (compared to 946
  MiB on the trunk).
---
 src/libexpr/attr-path.cc    |  2 +-
 src/libexpr/eval.cc         | 84 +++++++++++++++++++++----------------
 src/libexpr/eval.hh         | 18 +++++++-
 src/libexpr/get-drvs.cc     | 34 +++++++--------
 src/libexpr/primops.cc      | 65 +++++++++++++++-------------
 src/libexpr/symbol-table.hh |  2 +
 src/libexpr/value-to-xml.cc | 14 +++----
 7 files changed, 129 insertions(+), 90 deletions(-)

diff --git a/src/libexpr/attr-path.cc b/src/libexpr/attr-path.cc
index 365b03c0b..49c08339a 100644
--- a/src/libexpr/attr-path.cc
+++ b/src/libexpr/attr-path.cc
@@ -49,7 +49,7 @@ void findAlongAttrPath(EvalState & state, const string & attrPath,
             Bindings::iterator a = v.attrs->find(state.symbols.create(attr));
             if (a == v.attrs->end())
                 throw Error(format("attribute `%1%' in selection path `%2%' not found") % attr % curPath);
-            v = *a->second.value;
+            v = *a->value;
         }
 
         else if (apType == apIndex) {
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index d73f99a15..85394b768 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -13,7 +13,7 @@
 #include <gc/gc.h>
 #include <gc/gc_cpp.h>
 
-#define NEW (UseGC)
+#define NEW new (UseGC)
 
 #else
 
@@ -30,6 +30,23 @@
 
 
 namespace nix {
+
+
+Bindings::iterator Bindings::find(const Symbol & name)
+{
+    iterator i = begin();
+    for ( ; i != end() && i->name != name; ++i) ;
+    return i;
+}
+
+
+Attr & Bindings::operator [] (const Symbol & name)
+{
+    iterator i = find(name);
+    if (i != end()) return *i;
+    push_back(Attr(name, 0));
+    return back();
+}
     
 
 std::ostream & operator << (std::ostream & str, const Value & v)
@@ -62,7 +79,7 @@ std::ostream & operator << (std::ostream & str, const Value & v)
         typedef std::map<string, Value *> Sorted;
         Sorted sorted;
         foreach (Bindings::iterator, i, *v.attrs)
-            sorted[i->first] = i->second.value;
+            sorted[i->name] = i->value;
         foreach (Sorted::iterator, i, sorted)
             str << i->first << " = " << *i->second << "; ";
         str << "}";
@@ -152,7 +169,7 @@ void EvalState::addConstant(const string & name, Value & v)
     staticBaseEnv.vars[symbols.create(name)] = baseEnvDispl;
     baseEnv.values[baseEnvDispl++] = v2;
     string name2 = string(name, 0, 2) == "__" ? string(name, 2) : name;
-    (*baseEnv.values[0]->attrs)[symbols.create(name2)].value = v2;
+    baseEnv.values[0]->attrs->push_back(Attr(symbols.create(name2), v2));
 }
 
 
@@ -166,7 +183,7 @@ void EvalState::addPrimOp(const string & name,
     v->primOp = NEW PrimOp(primOp, arity, sym);
     staticBaseEnv.vars[sym] = baseEnvDispl;
     baseEnv.values[baseEnvDispl++] = v;
-    (*baseEnv.values[0]->attrs)[symbols.create(name2)].value = v;
+    baseEnv.values[0]->attrs->push_back(Attr(symbols.create(name2), v));
 }
 
 
@@ -277,7 +294,7 @@ Value * EvalState::lookupVar(Env * env, const VarRef & var)
         while (1) {
             Bindings::iterator j = env->values[0]->attrs->find(var.name);
             if (j != env->values[0]->attrs->end())
-                return j->second.value;
+                return j->value;
             if (env->prevWith == 0)
                 throwEvalError("undefined variable `%1%'", var.name);
             for (unsigned int l = env->prevWith; l; --l, env = env->up) ;
@@ -305,9 +322,9 @@ Env & EvalState::allocEnv(unsigned int size)
 
 Value * EvalState::allocAttr(Value & vAttrs, const Symbol & name)
 {
-    Attr & a = (*vAttrs.attrs)[name];
-    a.value = allocValue();
-    return a.value;
+    Value * v = allocValue();
+    vAttrs.attrs->push_back(Attr(name, v));
+    return v;
 }
 
     
@@ -338,8 +355,7 @@ void EvalState::mkThunk_(Value & v, Expr * expr)
 void EvalState::cloneAttrs(Value & src, Value & dst)
 {
     mkAttrs(dst);
-    foreach (Bindings::iterator, i, *src.attrs)
-        (*dst.attrs)[i->first] = i->second;
+    *dst.attrs = *src.attrs;
 }
 
 
@@ -462,21 +478,18 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
         /* The recursive attributes are evaluated in the new
            environment. */
         foreach (Attrs::iterator, i, attrs) {
-            nix::Attr & a = (*v.attrs)[i->first];
-            a.value = state.allocValue();
-            mkThunk(*a.value, env2, i->second.first);
-            env2.values[displ++] = a.value;
-            a.pos = &i->second.second;
+            Value * vAttr = state.allocValue();
+            mkThunk(*vAttr, env2, i->second.first);
+            env2.values[displ++] = vAttr;
+            v.attrs->push_back(nix::Attr(i->first, vAttr, &i->second.second));
         }
 
         /* The inherited attributes, on the other hand, are
            evaluated in the original environment. */
         foreach (list<Inherited>::iterator, i, inherited) {
-            nix::Attr & a = (*v.attrs)[i->first.name];
-            Value * v2 = state.lookupVar(&env, i->first);
-            a.value = v2;
-            env2.values[displ++] = v2;
-            a.pos = &i->second;
+            Value * vAttr = state.lookupVar(&env, i->first);
+            env2.values[displ++] = vAttr;
+            v.attrs->push_back(nix::Attr(i->first.name, vAttr, &i->second));
         }
 
         /* If the rec contains an attribute called `__overrides', then
@@ -489,12 +502,12 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
            Hence we need __overrides.) */
         Bindings::iterator overrides = v.attrs->find(state.sOverrides);
         if (overrides != v.attrs->end()) {
-            state.forceAttrs(*overrides->second.value);
-            foreach (Bindings::iterator, i, *overrides->second.value->attrs) {
-                nix::Attr & a = (*v.attrs)[i->first];
+            state.forceAttrs(*overrides->value);
+            foreach (Bindings::iterator, i, *overrides->value->attrs) {
+                nix::Attr & a = (*v.attrs)[i->name];
                 if (a.value)
-                    env2.values[displs[i->first]] = i->second.value;
-                a = i->second;
+                    env2.values[displs[i->name]] = i->value;
+                a = *i;
             }
         }
     }
@@ -565,13 +578,13 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v)
     if (i == v2.attrs->end())
         throwEvalError("attribute `%1%' missing", name);
     try {            
-        state.forceValue(*i->second.value);
+        state.forceValue(*i->value);
     } catch (Error & e) {
         addErrorPrefix(e, "while evaluating the attribute `%1%' at %2%:\n",
-            name, *i->second.pos);
+            name, *i->pos);
         throw;
     }
-    v = *i->second.value;
+    v = *i->value;
 }
 
 
@@ -676,7 +689,7 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v)
                 mkThunk(*env2.values[displ++], env2, i->def);
             } else {
                 attrsUsed++;
-                env2.values[displ++] = j->second.value;
+                env2.values[displ++] = j->value;
             }
         }
 
@@ -712,7 +725,7 @@ void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res)
     foreach (Formals::Formals_::iterator, i, fun.lambda.fun->formals->formals) {
         Bindings::iterator j = args.find(i->name);
         if (j != args.end())
-            (*actualArgs.attrs)[i->name] = j->second;
+            (*actualArgs.attrs)[i->name] = *j;
         else if (!i->def)
             throwTypeError("cannot auto-call a function that has an argument without a default value (`%1%')", i->name);
     }
@@ -801,8 +814,9 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v)
 
     state.cloneAttrs(v1, v);
 
+    /* !!! fix */
     foreach (Bindings::iterator, i, *v2.attrs)
-        (*v.attrs)[i->first] = i->second;
+        (*v.attrs)[i->name] = *i;
 
     state.nrOpUpdateValuesCopied += v.attrs->size();
 }
@@ -880,7 +894,7 @@ void EvalState::strictForceValue(Value & v)
     
     if (v.type == tAttrs) {
         foreach (Bindings::iterator, i, *v.attrs)
-            strictForceValue(*i->second.value);
+            strictForceValue(*i->value);
     }
     
     else if (v.type == tList) {
@@ -971,7 +985,7 @@ bool EvalState::isDerivation(Value & v)
 {
     if (v.type != tAttrs) return false;
     Bindings::iterator i = v.attrs->find(sType);
-    return i != v.attrs->end() && forceStringNoCtx(*i->second.value) == "derivation";
+    return i != v.attrs->end() && forceStringNoCtx(*i->value) == "derivation";
 }
 
 
@@ -1015,7 +1029,7 @@ string EvalState::coerceToString(Value & v, PathSet & context,
         Bindings::iterator i = v.attrs->find(sOutPath);
         if (i == v.attrs->end())
             throwTypeError("cannot coerce an attribute set (except a derivation) to a string");
-        return coerceToString(*i->second.value, context, coerceMore, copyToStore);
+        return coerceToString(*i->value, context, coerceMore, copyToStore);
     }
 
     if (coerceMore) {
@@ -1103,7 +1117,7 @@ bool EvalState::eqValues(Value & v1, Value & v2)
             if (v1.attrs->size() != v2.attrs->size()) return false;
             Bindings::iterator i = v1.attrs->begin(), j = v2.attrs->begin();
             for ( ; i != v1.attrs->end(); ++i, ++j)
-                if (i->first != j->first || !eqValues(*i->second.value, *j->second.value))
+                if (i->name != j->name || !eqValues(*i->value, *j->value))
                     return false;
             return true;
         }
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index ec4939442..2f1b3fa45 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -20,13 +20,24 @@ struct Env;
 struct Value;
 struct Attr;
 
+
+/* Attribute sets are represented as a vector of attributes, sorted by
+   symbol (i.e. pointer to the attribute name in the symbol table). */
 #if HAVE_BOEHMGC
-typedef std::map<Symbol, Attr, std::less<Symbol>, gc_allocator<std::pair<const Symbol, Attr> > > Bindings;
+typedef std::vector<Attr, gc_allocator<Attr> > BindingsBase;
 #else
-typedef std::map<Symbol, Attr> Bindings;
+typedef std::vector<Attr> BindingsBase;
 #endif
 
 
+class Bindings : public BindingsBase
+{
+public:
+    iterator find(const Symbol & name);
+    Attr & operator [] (const Symbol & name);
+};
+
+
 typedef enum {
     tInt = 1,
     tBool,
@@ -125,8 +136,11 @@ struct Env
 
 struct Attr
 {
+    Symbol name;
     Value * value;
     Pos * pos;
+    Attr(Symbol name, Value * value, Pos * pos = &noPos)
+        : name(name), value(value), pos(pos) { };
     Attr() : pos(&noPos) { };
 };
 
diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc
index 63106b87b..312c2cd40 100644
--- a/src/libexpr/get-drvs.cc
+++ b/src/libexpr/get-drvs.cc
@@ -10,7 +10,7 @@ string DrvInfo::queryDrvPath(EvalState & state) const
     if (drvPath == "" && attrs) {
         Bindings::iterator i = attrs->find(state.sDrvPath);
         PathSet context;
-        (string &) drvPath = i != attrs->end() ? state.coerceToPath(*i->second.value, context) : "";
+        (string &) drvPath = i != attrs->end() ? state.coerceToPath(*i->value, context) : "";
     }
     return drvPath;
 }
@@ -21,7 +21,7 @@ string DrvInfo::queryOutPath(EvalState & state) const
     if (outPath == "" && attrs) {
         Bindings::iterator i = attrs->find(state.sOutPath);
         PathSet context;
-        (string &) outPath = i != attrs->end() ? state.coerceToPath(*i->second.value, context) : "";
+        (string &) outPath = i != attrs->end() ? state.coerceToPath(*i->value, context) : "";
     }
     return outPath;
 }
@@ -36,23 +36,23 @@ MetaInfo DrvInfo::queryMetaInfo(EvalState & state) const
     Bindings::iterator a = attrs->find(state.sMeta);
     if (a == attrs->end()) return meta; /* fine, empty meta information */
 
-    state.forceAttrs(*a->second.value);
+    state.forceAttrs(*a->value);
 
-    foreach (Bindings::iterator, i, *a->second.value->attrs) {
+    foreach (Bindings::iterator, i, *a->value->attrs) {
         MetaValue value;
-        state.forceValue(*i->second.value);
-        if (i->second.value->type == tString) {
+        state.forceValue(*i->value);
+        if (i->value->type == tString) {
             value.type = MetaValue::tpString;
-            value.stringValue = i->second.value->string.s;
-        } else if (i->second.value->type == tInt) {
+            value.stringValue = i->value->string.s;
+        } else if (i->value->type == tInt) {
             value.type = MetaValue::tpInt;
-            value.intValue = i->second.value->integer;
-        } else if (i->second.value->type == tList) {
+            value.intValue = i->value->integer;
+        } else if (i->value->type == tList) {
             value.type = MetaValue::tpStrings;
-            for (unsigned int j = 0; j < i->second.value->list.length; ++j)
-                value.stringValues.push_back(state.forceStringNoCtx(*i->second.value->list.elems[j]));
+            for (unsigned int j = 0; j < i->value->list.length; ++j)
+                value.stringValues.push_back(state.forceStringNoCtx(*i->value->list.elems[j]));
         } else continue;
-        ((MetaInfo &) meta)[i->first] = value;
+        ((MetaInfo &) meta)[i->name] = value;
     }
 
     return meta;
@@ -99,13 +99,13 @@ static bool getDerivation(EvalState & state, Value & v,
         Bindings::iterator i = v.attrs->find(state.sName);
         /* !!! We really would like to have a decent back trace here. */
         if (i == v.attrs->end()) throw TypeError("derivation name missing");
-        drv.name = state.forceStringNoCtx(*i->second.value);
+        drv.name = state.forceStringNoCtx(*i->value);
 
         Bindings::iterator i2 = v.attrs->find(state.sSystem);
         if (i2 == v.attrs->end())
             drv.system = "unknown";
         else
-            drv.system = state.forceStringNoCtx(*i2->second.value);
+            drv.system = state.forceStringNoCtx(*i2->value);
 
         drv.attrs = v.attrs;
 
@@ -163,7 +163,7 @@ static void getDerivations(EvalState & state, Value & vIn,
         typedef std::map<string, Symbol> SortedSymbols;
         SortedSymbols attrs;
         foreach (Bindings::iterator, i, *v.attrs)
-            attrs.insert(std::pair<string, Symbol>(i->first, i->first));
+            attrs.insert(std::pair<string, Symbol>(i->name, i->name));
 
         foreach (SortedSymbols::iterator, i, attrs) {
             startNest(nest, lvlDebug, format("evaluating attribute `%1%'") % i->first);
@@ -178,7 +178,7 @@ static void getDerivations(EvalState & state, Value & vIn,
                    attribute. */
                 if (v2.type == tAttrs) {
                     Bindings::iterator j = v2.attrs->find(state.symbols.create("recurseForDerivations"));
-                    if (j != v2.attrs->end() && state.forceBool(*j->second.value))
+                    if (j != v2.attrs->end() && state.forceBool(*j->value))
                         getDerivations(state, v2, pathPrefix2, autoArgs, drvs, done);
                 }
             }
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 01cbf7a7c..20b839508 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -119,18 +119,18 @@ static void prim_genericClosure(EvalState & state, Value * * args, Value & v)
         args[0]->attrs->find(state.symbols.create("startSet"));
     if (startSet == args[0]->attrs->end())
         throw EvalError("attribute `startSet' required");
-    state.forceList(*startSet->second.value);
+    state.forceList(*startSet->value);
 
     list<Value *> workSet;
-    for (unsigned int n = 0; n < startSet->second.value->list.length; ++n)
-        workSet.push_back(startSet->second.value->list.elems[n]);
+    for (unsigned int n = 0; n < startSet->value->list.length; ++n)
+        workSet.push_back(startSet->value->list.elems[n]);
 
     /* Get the operator. */
     Bindings::iterator op =
         args[0]->attrs->find(state.symbols.create("operator"));
     if (op == args[0]->attrs->end())
         throw EvalError("attribute `operator' required");
-    state.forceValue(*op->second.value);
+    state.forceValue(*op->value);
 
     /* Construct the closure by applying the operator to element of
        `workSet', adding the result to `workSet', continuing until
@@ -147,15 +147,15 @@ static void prim_genericClosure(EvalState & state, Value * * args, Value & v)
             e->attrs->find(state.symbols.create("key"));
         if (key == e->attrs->end())
             throw EvalError("attribute `key' required");
-        state.forceValue(*key->second.value);
+        state.forceValue(*key->value);
 
-        if (doneKeys.find(*key->second.value) != doneKeys.end()) continue;
-        doneKeys.insert(*key->second.value);
+        if (doneKeys.find(*key->value) != doneKeys.end()) continue;
+        doneKeys.insert(*key->value);
         res.push_back(*e);
         
         /* Call the `operator' function with `e' as argument. */
         Value call;
-        mkApp(call, *op->second.value, *e);
+        mkApp(call, *op->value, *e);
         state.forceList(call);
 
         /* Add the values returned by the operator to the work set. */
@@ -322,9 +322,9 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
     if (attr == args[0]->attrs->end())
         throw EvalError("required attribute `name' missing");
     string drvName;
-    Pos & posDrvName(*attr->second.pos);
+    Pos & posDrvName(*attr->pos);
     try {        
-        drvName = state.forceStringNoCtx(*attr->second.value);
+        drvName = state.forceStringNoCtx(*attr->value);
     } catch (Error & e) {
         e.addPrefix(format("while evaluating the derivation attribute `name' at %1%:\n") % posDrvName);
         throw;
@@ -339,7 +339,7 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
     bool outputHashRecursive = false;
 
     foreach (Bindings::iterator, i, *args[0]->attrs) {
-        string key = i->first;
+        string key = i->name;
         startNest(nest, lvlVomit, format("processing attribute `%1%'") % key);
 
         try {
@@ -347,9 +347,9 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
             /* The `args' attribute is special: it supplies the
                command-line arguments to the builder. */
             if (key == "args") {
-                state.forceList(*i->second.value);
-                for (unsigned int n = 0; n < i->second.value->list.length; ++n) {
-                    string s = state.coerceToString(*i->second.value->list.elems[n], context, true);
+                state.forceList(*i->value);
+                for (unsigned int n = 0; n < i->value->list.length; ++n) {
+                    string s = state.coerceToString(*i->value->list.elems[n], context, true);
                     drv.args.push_back(s);
                 }
             }
@@ -357,11 +357,11 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
             /* All other attributes are passed to the builder through
                the environment. */
             else {
-                string s = state.coerceToString(*i->second.value, context, true);
+                string s = state.coerceToString(*i->value, context, true);
                 drv.env[key] = s;
                 if (key == "builder") drv.builder = s;
-                else if (i->first == state.sSystem) drv.platform = s;
-                else if (i->first == state.sName) drvName = s;
+                else if (i->name == state.sSystem) drv.platform = s;
+                else if (i->name == state.sName) drvName = s;
                 else if (key == "outputHash") outputHash = s;
                 else if (key == "outputHashAlgo") outputHashAlgo = s;
                 else if (key == "outputHashMode") {
@@ -373,7 +373,7 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
 
         } catch (Error & e) {
             e.addPrefix(format("while evaluating the derivation attribute `%1%' at %2%:\n")
-                % key % *i->second.pos);
+                % key % *i->pos);
             e.addPrefix(format("while instantiating the derivation named `%1%' at %2%:\n")
                 % drvName % posDrvName);
             throw;
@@ -690,7 +690,7 @@ static void prim_attrNames(EvalState & state, Value * * args, Value & v)
 
     StringSet names;
     foreach (Bindings::iterator, i, *args[0]->attrs)
-        names.insert(i->first);
+        names.insert(i->name);
 
     unsigned int n = 0;
     foreach (StringSet::iterator, i, names)
@@ -708,8 +708,8 @@ static void prim_getAttr(EvalState & state, Value * * args, Value & v)
     if (i == args[1]->attrs->end())
         throw EvalError(format("attribute `%1%' missing") % attr);
     // !!! add to stack trace?
-    state.forceValue(*i->second.value);
-    v = *i->second.value;
+    state.forceValue(*i->value);
+    v = *i->value;
 }
 
 
@@ -735,11 +735,18 @@ static void prim_removeAttrs(EvalState & state, Value * * args, Value & v)
     state.forceAttrs(*args[0]);
     state.forceList(*args[1]);
 
-    state.cloneAttrs(*args[0], v);
-
+    /* Get the attribute names to be removed. */
+    std::set<Symbol> names;
     for (unsigned int i = 0; i < args[1]->list.length; ++i) {
         state.forceStringNoCtx(*args[1]->list.elems[i]);
-        v.attrs->erase(state.symbols.create(args[1]->list.elems[i]->string.s));
+        names.insert(state.symbols.create(args[1]->list.elems[i]->string.s));
+    }
+
+    /* Copy all attributes not in that set. */
+    state.mkAttrs(v);
+    foreach (Bindings::iterator, i, *args[0]->attrs) {
+        if (names.find(i->name) == names.end())
+            v.attrs->push_back(*i);
     }
 }
 
@@ -761,13 +768,15 @@ static void prim_listToAttrs(EvalState & state, Value * * args, Value & v)
         Bindings::iterator j = v2.attrs->find(state.sName);
         if (j == v2.attrs->end())
             throw TypeError("`name' attribute missing in a call to `listToAttrs'");
-        string name = state.forceStringNoCtx(*j->second.value);
+        string name = state.forceStringNoCtx(*j->value);
         
         Bindings::iterator j2 = v2.attrs->find(state.symbols.create("value"));
         if (j2 == v2.attrs->end())
             throw TypeError("`value' attribute missing in a call to `listToAttrs'");
 
-        (*v.attrs)[state.symbols.create(name)] = j2->second;
+        Attr & a = (*v.attrs)[state.symbols.create(name)];
+        a.value = j2->value;
+        a.pos = j2->pos;
     }
 }
 
@@ -783,9 +792,9 @@ static void prim_intersectAttrs(EvalState & state, Value * * args, Value & v)
     state.mkAttrs(v);
 
     foreach (Bindings::iterator, i, *args[0]->attrs) {
-        Bindings::iterator j = args[1]->attrs->find(i->first);
+        Bindings::iterator j = args[1]->attrs->find(i->name);
         if (j != args[1]->attrs->end())
-            (*v.attrs)[j->first] = j->second;
+            v.attrs->push_back(*j);
     }
 }
 
diff --git a/src/libexpr/symbol-table.hh b/src/libexpr/symbol-table.hh
index 20ebe5fed..976117a20 100644
--- a/src/libexpr/symbol-table.hh
+++ b/src/libexpr/symbol-table.hh
@@ -28,6 +28,8 @@ private:
     friend class SymbolTable;
 
 public:
+    Symbol() : s(0) { };
+    
     bool operator == (const Symbol & s2) const
     {
         return s == s2.s;
diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc
index 1bb200fb8..4f9b62d5f 100644
--- a/src/libexpr/value-to-xml.cc
+++ b/src/libexpr/value-to-xml.cc
@@ -34,7 +34,7 @@ static void showAttrs(EvalState & state, bool strict, bool location,
     StringSet names;
     
     foreach (Bindings::iterator, i, attrs)
-        names.insert(i->first);
+        names.insert(i->name);
     
     foreach (StringSet::iterator, i, names) {
         Attr & a(attrs[state.symbols.create(*i)]);
@@ -90,16 +90,16 @@ static void printValueAsXML(EvalState & state, bool strict, bool location,
                 Path drvPath;
                 a = v.attrs->find(state.sDrvPath);
                 if (a != v.attrs->end()) {
-                    if (strict) state.forceValue(*a->second.value);
-                    if (a->second.value->type == tString)
-                        xmlAttrs["drvPath"] = drvPath = a->second.value->string.s;
+                    if (strict) state.forceValue(*a->value);
+                    if (a->value->type == tString)
+                        xmlAttrs["drvPath"] = drvPath = a->value->string.s;
                 }
         
                 a = v.attrs->find(state.sOutPath);
                 if (a != v.attrs->end()) {
-                    if (strict) state.forceValue(*a->second.value);
-                    if (a->second.value->type == tString)
-                        xmlAttrs["outPath"] = a->second.value->string.s;
+                    if (strict) state.forceValue(*a->value);
+                    if (a->value->type == tString)
+                        xmlAttrs["outPath"] = a->value->string.s;
                 }
 
                 XMLOpenElement _(doc, "derivation", xmlAttrs);

From 2dc6d5094183edee523a48d449eab1a376e839a2 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Sun, 24 Oct 2010 14:20:02 +0000
Subject: [PATCH 14/23] * Don't create thunks for variable lookups (if
 possible).  This   significantly reduces the number of values allocated (e.g.
 from 8.7m   to 4.9m for the Bittorrent test).

---
 src/libexpr/eval.cc    | 71 +++++++++++++++++++++++++++++++++---------
 src/libexpr/eval.hh    | 10 ++----
 src/libexpr/primops.cc |  2 +-
 3 files changed, 60 insertions(+), 23 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 85394b768..124cf6498 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -316,6 +316,11 @@ Env & EvalState::allocEnv(unsigned int size)
     nrEnvs++;
     nrValuesInEnvs += size;
     Env * env = (Env *) GC_MALLOC(sizeof(Env) + size * sizeof(Value *));
+
+    /* Clear the values because maybeThunk() expects this. */
+    for (unsigned i = 0; i < size; ++i)
+        env->values[i] = 0;
+    
     return *env;
 }
 
@@ -346,12 +351,48 @@ void EvalState::mkAttrs(Value & v)
 }
 
 
+unsigned long nrThunks = 0;
+
+static inline void mkThunk(Value & v, Env & env, Expr * expr)
+{
+    v.type = tThunk;
+    v.thunk.env = &env;
+    v.thunk.expr = expr;
+    nrThunks++;
+}
+
+
 void EvalState::mkThunk_(Value & v, Expr * expr)
 {
     mkThunk(v, baseEnv, expr);
 }
 
 
+unsigned long nrAvoided = 0;
+
+/* Create a thunk for the delayed computation of the given expression
+   in the given environment.  But if the expression is a variable,
+   then look it up right away.  This significantly reduces the number
+   of thunks allocated. */
+Value * EvalState::maybeThunk(Env & env, Expr * expr)
+{
+    ExprVar * var;
+    /* Ignore variables from `withs' because they can throw an
+       exception. */
+    if ((var = dynamic_cast<ExprVar *>(expr))) {
+        Value * v = lookupVar(&env, var->info);
+        /* The value might not be initialised in the environment yet.
+           In that case, ignore it. */
+        if (v) { nrAvoided++; return v; }
+    }
+
+    Value * v = allocValue();
+    mkThunk(*v, env, expr);
+
+    return v;
+}
+
+
 void EvalState::cloneAttrs(Value & src, Value & dst)
 {
     mkAttrs(dst);
@@ -478,8 +519,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
         /* The recursive attributes are evaluated in the new
            environment. */
         foreach (Attrs::iterator, i, attrs) {
-            Value * vAttr = state.allocValue();
-            mkThunk(*vAttr, env2, i->second.first);
+            Value * vAttr = state.maybeThunk(env2, i->second.first);
             env2.values[displ++] = vAttr;
             v.attrs->push_back(nix::Attr(i->first, vAttr, &i->second.second));
         }
@@ -515,8 +555,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
     else {
         foreach (Attrs::iterator, i, attrs) {
             nix::Attr & a = (*v.attrs)[i->first];
-            a.value = state.allocValue();
-            mkThunk(*a.value, env, i->second.first);
+            a.value = state.maybeThunk(env, i->second.first);
             a.pos = &i->second.second;
         }
 
@@ -540,10 +579,8 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)
 
     /* The recursive attributes are evaluated in the new
        environment. */
-    foreach (ExprAttrs::Attrs::iterator, i, attrs->attrs) {
-        env2.values[displ] = state.allocValue();
-        mkThunk(*env2.values[displ++], env2, i->second.first);
-    }
+    foreach (ExprAttrs::Attrs::iterator, i, attrs->attrs)
+        env2.values[displ++] = state.maybeThunk(env2, i->second.first);
 
     /* The inherited attributes, on the other hand, are evaluated in
        the original environment. */
@@ -558,7 +595,7 @@ void ExprList::eval(EvalState & state, Env & env, Value & v)
 {
     state.mkList(v, elems.size());
     for (unsigned int n = 0; n < v.list.length; ++n)
-        mkThunk(*(v.list.elems[n] = state.allocValue()), env, elems[n]);
+        v.list.elems[n] = state.maybeThunk(env, elems[n]);
 }
 
 
@@ -570,10 +607,15 @@ void ExprVar::eval(EvalState & state, Env & env, Value & v)
 }
 
 
+unsigned long nrLookups = 0;
+unsigned long nrLookupSize = 0;
+
 void ExprSelect::eval(EvalState & state, Env & env, Value & v)
 {
+    nrLookups++;
     Value v2;
     state.evalAttrs(env, e, v2);
+    nrLookupSize += v2.attrs->size();
     Bindings::iterator i = v2.attrs->find(name);
     if (i == v2.attrs->end())
         throwEvalError("attribute `%1%' missing", name);
@@ -608,9 +650,7 @@ void ExprApp::eval(EvalState & state, Env & env, Value & v)
 {
     Value vFun;
     state.eval(env, e1, vFun);
-    Value * vArg = state.allocValue();
-    mkThunk(*vArg, env, e2);
-    state.callFunction(vFun, *vArg, v);
+    state.callFunction(vFun, *state.maybeThunk(env, e2), v);
 }
 
 
@@ -685,8 +725,7 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v)
             if (j == arg.attrs->end()) {
                 if (!i->def) throwTypeError("function at %1% called without required argument `%2%'",
                     fun.lambda.fun->pos, i->name);
-                env2.values[displ] = allocValue();
-                mkThunk(*env2.values[displ++], env2, i->def);
+                env2.values[displ++] = maybeThunk(env2, i->def);
             } else {
                 attrsUsed++;
                 env2.values[displ++] = j->value;
@@ -1156,6 +1195,10 @@ void EvalState::printStats()
     printMsg(v, format("  right-biased unions: %1%") % nrOpUpdates);
     printMsg(v, format("  values copied in right-biased unions: %1%") % nrOpUpdateValuesCopied);
     printMsg(v, format("  symbols in symbol table: %1%") % symbols.size());
+    printMsg(v, format("  number of thunks: %1%") % nrThunks);
+    printMsg(v, format("  number of thunks avoided: %1%") % nrAvoided);
+    printMsg(v, format("  number of attr lookups: %1%") % nrLookups);
+    printMsg(v, format("  attr lookup size: %1%") % nrLookupSize);
 }
 
 
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 2f1b3fa45..e1aa69dd3 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -169,14 +169,6 @@ static inline void mkBool(Value & v, bool b)
 }
 
 
-static inline void mkThunk(Value & v, Env & env, Expr * expr)
-{
-    v.type = tThunk;
-    v.thunk.env = &env;
-    v.thunk.expr = expr;
-}
-
-
 static inline void mkApp(Value & v, Value & left, Value & right)
 {
     v.type = tApp;
@@ -325,6 +317,8 @@ public:
     void mkList(Value & v, unsigned int length);
     void mkAttrs(Value & v);
     void mkThunk_(Value & v, Expr * expr);
+
+    Value * maybeThunk(Env & env, Expr * expr);
     
     void cloneAttrs(Value & src, Value & dst);
 
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 20b839508..3e32dd3ff 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -1070,7 +1070,7 @@ void EvalState::createBaseEnv()
     /* Add a wrapper around the derivation primop that computes the
        `drvPath' and `outPath' attributes lazily. */
     string s = "attrs: let res = derivationStrict attrs; in attrs // { drvPath = res.drvPath; outPath = res.outPath; type = \"derivation\"; }";
-    mkThunk(v, baseEnv, parseExprFromString(*this, s, "/"));
+    mkThunk_(v, parseExprFromString(*this, s, "/"));
     addConstant("derivation", v);
 
     // Paths

From e0b7fb8f2710ec3012afe6b9d2096f770429a389 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Sun, 24 Oct 2010 19:52:33 +0000
Subject: [PATCH 15/23] * Keep attribute sets in sorted order to speed up
 attribute lookups. * Simplify the representation of attributes in the AST. *
 Change the behaviour of listToAttrs() in case of duplicate names.

---
 src/libexpr/common-opts.cc           |   7 +-
 src/libexpr/eval.cc                  | 149 +++++++++++++++------------
 src/libexpr/eval.hh                  |   6 +-
 src/libexpr/get-drvs.cc              |   2 +-
 src/libexpr/nixexpr.cc               |  63 +++++------
 src/libexpr/nixexpr.hh               |  20 ++--
 src/libexpr/parser.y                 |  32 +++---
 src/libexpr/primops.cc               |  32 ++++--
 src/libexpr/value-to-xml.cc          |   2 +-
 src/nix-env/nix-env.cc               |   4 +-
 src/nix-env/user-env.cc              |  15 ++-
 tests/lang/eval-okay-listtoattrs.nix |   2 +-
 12 files changed, 186 insertions(+), 148 deletions(-)

diff --git a/src/libexpr/common-opts.cc b/src/libexpr/common-opts.cc
index 6b393c07d..c364aa9cb 100644
--- a/src/libexpr/common-opts.cc
+++ b/src/libexpr/common-opts.cc
@@ -20,14 +20,17 @@ bool parseOptionArg(const string & arg, Strings::iterator & i,
     if (i == argsEnd) throw error;
     string value = *i++;
 
+    /* !!! check for duplicates! */
     Value * v = state.allocValue();
-    autoArgs[state.symbols.create(name)].value = v;
+    autoArgs.push_back(Attr(state.symbols.create(name), v));
 
     if (arg == "--arg")
         state.mkThunk_(*v, parseExprFromString(state, value, absPath(".")));
     else
         mkString(*v, value);
-    
+
+    autoArgs.sort(); // !!! inefficient
+
     return true;
 }
 
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 124cf6498..4aa8fc1e4 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -34,18 +34,16 @@ namespace nix {
 
 Bindings::iterator Bindings::find(const Symbol & name)
 {
-    iterator i = begin();
-    for ( ; i != end() && i->name != name; ++i) ;
-    return i;
+    Attr key(name, 0);
+    iterator i = lower_bound(begin(), end(), key);
+    if (i != end() && i->name == name) return i;
+    return end();
 }
 
 
-Attr & Bindings::operator [] (const Symbol & name)
+void Bindings::sort()
 {
-    iterator i = find(name);
-    if (i != end()) return *i;
-    push_back(Attr(name, 0));
-    return back();
+    std::sort(begin(), end());
 }
     
 
@@ -178,12 +176,12 @@ void EvalState::addPrimOp(const string & name,
 {
     Value * v = allocValue();
     string name2 = string(name, 0, 2) == "__" ? string(name, 2) : name;
-    Symbol sym = symbols.create(name);
+    Symbol sym = symbols.create(name2);
     v->type = tPrimOp;
     v->primOp = NEW PrimOp(primOp, arity, sym);
-    staticBaseEnv.vars[sym] = baseEnvDispl;
+    staticBaseEnv.vars[symbols.create(name)] = baseEnvDispl;
     baseEnv.values[baseEnvDispl++] = v;
-    baseEnv.values[0]->attrs->push_back(Attr(symbols.create(name2), v));
+    baseEnv.values[0]->attrs->push_back(Attr(sym, v));
 }
 
 
@@ -506,32 +504,38 @@ void ExprPath::eval(EvalState & state, Env & env, Value & v)
 
 void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
 {
-    state.mkAttrs(v);
+    state.mkAttrs(v); // !!! reserve size
 
     if (recursive) {
         /* Create a new environment that contains the attributes in
            this `rec'. */
-        Env & env2(state.allocEnv(attrs.size() + inherited.size()));
+        Env & env2(state.allocEnv(attrs.size()));
         env2.up = &env;
 
-        unsigned int displ = 0;
-        
+        AttrDefs::iterator overrides = attrs.find(state.sOverrides);
+        bool hasOverrides = overrides != attrs.end();
+
         /* The recursive attributes are evaluated in the new
-           environment. */
-        foreach (Attrs::iterator, i, attrs) {
-            Value * vAttr = state.maybeThunk(env2, i->second.first);
-            env2.values[displ++] = vAttr;
-            v.attrs->push_back(nix::Attr(i->first, vAttr, &i->second.second));
-        }
-
-        /* The inherited attributes, on the other hand, are
-           evaluated in the original environment. */
-        foreach (list<Inherited>::iterator, i, inherited) {
-            Value * vAttr = state.lookupVar(&env, i->first);
-            env2.values[displ++] = vAttr;
-            v.attrs->push_back(nix::Attr(i->first.name, vAttr, &i->second));
-        }
-
+           environment, while the inherited attributes are evaluated
+           in the original environment. */
+        unsigned int displ = 0;
+        foreach (AttrDefs::iterator, i, attrs)
+            if (i->second.inherited) {
+                /* !!! handle overrides? */
+                Value * vAttr = state.lookupVar(&env, i->second.var);
+                env2.values[displ++] = vAttr;
+                v.attrs->push_back(Attr(i->first, vAttr, &i->second.pos));
+            } else {
+                Value * vAttr;
+                if (hasOverrides) {
+                    vAttr = state.allocValue();
+                    mkThunk(*vAttr, env2, i->second.e);
+                } else
+                    vAttr = state.maybeThunk(env2, i->second.e);
+                env2.values[displ++] = vAttr;
+                v.attrs->push_back(Attr(i->first, vAttr, &i->second.pos));
+            }
+        
         /* If the rec contains an attribute called `__overrides', then
            evaluate it, and add the attributes in that set to the rec.
            This allows overriding of recursive attributes, which is
@@ -540,30 +544,27 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
            still reference the original value, because that value has
            been substituted into the bodies of the other attributes.
            Hence we need __overrides.) */
-        Bindings::iterator overrides = v.attrs->find(state.sOverrides);
-        if (overrides != v.attrs->end()) {
-            state.forceAttrs(*overrides->value);
-            foreach (Bindings::iterator, i, *overrides->value->attrs) {
-                nix::Attr & a = (*v.attrs)[i->name];
-                if (a.value)
-                    env2.values[displs[i->name]] = i->value;
-                a = *i;
+        if (hasOverrides) {
+            Value * vOverrides = (*v.attrs)[overrides->second.displ].value;
+            state.forceAttrs(*vOverrides);
+            foreach (Bindings::iterator, i, *vOverrides->attrs) {
+                AttrDefs::iterator j = attrs.find(i->name);
+                if (j != attrs.end()) {
+                    (*v.attrs)[j->second.displ] = *i;
+                    env2.values[j->second.displ] = i->value;
+                } else
+                    v.attrs->push_back(*i);
             }
+            v.attrs->sort();
         }
     }
 
     else {
-        foreach (Attrs::iterator, i, attrs) {
-            nix::Attr & a = (*v.attrs)[i->first];
-            a.value = state.maybeThunk(env, i->second.first);
-            a.pos = &i->second.second;
-        }
-
-        foreach (list<Inherited>::iterator, i, inherited) {
-            nix::Attr & a = (*v.attrs)[i->first.name];
-            a.value = state.lookupVar(&env, i->first);
-            a.pos = &i->second;
-        }
+        foreach (AttrDefs::iterator, i, attrs)
+            if (i->second.inherited)
+                v.attrs->push_back(Attr(i->first, state.lookupVar(&env, i->second.var), &i->second.pos));
+            else
+                v.attrs->push_back(Attr(i->first, state.maybeThunk(env, i->second.e), &i->second.pos));
     }
 }
 
@@ -572,20 +573,18 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)
 {
     /* Create a new environment that contains the attributes in this
        `let'. */
-    Env & env2(state.allocEnv(attrs->attrs.size() + attrs->inherited.size()));
+    Env & env2(state.allocEnv(attrs->attrs.size()));
     env2.up = &env;
 
-    unsigned int displ = 0;
-
-    /* The recursive attributes are evaluated in the new
+    /* The recursive attributes are evaluated in the new environment,
+       while the inherited attributes are evaluated in the original
        environment. */
-    foreach (ExprAttrs::Attrs::iterator, i, attrs->attrs)
-        env2.values[displ++] = state.maybeThunk(env2, i->second.first);
-
-    /* The inherited attributes, on the other hand, are evaluated in
-       the original environment. */
-    foreach (list<ExprAttrs::Inherited>::iterator, i, attrs->inherited)
-        env2.values[displ++] = state.lookupVar(&env, i->first);
+    unsigned int displ = 0;
+    foreach (ExprAttrs::AttrDefs::iterator, i, attrs->attrs)
+        if (i->second.inherited)
+            env2.values[displ++] = state.lookupVar(&env, i->second.var);
+        else
+            env2.values[displ++] = state.maybeThunk(env2, i->second.e);
 
     state.eval(env2, body, v);
 }
@@ -736,7 +735,7 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v)
            argument (unless the attribute match specifies a `...').
            TODO: show the names of the expected/unexpected
            arguments. */
-        if (!fun.lambda.fun->formals->ellipsis && attrsUsed != arg.attrs->size())
+        if (!fun.lambda.fun->formals->ellipsis && attrsUsed != arg.attrs->size()) 
             throwTypeError("function at %1% called with unexpected argument", fun.lambda.fun->pos);
     }
 
@@ -764,11 +763,13 @@ void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res)
     foreach (Formals::Formals_::iterator, i, fun.lambda.fun->formals->formals) {
         Bindings::iterator j = args.find(i->name);
         if (j != args.end())
-            (*actualArgs.attrs)[i->name] = *j;
+            actualArgs.attrs->push_back(*j);
         else if (!i->def)
             throwTypeError("cannot auto-call a function that has an argument without a default value (`%1%')", i->name);
     }
 
+    actualArgs.attrs->sort();
+
     callFunction(fun, actualArgs, res);
 }
 
@@ -851,12 +852,28 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v)
     if (v1.attrs->size() == 0) { v = v2; return; }
     if (v2.attrs->size() == 0) { v = v1; return; }
 
-    state.cloneAttrs(v1, v);
+    state.mkAttrs(v);
 
-    /* !!! fix */
-    foreach (Bindings::iterator, i, *v2.attrs)
-        (*v.attrs)[i->name] = *i;
+    /* Merge the attribute sets, preferring values from the second
+       set.  Make sure to keep the resulting vector in sorted
+       order. */
+    Bindings::iterator i = v1.attrs->begin();
+    Bindings::iterator j = v2.attrs->begin();
 
+    while (i != v1.attrs->end() && j != v2.attrs->end()) {
+        if (i->name == j->name) {
+            v.attrs->push_back(*j);
+            ++i; ++j;
+        }
+        else if (i->name < j->name)
+            v.attrs->push_back(*i++);
+        else
+            v.attrs->push_back(*j++);
+    }
+
+    while (i != v1.attrs->end()) v.attrs->push_back(*i++);
+    while (j != v2.attrs->end()) v.attrs->push_back(*j++);
+    
     state.nrOpUpdateValuesCopied += v.attrs->size();
 }
 
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index e1aa69dd3..7f801d112 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -34,7 +34,7 @@ class Bindings : public BindingsBase
 {
 public:
     iterator find(const Symbol & name);
-    Attr & operator [] (const Symbol & name);
+    void sort();
 };
 
 
@@ -142,6 +142,10 @@ struct Attr
     Attr(Symbol name, Value * value, Pos * pos = &noPos)
         : name(name), value(value), pos(pos) { };
     Attr() : pos(&noPos) { };
+    bool operator < (const Attr & a) const
+    {
+        return name < a.name;
+    }
 };
 
 
diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc
index 312c2cd40..a28a705d2 100644
--- a/src/libexpr/get-drvs.cc
+++ b/src/libexpr/get-drvs.cc
@@ -168,7 +168,7 @@ static void getDerivations(EvalState & state, Value & vIn,
         foreach (SortedSymbols::iterator, i, attrs) {
             startNest(nest, lvlDebug, format("evaluating attribute `%1%'") % i->first);
             string pathPrefix2 = addToPath(pathPrefix, i->first);
-            Value & v2(*(*v.attrs)[i->second].value);
+            Value & v2(*v.attrs->find(i->second)->value);
             if (combineChannels)
                 getDerivations(state, v2, pathPrefix2, autoArgs, drvs, done);
             else if (getDerivation(state, v2, pathPrefix2, drvs, done)) {
diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc
index 9f2ea7883..147f50853 100644
--- a/src/libexpr/nixexpr.cc
+++ b/src/libexpr/nixexpr.cc
@@ -55,10 +55,11 @@ void ExprAttrs::show(std::ostream & str)
 {
     if (recursive) str << "rec ";
     str << "{ ";
-    foreach (list<Inherited>::iterator, i, inherited)
-        str << "inherit " << i->first.name << "; ";
-    foreach (Attrs::iterator, i, attrs)
-        str << i->first << " = " << *i->second.first << "; ";
+    foreach (AttrDefs::iterator, i, attrs)
+        if (i->second.inherited)
+            str << "inherit " << i->first << " " << "; ";
+        else
+            str << i->first << " = " << *i->second.e << "; ";
     str << "}";
 }
 
@@ -91,10 +92,11 @@ void ExprLambda::show(std::ostream & str)
 void ExprLet::show(std::ostream & str)
 {
     str << "let ";
-    foreach (list<ExprAttrs::Inherited>::iterator, i, attrs->inherited)
-        str << "inherit " << i->first.name << "; ";
-    foreach (ExprAttrs::Attrs::iterator, i, attrs->attrs)
-        str << i->first << " = " << *i->second.first << "; ";
+    foreach (ExprAttrs::AttrDefs::iterator, i, attrs->attrs)
+        if (i->second.inherited)
+            str << "inherit " << i->first << "; ";
+        else
+            str << i->first << " = " << *i->second.e << "; ";
     str << "in " << *body;
 }
 
@@ -211,26 +213,18 @@ void ExprAttrs::bindVars(const StaticEnv & env)
         StaticEnv newEnv(false, &env);
     
         unsigned int displ = 0;
-
-        foreach (ExprAttrs::Attrs::iterator, i, attrs)
-            displs[i->first] = newEnv.vars[i->first] = displ++;
-
-        foreach (list<Inherited>::iterator, i, inherited) {
-            displs[i->first.name] = newEnv.vars[i->first.name] = displ++;
-            i->first.bind(env);
-        }
-
-        foreach (ExprAttrs::Attrs::iterator, i, attrs)
-            i->second.first->bindVars(newEnv);
+        foreach (AttrDefs::iterator, i, attrs)
+            newEnv.vars[i->first] = i->second.displ = displ++;
+        
+        foreach (AttrDefs::iterator, i, attrs)
+            if (i->second.inherited) i->second.var.bind(env);
+            else i->second.e->bindVars(newEnv);
     }
 
-    else {
-        foreach (ExprAttrs::Attrs::iterator, i, attrs)
-            i->second.first->bindVars(env);
-
-        foreach (list<Inherited>::iterator, i, inherited)
-            i->first.bind(env);
-    }
+    else
+        foreach (AttrDefs::iterator, i, attrs)
+            if (i->second.inherited) i->second.var.bind(env);
+            else i->second.e->bindVars(env);
 }
 
 void ExprList::bindVars(const StaticEnv & env)
@@ -263,17 +257,12 @@ void ExprLet::bindVars(const StaticEnv & env)
     StaticEnv newEnv(false, &env);
     
     unsigned int displ = 0;
-
-    foreach (ExprAttrs::Attrs::iterator, i, attrs->attrs)
-        newEnv.vars[i->first] = displ++;
-
-    foreach (list<ExprAttrs::Inherited>::iterator, i, attrs->inherited) {
-        newEnv.vars[i->first.name] = displ++;
-        i->first.bind(env);
-    }
-
-    foreach (ExprAttrs::Attrs::iterator, i, attrs->attrs)
-        i->second.first->bindVars(newEnv);
+    foreach (ExprAttrs::AttrDefs::iterator, i, attrs->attrs)
+        newEnv.vars[i->first] = i->second.displ = displ++;
+    
+    foreach (ExprAttrs::AttrDefs::iterator, i, attrs->attrs)
+        if (i->second.inherited) i->second.var.bind(env);
+        else i->second.e->bindVars(newEnv);
     
     body->bindVars(newEnv);
 }
diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh
index 1aa3df368..8f976f1de 100644
--- a/src/libexpr/nixexpr.hh
+++ b/src/libexpr/nixexpr.hh
@@ -101,6 +101,7 @@ struct VarRef
     unsigned int level;
     unsigned int displ;
 
+    VarRef() { };
     VarRef(const Symbol & name) : name(name) { };
     void bind(const StaticEnv & env);
 };
@@ -131,13 +132,18 @@ struct ExprOpHasAttr : Expr
 struct ExprAttrs : Expr
 {
     bool recursive;
-    typedef std::pair<Expr *, Pos> Attr;
-    typedef std::pair<VarRef, Pos> Inherited;
-    typedef std::map<Symbol, Attr> Attrs;
-    Attrs attrs;
-    list<Inherited> inherited;
-    std::map<Symbol, Pos> attrNames; // used during parsing
-    std::map<Symbol, unsigned int> displs;
+    struct AttrDef {
+        bool inherited;
+        Expr * e; // if not inherited
+        VarRef var; // if inherited
+        Pos pos;
+        unsigned int displ; // displacement
+        AttrDef(Expr * e, const Pos & pos) : inherited(false), e(e), pos(pos) { };
+        AttrDef(const Symbol & name, const Pos & pos) : inherited(true), var(name), pos(pos) { };
+        AttrDef() { };
+    };
+    typedef std::map<Symbol, AttrDef> AttrDefs;
+    AttrDefs attrs;
     ExprAttrs() : recursive(false) { };
     COMMON_METHODS
 };
diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y
index 3a72a4ade..c6d29b6ca 100644
--- a/src/libexpr/parser.y
+++ b/src/libexpr/parser.y
@@ -93,20 +93,20 @@ static void addAttr(ExprAttrs * attrs, const vector<Symbol> & attrPath,
     unsigned int n = 0;
     foreach (vector<Symbol>::const_iterator, i, attrPath) {
         n++;
-        ExprAttrs::Attrs::iterator j = attrs->attrs.find(*i);
+        ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(*i);
         if (j != attrs->attrs.end()) {
-            ExprAttrs * attrs2 = dynamic_cast<ExprAttrs *>(j->second.first);
-            if (!attrs2 || n == attrPath.size()) dupAttr(attrPath, pos, j->second.second);
-            attrs = attrs2;
+            if (!j->second.inherited) {
+                ExprAttrs * attrs2 = dynamic_cast<ExprAttrs *>(j->second.e);
+                if (!attrs2 || n == attrPath.size()) dupAttr(attrPath, pos, j->second.pos);
+                attrs = attrs2;
+            } else
+                dupAttr(attrPath, pos, j->second.pos);
         } else {
-            if (attrs->attrNames.find(*i) != attrs->attrNames.end())
-                dupAttr(attrPath, pos, attrs->attrNames[*i]);
-            attrs->attrNames[*i] = pos;
             if (n == attrPath.size())
-                attrs->attrs[*i] = ExprAttrs::Attr(e, pos);
+                attrs->attrs[*i] = ExprAttrs::AttrDef(e, pos);
             else {
                 ExprAttrs * nested = new ExprAttrs;
-                attrs->attrs[*i] = ExprAttrs::Attr(nested, pos);
+                attrs->attrs[*i] = ExprAttrs::AttrDef(nested, pos);
                 attrs = nested;
             }
         }
@@ -383,21 +383,19 @@ binds
   | binds INHERIT ids ';'
     { $$ = $1;
       foreach (vector<Symbol>::iterator, i, *$3) {
-          if ($$->attrNames.find(*i) != $$->attrNames.end())
-              dupAttr(*i, makeCurPos(@3, data), $$->attrNames[*i]);
+          if ($$->attrs.find(*i) != $$->attrs.end())
+              dupAttr(*i, makeCurPos(@3, data), $$->attrs[*i].pos);
           Pos pos = makeCurPos(@3, data);
-          $$->inherited.push_back(ExprAttrs::Inherited(*i, pos));
-          $$->attrNames[*i] = pos;
+          $$->attrs[*i] = ExprAttrs::AttrDef(*i, pos);
       }
     }
   | binds INHERIT '(' expr ')' ids ';'
     { $$ = $1;
       /* !!! Should ensure sharing of the expression in $4. */
       foreach (vector<Symbol>::iterator, i, *$6) {
-          if ($$->attrNames.find(*i) != $$->attrNames.end())
-              dupAttr(*i, makeCurPos(@6, data), $$->attrNames[*i]);
-          $$->attrs[*i] = ExprAttrs::Attr(new ExprSelect($4, *i), makeCurPos(@6, data));
-          $$->attrNames[*i] = makeCurPos(@6, data);
+          if ($$->attrs.find(*i) != $$->attrs.end())
+              dupAttr(*i, makeCurPos(@6, data), $$->attrs[*i].pos);
+          $$->attrs[*i] = ExprAttrs::AttrDef(new ExprSelect($4, *i), makeCurPos(@6, data));
       }}
 
   | { $$ = new ExprAttrs; }
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 3e32dd3ff..7c713f384 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -209,14 +209,13 @@ static void prim_tryEval(EvalState & state, Value * * args, Value & v)
     state.mkAttrs(v);
     try {
         state.forceValue(*args[0]);
-        printMsg(lvlError, format("%1%") % *args[0]);
-        (*v.attrs)[state.symbols.create("value")].value = args[0];
+        v.attrs->push_back(Attr(state.symbols.create("value"), args[0]));
         mkBool(*state.allocAttr(v, state.symbols.create("success")), true);
-        printMsg(lvlError, format("%1%") % v);
     } catch (AssertionError & e) {
         mkBool(*state.allocAttr(v, state.symbols.create("value")), false);
         mkBool(*state.allocAttr(v, state.symbols.create("success")), false);
     }
+    v.attrs->sort();
 }
 
 
@@ -488,6 +487,7 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
     state.mkAttrs(v);
     mkString(*state.allocAttr(v, state.sOutPath), outPath, singleton<PathSet>(drvPath));
     mkString(*state.allocAttr(v, state.sDrvPath), drvPath, singleton<PathSet>("=" + drvPath));
+    v.attrs->sort();
 }
 
 
@@ -742,7 +742,9 @@ static void prim_removeAttrs(EvalState & state, Value * * args, Value & v)
         names.insert(state.symbols.create(args[1]->list.elems[i]->string.s));
     }
 
-    /* Copy all attributes not in that set. */
+    /* Copy all attributes not in that set.  Note that we don't need
+       to sort v.attrs because it's a subset of an already sorted
+       vector. */
     state.mkAttrs(v);
     foreach (Bindings::iterator, i, *args[0]->attrs) {
         if (names.find(i->name) == names.end())
@@ -761,6 +763,8 @@ static void prim_listToAttrs(EvalState & state, Value * * args, Value & v)
 
     state.mkAttrs(v);
 
+    std::set<Symbol> seen;
+
     for (unsigned int i = 0; i < args[0]->list.length; ++i) {
         Value & v2(*args[0]->list.elems[i]);
         state.forceAttrs(v2);
@@ -774,10 +778,15 @@ static void prim_listToAttrs(EvalState & state, Value * * args, Value & v)
         if (j2 == v2.attrs->end())
             throw TypeError("`value' attribute missing in a call to `listToAttrs'");
 
-        Attr & a = (*v.attrs)[state.symbols.create(name)];
-        a.value = j2->value;
-        a.pos = j2->pos;
+        Symbol sym = state.symbols.create(name);
+        if (seen.find(sym) == seen.end()) {
+            v.attrs->push_back(Attr(sym, j2->value, j2->pos));
+            seen.insert(sym);
+        }
+        /* !!! Throw an error if `name' already exists? */
     }
+
+    v.attrs->sort();
 }
 
 
@@ -825,6 +834,8 @@ static void prim_functionArgs(EvalState & state, Value * * args, Value & v)
     foreach (Formals::Formals_::iterator, i, args[0]->lambda.fun->formals->formals)
         // !!! should optimise booleans (allocate only once)
         mkBool(*state.allocAttr(v, i->name), i->def);
+
+    v.attrs->sort();
 }
 
 
@@ -1007,6 +1018,7 @@ static void prim_parseDrvName(EvalState & state, Value * * args, Value & v)
     state.mkAttrs(v);
     mkString(*state.allocAttr(v, state.sName), parsed.name);
     mkString(*state.allocAttr(v, state.symbols.create("version")), parsed.version);
+    v.attrs->sort();
 }
 
 
@@ -1119,7 +1131,11 @@ void EvalState::createBaseEnv()
 
     // Versions
     addPrimOp("__parseDrvName", 1, prim_parseDrvName);
-    addPrimOp("__compareVersions", 2, prim_compareVersions);    
+    addPrimOp("__compareVersions", 2, prim_compareVersions);
+
+    /* Now that we've added all primops, sort the `builtins' attribute
+       set, because attribute lookups expect it to be sorted. */
+    baseEnv.values[0]->attrs->sort();
 }
 
 
diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc
index 4f9b62d5f..bc63f7762 100644
--- a/src/libexpr/value-to-xml.cc
+++ b/src/libexpr/value-to-xml.cc
@@ -37,7 +37,7 @@ static void showAttrs(EvalState & state, bool strict, bool location,
         names.insert(i->name);
     
     foreach (StringSet::iterator, i, names) {
-        Attr & a(attrs[state.symbols.create(*i)]);
+        Attr & a(*attrs.find(state.symbols.create(*i)));
         
         XMLAttrs xmlAttrs;
         xmlAttrs["name"] = *i;
diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc
index 4895c68e9..58bc9af12 100644
--- a/src/nix-env/nix-env.cc
+++ b/src/nix-env/nix-env.cc
@@ -132,7 +132,7 @@ static void getAllExprs(EvalState & state,
             if (hasSuffix(attrName, ".nix"))
                 attrName = string(attrName, 0, attrName.size() - 4);
             attrs.attrs[state.symbols.create(attrName)] =
-                ExprAttrs::Attr(parseExprFromFile(state, absPath(path2)), noPos);
+                ExprAttrs::AttrDef(parseExprFromFile(state, absPath(path2)), noPos);
         }
         else
             /* `path2' is a directory (with no default.nix in it);
@@ -154,7 +154,7 @@ static Expr * loadSourceExpr(EvalState & state, const Path & path)
        some system-wide directory). */
     ExprAttrs * attrs = new ExprAttrs;
     attrs->attrs[state.symbols.create("_combineChannels")] =
-        ExprAttrs::Attr(new ExprList(), noPos);
+        ExprAttrs::AttrDef(new ExprList(), noPos);
     getAllExprs(state, path, *attrs);
     return attrs;
 }
diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc
index 34e00b7c2..acd866197 100644
--- a/src/nix-env/user-env.cc
+++ b/src/nix-env/user-env.cc
@@ -69,13 +69,14 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
         mkString(*state.allocAttr(v, state.sOutPath), i->queryOutPath(state));
         if (drvPath != "")
             mkString(*state.allocAttr(v, state.sDrvPath), i->queryDrvPath(state));
-        
-        state.mkAttrs(*state.allocAttr(v, state.sMeta));
-        
+
+        Value & vMeta = *state.allocAttr(v, state.sMeta);
+        state.mkAttrs(vMeta);
+
         MetaInfo meta = i->queryMetaInfo(state);
 
         foreach (MetaInfo::const_iterator, j, meta) {
-            Value & v2(*state.allocAttr(*(*v.attrs)[state.sMeta].value, state.symbols.create(j->first)));
+            Value & v2(*state.allocAttr(vMeta, state.symbols.create(j->first)));
             switch (j->second.type) {
                 case MetaValue::tpInt: mkInt(v2, j->second.intValue); break;
                 case MetaValue::tpString: mkString(v2, j->second.stringValue); break;
@@ -92,6 +93,9 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
             }
         }
     
+        vMeta.attrs->sort();
+        v.attrs->sort();
+        
         /* This is only necessary when installing store paths, e.g.,
            `nix-env -i /nix/store/abcd...-foo'. */
         store->addTempRoot(i->queryOutPath(state));
@@ -118,7 +122,8 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
     mkString(*state.allocAttr(args, state.sSystem), thisSystem);
     mkString(*state.allocAttr(args, state.symbols.create("manifest")),
         manifestFile, singleton<PathSet>(manifestFile));
-    (*args.attrs)[state.symbols.create("derivations")].value = &manifest;
+    args.attrs->push_back(Attr(state.symbols.create("derivations"), &manifest));
+    args.attrs->sort();
     mkApp(topLevel, envBuilder, args);
         
     /* Evaluate it. */
diff --git a/tests/lang/eval-okay-listtoattrs.nix b/tests/lang/eval-okay-listtoattrs.nix
index 023c1d84e..4186e029b 100644
--- a/tests/lang/eval-okay-listtoattrs.nix
+++ b/tests/lang/eval-okay-listtoattrs.nix
@@ -7,5 +7,5 @@ let
   a = builtins.listToAttrs list;
   b = builtins.listToAttrs ( list ++ list );
   r = builtins.listToAttrs [ (asi "result" [ a b ]) ( asi "throw" (throw "this should not be thrown")) ];
-  x = builtins.listToAttrs [ (asi "foo" "bla") (asi "foo" "bar") ];
+  x = builtins.listToAttrs [ (asi "foo" "bar") (asi "foo" "bla") ];
 in concat (map (x: x.a) r.result) + x.foo

From 43535499f38acc04367eeb4dd0d9938e9f8666f8 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Sun, 24 Oct 2010 20:09:37 +0000
Subject: [PATCH 16/23] * When allocating an attribute set, reserve enough
 space for all   elements.  This prevents the vector from having to resize
 itself.

---
 src/libexpr/eval.cc     | 16 +++++-----------
 src/libexpr/eval.hh     |  4 +---
 src/libexpr/primops.cc  | 23 ++++++++++++-----------
 src/nix-env/user-env.cc |  6 +++---
 4 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 4aa8fc1e4..919ebd4ba 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -340,11 +340,12 @@ void EvalState::mkList(Value & v, unsigned int length)
 }
 
 
-void EvalState::mkAttrs(Value & v)
+void EvalState::mkAttrs(Value & v, unsigned int expected)
 {
     clearValue(v);
     v.type = tAttrs;
     v.attrs = NEW Bindings;
+    v.attrs->reserve(expected);
     nrAttrsets++;
 }
 
@@ -391,13 +392,6 @@ Value * EvalState::maybeThunk(Env & env, Expr * expr)
 }
 
 
-void EvalState::cloneAttrs(Value & src, Value & dst)
-{
-    mkAttrs(dst);
-    *dst.attrs = *src.attrs;
-}
-
-
 void EvalState::evalFile(const Path & path, Value & v)
 {
     startNest(nest, lvlTalkative, format("evaluating file `%1%'") % path);
@@ -504,7 +498,7 @@ void ExprPath::eval(EvalState & state, Env & env, Value & v)
 
 void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
 {
-    state.mkAttrs(v); // !!! reserve size
+    state.mkAttrs(v, attrs.size());
 
     if (recursive) {
         /* Create a new environment that contains the attributes in
@@ -758,7 +752,7 @@ void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res)
     }
 
     Value actualArgs;
-    mkAttrs(actualArgs);
+    mkAttrs(actualArgs, fun.lambda.fun->formals->formals.size());
 
     foreach (Formals::Formals_::iterator, i, fun.lambda.fun->formals->formals) {
         Bindings::iterator j = args.find(i->name);
@@ -852,7 +846,7 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v)
     if (v1.attrs->size() == 0) { v = v2; return; }
     if (v2.attrs->size() == 0) { v = v1; return; }
 
-    state.mkAttrs(v);
+    state.mkAttrs(v, v1.attrs->size() + v2.attrs->size());
 
     /* Merge the attribute sets, preferring values from the second
        set.  Make sure to keep the resulting vector in sorted
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 7f801d112..7453ac189 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -319,13 +319,11 @@ public:
     Value * allocAttr(Value & vAttrs, const Symbol & name);
 
     void mkList(Value & v, unsigned int length);
-    void mkAttrs(Value & v);
+    void mkAttrs(Value & v, unsigned int expected);
     void mkThunk_(Value & v, Expr * expr);
 
     Value * maybeThunk(Env & env, Expr * expr);
     
-    void cloneAttrs(Value & src, Value & dst);
-
     /* Print statistics. */
     void printStats();
 
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 7c713f384..a4812de06 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -206,7 +206,7 @@ static void prim_addErrorContext(EvalState & state, Value * * args, Value & v)
  * else => {success=false; value=false;} */
 static void prim_tryEval(EvalState & state, Value * * args, Value & v)
 {
-    state.mkAttrs(v);
+    state.mkAttrs(v, 2);
     try {
         state.forceValue(*args[0]);
         v.attrs->push_back(Attr(state.symbols.create("value"), args[0]));
@@ -484,7 +484,7 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
     state.drvHashes[drvPath] = hashDerivationModulo(state, drv);
 
     /* !!! assumes a single output */
-    state.mkAttrs(v);
+    state.mkAttrs(v, 2);
     mkString(*state.allocAttr(v, state.sOutPath), outPath, singleton<PathSet>(drvPath));
     mkString(*state.allocAttr(v, state.sDrvPath), drvPath, singleton<PathSet>("=" + drvPath));
     v.attrs->sort();
@@ -745,7 +745,7 @@ static void prim_removeAttrs(EvalState & state, Value * * args, Value & v)
     /* Copy all attributes not in that set.  Note that we don't need
        to sort v.attrs because it's a subset of an already sorted
        vector. */
-    state.mkAttrs(v);
+    state.mkAttrs(v, args[0]->attrs->size());
     foreach (Bindings::iterator, i, *args[0]->attrs) {
         if (names.find(i->name) == names.end())
             v.attrs->push_back(*i);
@@ -761,7 +761,7 @@ static void prim_listToAttrs(EvalState & state, Value * * args, Value & v)
 {
     state.forceList(*args[0]);
 
-    state.mkAttrs(v);
+    state.mkAttrs(v, args[0]->list.length);
 
     std::set<Symbol> seen;
 
@@ -798,7 +798,7 @@ static void prim_intersectAttrs(EvalState & state, Value * * args, Value & v)
     state.forceAttrs(*args[0]);
     state.forceAttrs(*args[1]);
         
-    state.mkAttrs(v);
+    state.mkAttrs(v, std::min(args[0]->attrs->size(), args[1]->attrs->size()));
 
     foreach (Bindings::iterator, i, *args[0]->attrs) {
         Bindings::iterator j = args[1]->attrs->find(i->name);
@@ -827,14 +827,15 @@ static void prim_functionArgs(EvalState & state, Value * * args, Value & v)
     if (args[0]->type != tLambda)
         throw TypeError("`functionArgs' requires a function");
 
-    state.mkAttrs(v);
-
-    if (!args[0]->lambda.fun->matchAttrs) return;
+    if (!args[0]->lambda.fun->matchAttrs) {
+        state.mkAttrs(v, 0);
+        return;
+    }
 
+    state.mkAttrs(v, args[0]->lambda.fun->formals->formals.size());
     foreach (Formals::Formals_::iterator, i, args[0]->lambda.fun->formals->formals)
         // !!! should optimise booleans (allocate only once)
         mkBool(*state.allocAttr(v, i->name), i->def);
-
     v.attrs->sort();
 }
 
@@ -1015,7 +1016,7 @@ static void prim_parseDrvName(EvalState & state, Value * * args, Value & v)
 {
     string name = state.forceStringNoCtx(*args[0]);
     DrvName parsed(name);
-    state.mkAttrs(v);
+    state.mkAttrs(v, 2);
     mkString(*state.allocAttr(v, state.sName), parsed.name);
     mkString(*state.allocAttr(v, state.symbols.create("version")), parsed.version);
     v.attrs->sort();
@@ -1043,7 +1044,7 @@ void EvalState::createBaseEnv()
     Value v;
 
     /* `builtins' must be first! */
-    mkAttrs(v);
+    mkAttrs(v, 128);
     addConstant("builtins", v);
 
     mkBool(v, true);
diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc
index acd866197..865d24e2f 100644
--- a/src/nix-env/user-env.cc
+++ b/src/nix-env/user-env.cc
@@ -61,7 +61,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
 
         Value & v(*state.allocValue());
         manifest.list.elems[n++] = &v;
-        state.mkAttrs(v);
+        state.mkAttrs(v, 8);
 
         mkString(*state.allocAttr(v, state.sType), "derivation");
         mkString(*state.allocAttr(v, state.sName), i->name);
@@ -71,7 +71,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
             mkString(*state.allocAttr(v, state.sDrvPath), i->queryDrvPath(state));
 
         Value & vMeta = *state.allocAttr(v, state.sMeta);
-        state.mkAttrs(vMeta);
+        state.mkAttrs(vMeta, 16);
 
         MetaInfo meta = i->queryMetaInfo(state);
 
@@ -118,7 +118,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
     /* Construct a Nix expression that calls the user environment
        builder with the manifest as argument. */
     Value args, topLevel;
-    state.mkAttrs(args);
+    state.mkAttrs(args, 3);
     mkString(*state.allocAttr(args, state.sSystem), thisSystem);
     mkString(*state.allocAttr(args, state.symbols.create("manifest")),
         manifestFile, singleton<PathSet>(manifestFile));

From 11ccd44e95c16b9f7fcf51e75b511b1b6587397b Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Sun, 24 Oct 2010 21:48:59 +0000
Subject: [PATCH 17/23] * We need Bison 2.4 now.

---
 release.nix | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/release.nix b/release.nix
index 695521d39..a34a202ae 100644
--- a/release.nix
+++ b/release.nix
@@ -18,7 +18,7 @@ let
         inherit officialRelease;
 
         buildInputs =
-          [ curl bison flex2533 perl libxml2 libxslt w3m bzip2
+          [ curl bison24 flex2535 perl libxml2 libxslt w3m bzip2
             tetex dblatex nukeReferences
           ];
 
@@ -72,6 +72,7 @@ let
         configureFlags = ''
           --disable-init-state
           --with-bzip2=${bzip2}
+          --with-boehm-gc=${boehmgc}
         '';
       };
 

From 8a788e38ac7efc785ffe4fcf49a4e031c7784216 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Tue, 26 Oct 2010 10:47:02 +0000
Subject: [PATCH 18/23] * Install config.h.

---
 Makefile.am | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 2f39f90d6..cb1a321dd 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2,6 +2,8 @@ SUBDIRS = externals src scripts corepkgs doc misc tests
 EXTRA_DIST = substitute.mk nix.spec nix.spec.in bootstrap.sh \
   nix.conf.example NEWS version
 
+pkginclude_HEADERS = config.h
+
 include ./substitute.mk
 
 nix.spec: nix.spec.in

From e11e6fb1c6709ca3f0e596a7b1fb988df2fbd9b1 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Thu, 28 Oct 2010 12:29:40 +0000
Subject: [PATCH 19/23] * Handle out of memory condition.

---
 src/libexpr/eval.cc             | 10 +++++-----
 src/libmain/Makefile.am         |  2 +-
 src/libmain/shared.cc           | 20 ++++++++++++++++++++
 src/nix-env/Makefile.am         |  3 +--
 src/nix-instantiate/Makefile.am |  3 +--
 5 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 919ebd4ba..c36a679d3 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -259,9 +259,8 @@ void mkString(Value & v, const string & s, const PathSet & context)
     mkString(v, s.c_str());
     if (!context.empty()) {
         unsigned int n = 0;
-        v.string.context = (const char * *)
-            GC_MALLOC((context.size() + 1) * sizeof(char *));
-        foreach (PathSet::const_iterator, i, context) 
+        v.string.context = NEW const char *[context.size() + 1];
+        foreach (PathSet::const_iterator, i, context)  
             v.string.context[n++] = GC_STRDUP(i->c_str());
         v.string.context[n] = 0;
     }
@@ -305,7 +304,7 @@ Value * EvalState::lookupVar(Env * env, const VarRef & var)
 Value * EvalState::allocValue()
 {
     nrValues++;
-    return (Value *) GC_MALLOC(sizeof(Value));
+    return NEW Value;
 }
 
 
@@ -314,6 +313,7 @@ Env & EvalState::allocEnv(unsigned int size)
     nrEnvs++;
     nrValuesInEnvs += size;
     Env * env = (Env *) GC_MALLOC(sizeof(Env) + size * sizeof(Value *));
+    if (!env) throw std::bad_alloc();
 
     /* Clear the values because maybeThunk() expects this. */
     for (unsigned i = 0; i < size; ++i)
@@ -335,7 +335,7 @@ void EvalState::mkList(Value & v, unsigned int length)
 {
     v.type = tList;
     v.list.length = length;
-    v.list.elems = (Value * *) GC_MALLOC(length * sizeof(Value *));
+    v.list.elems = NEW Value *[length];
     nrListElems += length;
 }
 
diff --git a/src/libmain/Makefile.am b/src/libmain/Makefile.am
index a9ee66042..1a2146e04 100644
--- a/src/libmain/Makefile.am
+++ b/src/libmain/Makefile.am
@@ -2,7 +2,7 @@ pkglib_LTLIBRARIES = libmain.la
 
 libmain_la_SOURCES = shared.cc
 
-libmain_la_LIBADD = ../libstore/libstore.la
+libmain_la_LIBADD = ../libstore/libstore.la @boehmgc_lib@
 
 pkginclude_HEADERS = shared.hh
 
diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc
index eddc4e64b..440949bd2 100644
--- a/src/libmain/shared.cc
+++ b/src/libmain/shared.cc
@@ -13,6 +13,10 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+#if HAVE_BOEHMGC
+#include <gc/gc.h>
+#endif
+
 
 namespace nix {
 
@@ -314,6 +318,14 @@ static void setuidInit()
 }
 
 
+/* Called when the Boehm GC runs out of memory. */
+static void * oomHandler(size_t requested)
+{
+    /* Convert this to a proper C++ exception. */
+    throw std::bad_alloc();
+}
+
+
 }
 
 
@@ -335,6 +347,14 @@ int main(int argc, char * * argv)
 
     std::ios::sync_with_stdio(false);
 
+#if HAVE_BOEHMGC
+    /* Initialise the Boehm garbage collector.  This isn't necessary
+       on most platforms, but for portability we do it anyway. */
+    GC_INIT();
+
+    GC_oom_fn = oomHandler;
+#endif
+
     try {
         try {
             initAndRun(argc, argv);
diff --git a/src/nix-env/Makefile.am b/src/nix-env/Makefile.am
index a876b3eb3..7dfa7425a 100644
--- a/src/nix-env/Makefile.am
+++ b/src/nix-env/Makefile.am
@@ -4,8 +4,7 @@ nix_env_SOURCES = nix-env.cc profiles.cc profiles.hh user-env.cc user-env.hh hel
 
 nix_env_LDADD = ../libmain/libmain.la ../libexpr/libexpr.la \
  ../libstore/libstore.la ../libutil/libutil.la \
- ../boost/format/libformat.la @ADDITIONAL_NETWORK_LIBS@ \
- @boehmgc_lib@
+ ../boost/format/libformat.la @ADDITIONAL_NETWORK_LIBS@
 
 nix-env.o: help.txt.hh
 
diff --git a/src/nix-instantiate/Makefile.am b/src/nix-instantiate/Makefile.am
index bc9792818..a65907a8d 100644
--- a/src/nix-instantiate/Makefile.am
+++ b/src/nix-instantiate/Makefile.am
@@ -3,8 +3,7 @@ bin_PROGRAMS = nix-instantiate
 nix_instantiate_SOURCES = nix-instantiate.cc help.txt
 nix_instantiate_LDADD = ../libmain/libmain.la ../libexpr/libexpr.la \
  ../libstore/libstore.la ../libutil/libutil.la \
- ../boost/format/libformat.la @ADDITIONAL_NETWORK_LIBS@ \
- @boehmgc_lib@
+ ../boost/format/libformat.la @ADDITIONAL_NETWORK_LIBS@
 
 nix-instantiate.o: help.txt.hh
 

From 0c4828ea05798b9e070e233884739736115a830d Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Thu, 28 Oct 2010 12:50:01 +0000
Subject: [PATCH 20/23] * new(UseGC) is inexplicably slower than GC_MALLOC, so
 prefer the   latter.

---
 src/libexpr/eval.cc | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index c36a679d3..3ef17c36a 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -259,7 +259,8 @@ void mkString(Value & v, const string & s, const PathSet & context)
     mkString(v, s.c_str());
     if (!context.empty()) {
         unsigned int n = 0;
-        v.string.context = NEW const char *[context.size() + 1];
+        v.string.context = (const char * *)
+            GC_MALLOC((context.size() + 1) * sizeof(char *));
         foreach (PathSet::const_iterator, i, context)  
             v.string.context[n++] = GC_STRDUP(i->c_str());
         v.string.context[n] = 0;
@@ -304,7 +305,7 @@ Value * EvalState::lookupVar(Env * env, const VarRef & var)
 Value * EvalState::allocValue()
 {
     nrValues++;
-    return NEW Value;
+    return (Value *) GC_MALLOC(sizeof(Value));
 }
 
 
@@ -313,7 +314,6 @@ Env & EvalState::allocEnv(unsigned int size)
     nrEnvs++;
     nrValuesInEnvs += size;
     Env * env = (Env *) GC_MALLOC(sizeof(Env) + size * sizeof(Value *));
-    if (!env) throw std::bad_alloc();
 
     /* Clear the values because maybeThunk() expects this. */
     for (unsigned i = 0; i < size; ++i)
@@ -335,7 +335,7 @@ void EvalState::mkList(Value & v, unsigned int length)
 {
     v.type = tList;
     v.list.length = length;
-    v.list.elems = NEW Value *[length];
+    v.list.elems = (Value * *) GC_MALLOC(length * sizeof(Value *));
     nrListElems += length;
 }
 

From 14fbf85380b23efcc19c8479b65336fc7275d90b Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Fri, 29 Oct 2010 13:11:50 +0000
Subject: [PATCH 21/23] * Set libgc's initial heap size to 384 MiB to prevent
 garbage   collection in most cases (and therefore its performance overhead).

---
 src/libmain/shared.cc | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc
index 440949bd2..82309544a 100644
--- a/src/libmain/shared.cc
+++ b/src/libmain/shared.cc
@@ -353,6 +353,18 @@ int main(int argc, char * * argv)
     GC_INIT();
 
     GC_oom_fn = oomHandler;
+
+    /* Set the initial heap size to something fairly big (384 MiB) so
+       that in most cases we don't need to garbage collect at all.
+       (Collection has a fairly significant overhead, some.)  The heap
+       size can be overriden through libgc's GC_INITIAL_HEAP_SIZE
+       environment variable.  We should probably also provide a
+       nix.conf setting for this.  Note that GC_expand_hp() causes a
+       lot of virtual, but not physical (resident) memory to be
+       allocated.  This might be a problem on systems that don't
+       overcommit. */
+    if (!getenv("GC_INITIAL_HEAP_SIZE"))
+        GC_expand_hp(384000000);
 #endif
 
     try {

From 3d71c8013efa5d347b9767af54160b3d0fd9127b Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Fri, 29 Oct 2010 14:00:47 +0000
Subject: [PATCH 22/23] * Use pkgconfig to locate the Boehm GC (as suggested by
 Ludo), if   --enable-gc is given.

---
 configure.ac | 14 +++++++-------
 release.nix  |  6 +++---
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/configure.ac b/configure.ac
index 02eebf692..f108c53be 100644
--- a/configure.ac
+++ b/configure.ac
@@ -251,17 +251,17 @@ AC_SUBST(bzip2_bin_test)
 
 
 # Whether to use the Boehm garbage collector.
-AC_ARG_WITH(boehm-gc, AC_HELP_STRING([--with-boehm-gc=PATH],
-  [prefix of the Boehm GC package to enable garbage collection in the Nix expression evaluator]),
-  boehmgc=$withval, boehmgc=)
-if test -n "$boehmgc"; then
+AC_ARG_ENABLE(gc, AC_HELP_STRING([--enable-gc],
+  [enable garbage collection in the Nix expression evaluator (requires Boehm GC)]),
+  gc=$enableval, gc=)
+if test -n "$gc"; then
+  PKG_CHECK_MODULES([BDW_GC], [bdw-gc])
   boehmgc_lib="-L$boehmgc/lib -lgc"
-  CXXFLAGS="-I$boehmgc/include $CXXFLAGS"
+  CXXFLAGS="$BDW_GC_CFLAGS $CXXFLAGS"
   AC_DEFINE(HAVE_BOEHMGC, 1, [Whether to use the Boehm garbage collector.])
 fi
 AC_SUBST(boehmgc_lib)
-AC_SUBST(boehmgc_include)
-  
+
 
 AC_ARG_ENABLE(init-state, AC_HELP_STRING([--disable-init-state],
   [do not initialise DB etc. in `make install']),
diff --git a/release.nix b/release.nix
index a34a202ae..c89d79a7d 100644
--- a/release.nix
+++ b/release.nix
@@ -19,7 +19,7 @@ let
 
         buildInputs =
           [ curl bison24 flex2535 perl libxml2 libxslt w3m bzip2
-            tetex dblatex nukeReferences
+            tetex dblatex nukeReferences pkgconfig
           ];
 
         configureFlags = ''
@@ -67,12 +67,12 @@ let
         name = "nix";
         src = tarball;
 
-        buildInputs = [ curl perl bzip2 openssl ];
+        buildInputs = [ curl perl bzip2 openssl pkgconfig boehmgc ];
 
         configureFlags = ''
           --disable-init-state
           --with-bzip2=${bzip2}
-          --with-boehm-gc=${boehmgc}
+          --enable-gc
         '';
       };
 

From 26def5392f6f6364aa0939a2d4fc7705e786d38d Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Fri, 29 Oct 2010 14:44:02 +0000
Subject: [PATCH 23/23] * Document Boehm GC support.

---
 doc/manual/env-common.xml    | 11 +++++++++++
 doc/manual/installation.xml  |  7 +++++++
 doc/manual/release-notes.xml | 21 +++++++++++++++++++++
 src/libmain/shared.cc        |  2 +-
 4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/doc/manual/env-common.xml b/doc/manual/env-common.xml
index d67ef714d..99acc5949 100644
--- a/doc/manual/env-common.xml
+++ b/doc/manual/env-common.xml
@@ -271,6 +271,17 @@ $ mount -o bind /mnt/otherdisk/nix /nix</screen>
 
 </varlistentry>
 
+
+<varlistentry><term><envar>GC_INITIAL_HEAP_SIZE</envar></term>
+
+  <listitem><para>If Nix has been configured to use the Boehm garbage
+  collector, this variable sets the initial size of the heap in bytes.
+  It defaults to 384 MiB.  Setting it to a low value reduces memory
+  consumption, but will increase runtime due to the overhead of
+  garbage collection.</para></listitem>
+
+</varlistentry>
+
     
 </variablelist>
 
diff --git a/doc/manual/installation.xml b/doc/manual/installation.xml
index bc5e21f0d..87a6c446a 100644
--- a/doc/manual/installation.xml
+++ b/doc/manual/installation.xml
@@ -105,6 +105,13 @@ this packages.  Alternatively, if you already have it installed, you
 can use <command>configure</command>'s <option>--with-bzip2</option>
 options to point to their respective locations.</para>
 
+<para>Nix can optionally use the <link
+xlink:href="http://www.hpl.hp.com/personal/Hans_Boehm/gc/">Boehm
+garbage collector</link> to reduce the evaluator’s memory consumption.
+To enable it, install <literal>pkgconfig</literal> and the Boehm
+garbage collector, and pass the flag <option>--enable-gc</option> to
+<command>configure</command>.</para>
+
 </section>
 
 
diff --git a/doc/manual/release-notes.xml b/doc/manual/release-notes.xml
index 5b1c30bf8..1e579a37b 100644
--- a/doc/manual/release-notes.xml
+++ b/doc/manual/release-notes.xml
@@ -6,6 +6,27 @@
 
 
 
+<!--==================================================================-->
+
+<section xml:id="ssec-relnotes-1.0"><title>Release 1.0 (TBA)</title>
+
+<para>This release has the following improvements:</para>
+
+<itemizedlist>
+
+  <listitem>
+    <para>Nix can now optionally use the Boehm garbage collector.
+    This significantly reduces the Nix evaluator’s memory footprint,
+    especially when evaluating large NixOS system configurations.  It
+    can be enabled using the <option>--enable-gc</option> configure
+    option.</para>
+  </listitem>
+
+</itemizedlist>
+
+</section>
+
+
 <!--==================================================================-->
 
 <section xml:id="ssec-relnotes-0.16"><title>Release 0.16 (August 17, 2010)</title>
diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc
index 82309544a..29fc13e33 100644
--- a/src/libmain/shared.cc
+++ b/src/libmain/shared.cc
@@ -364,7 +364,7 @@ int main(int argc, char * * argv)
        allocated.  This might be a problem on systems that don't
        overcommit. */
     if (!getenv("GC_INITIAL_HEAP_SIZE"))
-        GC_expand_hp(384000000);
+        GC_expand_hp(384 * 1024 * 1024);
 #endif
 
     try {