Garbage-collect strings

src/libexpr/eval.cc

@@ -24,10 +24,13 @@ namespace nix {
 SymbolTable symbols;
 
 
+unsigned long stringBytes = 0;
+
 // FIXME
 static char * dupString(const char * s)
 {
     char * t;
+    stringBytes += strlen(s) + 1;
     t = strdup(s);
     if (!t) throw std::bad_alloc();
     return t;
@@ -52,6 +55,7 @@ static void printValue(std::ostream & str, std::set<const Value *> & active, con
         str << (v.boolean ? "true" : "false");
         break;
     case tShortString:
+    case tStaticString:
     case tLongString:
         str << "\"";
         for (const char * i = v.getString(); *i; i++)
@@ -141,7 +145,9 @@ string showType(const Value & v)
     switch (v.type) {
         case tInt: return "an integer";
         case tBool: return "a boolean";
-        case tShortString: return "a string";
+        case tShortString:
+        case tStaticString:
+            return "a string";
         case tLongString: return v.string.context ? "a string with context" : "a string";
         case tPath: return "a path";
         case tNull: return "null";
@@ -480,22 +486,14 @@ LocalNoInline(void addErrorPrefix(Error & e, const char * s, const string & s2,
 }
 
 
-void mkString(Value & v, const char * s)
-{
-    auto len = strlen(s); // FIXME: only need to know if > short
-    if (len < WORD_SIZE * 2 + Object::miscBytes)
-        v.setShortString(s);
-    else
-        mkStringNoCopy(v, dupString(s));
-}
-
-
 Value & mkString(Value & v, const string & s, const PathSet & context)
 {
     if (context.empty())
         mkString(v, s.c_str());
     else {
-        mkStringNoCopy(v, dupString(s.c_str()));
+        v.string.s = String::alloc(s.c_str());
+        v.string.context = nullptr;
+        v.type = tLongString;
         v.setContext(context);
     }
     return v;
@@ -1461,10 +1459,10 @@ string EvalState::forceStringNoCtx(Value & v, const Pos & pos)
         v.getContext(context);
         if (pos)
             throwEvalError("the string '%1%' is not allowed to refer to a store path (such as '%2%'), at %3%",
-                v.string._s, *context.begin(), pos);
+                v.getString(), *context.begin(), pos);
         else
             throwEvalError("the string '%1%' is not allowed to refer to a store path (such as '%2%')",
-                v.string._s, *context.begin());
+                v.getString(), *context.begin());
     }
     return s;
 }
@@ -1721,6 +1719,7 @@ void EvalState::printStats()
         topObj.attr("nrLookups", nrLookups);
         topObj.attr("nrPrimOpCalls", nrPrimOpCalls);
         topObj.attr("nrFunctionCalls", nrFunctionCalls);
+        topObj.attr("stringBytes", stringBytes);
 
         if (countCalls) {
             {
@@ -1766,6 +1765,7 @@ void EvalState::printStats()
 }
 
 
+// FIXME: move to gc.cc and make generic.
 size_t valueSize(Value & v)
 {
     std::set<const void *> seen;
@@ -1787,7 +1787,7 @@ size_t valueSize(Value & v)
 
         switch (v.type) {
         case tLongString:
-            sz += doString(v.string._s);
+            sz += v.string.s->words() * WORD_SIZE;
             break;
         case tPath:
             sz += doString(v.path);

src/libexpr/gc.cc

@@ -146,6 +146,7 @@ void GC::gc()
             break;
         }
 
+        case tString:
         case tContext:
         case tInt:
         case tBool:
@@ -153,11 +154,12 @@ void GC::gc()
         case tList0:
         case tFloat:
         case tShortString:
+        case tStaticString:
             break;
 
         case tLongString: {
             auto obj2 = (Value *) obj;
-            // FIXME: GC string
+            push(obj2->string.s);
             // See setContext().
             if (!(((ptrdiff_t) obj2->string.context) & 1))
                 push(obj2->string.context);
@@ -285,6 +287,9 @@ std::pair<size_t, size_t> GC::freeUnmarked(Arena & arena)
             case tFree:
                 objSize = ((Free *) obj)->words();
                 break;
+            case tString:
+                objSize = ((String *) obj)->words();
+                break;
             case tBindings:
                 objSize = ((Bindings *) obj)->words();
                 break;

src/libexpr/gc.hh

@@ -5,6 +5,7 @@
 #include <stack>
 #include <limits>
 #include <cassert>
+#include <cstring>
 
 //#define GC_DEBUG 1
 
@@ -16,6 +17,7 @@ enum Tag {
     tFree = 3,
 
     // Misc types
+    tString,
     tBindings,
     tValueList,
     tEnv,
@@ -27,6 +29,7 @@ enum Tag {
     tInt,
     tBool,
     tShortString,
+    tStaticString,
     tLongString,
     tPath,
     tNull,
@@ -436,4 +439,30 @@ struct Root
     }
 };
 
+struct String : Object
+{
+    char s[0];
+
+    String(size_t len, const char * src)
+        : Object(tString, len)
+    {
+        std::memcpy(s, src, len + 1);
+    }
+
+    size_t words() const { return wordsFor(getMisc()); }
+
+    /* Return the number of words needed to store a string of 'len'
+       characters (where 'len' excludes the terminator). */
+    static size_t wordsFor(size_t len)
+    {
+        return len / WORD_SIZE + 2;
+    }
+
+    static String * alloc(const char * src)
+    {
+        auto len = strlen(src);
+        return gc.alloc<String>(wordsFor(len), len, src);
+    }
+};
+
 }

src/libexpr/primops.cc

@@ -239,6 +239,7 @@ static void prim_typeOf(EvalState & state, const Pos & pos, Value * * args, Valu
         case tInt: t = "int"; break;
         case tBool: t = "bool"; break;
         case tShortString:
+        case tStaticString:
         case tLongString:
             t = "string"; break;
         case tPath: t = "path"; break;
@@ -937,15 +938,20 @@ static void prim_readDir(EvalState & state, const Pos & pos, Value * * args, Val
     DirEntries entries = readDirectory(state.checkSourcePath(path));
     state.mkAttrs(v, entries.size());
 
+    auto sRegular = state.symbols.create("regular");
+    auto sDirectory = state.symbols.create("directory");
+    auto sSymlink = state.symbols.create("symlink");
+    auto sUnknown = state.symbols.create("unknown");
+
     for (auto & ent : entries) {
         auto ent_val = state.allocAttr(v, state.symbols.create(ent.name));
         if (ent.type == DT_UNKNOWN)
             ent.type = getFileType(path + "/" + ent.name);
-        mkStringNoCopy(*ent_val,
+        mkString(*ent_val,
-            ent.type == DT_REG ? "regular" :
+            ent.type == DT_REG ? sRegular :
-            ent.type == DT_DIR ? "directory" :
+            ent.type == DT_DIR ? sDirectory :
-            ent.type == DT_LNK ? "symlink" :
+            ent.type == DT_LNK ? sSymlink :
-            "unknown");
+            sUnknown);
     }
 
     v.attrs->sort();

src/libexpr/primops/context.cc

@@ -160,7 +160,7 @@ static void prim_appendContext(EvalState & state, const Pos & pos, Value * * arg
         if (iter != i.value->attrs->end()) {
             if (state.forceBool(*iter->value, *iter->pos)) {
                 if (!isDerivation(i.name)) {
-                    throw EvalError("Tried to add all-outputs context of %s, which is not a derivation, to a string, at %s", i.name, i.pos);
+                    throw EvalError("tried to add all-outputs context of %s, which is not a derivation, to a string, at %s", i.name, *i.pos);
                 }
                 context.insert("=" + string(i.name));
             }
@@ -170,7 +170,7 @@ static void prim_appendContext(EvalState & state, const Pos & pos, Value * * arg
         if (iter != i.value->attrs->end()) {
             state.forceList(*iter->value, *iter->pos);
             if (iter->value->listSize() && !isDerivation(i.name)) {
-                throw EvalError("Tried to add derivation output context of %s, which is not a derivation, to a string, at %s", i.name, i.pos);
+                throw EvalError("tried to add derivation output context of %s, which is not a derivation, to a string, at %s", i.name, *i.pos);
             }
             for (unsigned int n = 0; n < iter->value->listSize(); ++n) {
                 auto name = state.forceStringNoCtx(*iter->value->listElems()[n], *iter->pos);

src/libexpr/value-to-json.cc

@@ -27,6 +27,7 @@ void printValueAsJSON(EvalState & state, bool strict,
             break;
 
         case tShortString:
+        case tStaticString:
         case tLongString:
             v.getContext(context);
             out.write(v.getString());

src/libexpr/value-to-xml.cc

@@ -69,6 +69,7 @@ static void printValueAsXML(EvalState & state, bool strict, bool location,
             break;
 
         case tShortString:
+        case tStaticString:
         case tLongString:
             /* !!! show the context? */
             v.getContext(context);

src/libexpr/value.hh

@@ -116,10 +116,10 @@ struct Value : Object
            derivation, and the other store paths in C will be added to
            the inputSrcs of the derivations. */
         struct {
-            const char * _s;
+            String * s;
             Context * context;
         } string;
-
+        const char * staticString;
         const char * path;
         Bindings * attrs;
         PtrList<Value> * bigList;
@@ -202,7 +202,7 @@ public:
 
     bool isString() const
     {
-        return type == tShortString || type == tLongString;
+        return type == tShortString || type == tStaticString || type == tLongString;
     }
 
     void setShortString(const char * s)
@@ -217,8 +217,10 @@ public:
     {
         if (type == tShortString)
             return getMiscData();
+        else if (type == tStaticString)
+            return staticString;
         else
-            return string._s;
+            return string.s->s;
     }
 };
 
@@ -266,21 +268,23 @@ static inline void mkPrimOpApp(Value & v, Value & left, Value & right)
 }
 
 
-static inline void mkStringNoCopy(Value & v, const char * s)
+static inline void mkString(Value & v, const char * s)
 {
-    // FIXME: copy short strings?
+    auto len = strlen(s); // FIXME: only need to know if > short
-    v.type = tLongString;
+    if (len < WORD_SIZE * 2 + Object::miscBytes)
-    v.string._s = s;
+        v.setShortString(s);
+    else {
+        v.string.s = gc.alloc<String>(String::wordsFor(len), len, s);
         v.string.context = 0;
+        v.type = tLongString;
+    }
 }
 
 
-void mkString(Value & v, const char * s);
-
-
 static inline void mkString(Value & v, const Symbol & s)
 {
-    mkString(v, ((const string &) s).c_str());
+    v.staticString = ((const string &) s).c_str();
+    v.type = tStaticString;
 }
 
 

src/nix/repl.cc

@@ -607,6 +607,7 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m
         break;
 
     case tShortString:
+    case tStaticString:
     case tLongString:
         str << ESC_YEL;
         printStringValue(str, v.getString());