diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index b7ca95fa1..e4438afa1 100644 --- a/src/libexpr/eval.cc +++ b/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 & 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 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); diff --git a/src/libexpr/gc.cc b/src/libexpr/gc.cc index 2ed5e0198..b38efab79 100644 --- a/src/libexpr/gc.cc +++ b/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 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; diff --git a/src/libexpr/gc.hh b/src/libexpr/gc.hh index 47edfd003..0d2e17b86 100644 --- a/src/libexpr/gc.hh +++ b/src/libexpr/gc.hh @@ -5,6 +5,7 @@ #include #include #include +#include //#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(wordsFor(len), len, src); + } +}; + } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 6bda0573a..3a3fb01c9 100644 --- a/src/libexpr/primops.cc +++ b/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, - ent.type == DT_REG ? "regular" : - ent.type == DT_DIR ? "directory" : - ent.type == DT_LNK ? "symlink" : - "unknown"); + mkString(*ent_val, + ent.type == DT_REG ? sRegular : + ent.type == DT_DIR ? sDirectory : + ent.type == DT_LNK ? sSymlink : + sUnknown); } v.attrs->sort(); diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index 2d79739ea..e0cee1f0d 100644 --- a/src/libexpr/primops/context.cc +++ b/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); diff --git a/src/libexpr/value-to-json.cc b/src/libexpr/value-to-json.cc index ad9a3086d..217fe0c3c 100644 --- a/src/libexpr/value-to-json.cc +++ b/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()); diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc index 1b48bb336..7d1a8d51f 100644 --- a/src/libexpr/value-to-xml.cc +++ b/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); diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 1112511fd..6801dd5d7 100644 --- a/src/libexpr/value.hh +++ b/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 * 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? - v.type = tLongString; - v.string._s = s; - v.string.context = 0; + auto len = strlen(s); // FIXME: only need to know if > short + if (len < WORD_SIZE * 2 + Object::miscBytes) + v.setShortString(s); + else { + v.string.s = gc.alloc(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; } diff --git a/src/nix/repl.cc b/src/nix/repl.cc index 8ab488e4e..34b853cca 100644 --- a/src/nix/repl.cc +++ b/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());