aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorpennae <github@quasiparticle.net>2021-12-27 02:04:49 +0100
committerpennae <github@quasiparticle.net>2022-01-12 10:07:21 +0100
commit5838354d342f1cdd09da7099e86123b36ecec409 (patch)
treeebb68fd2ba7e9b29a10b36a90429b94c8efa1515
parent26a8b220eb7470e132b9bcedb94b58492cdd786f (diff)
optimize ExprConcatStrings::eval
constructing an ostringstream for non-string concats (like integer addition) is a small constant cost that we can avoid. for string concats we can keep all the string temporaries we get from coerceToString and concatenate them in one go, which saves a lot of intermediate temporaries and copies in ostringstream. we can also avoid copying the concatenated string again by directly allocating it in GC memory and moving ownership of the concatenated string into the target value. saves about 2% on system eval. before: Benchmark 1: nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system' Time (mean ± σ): 2.837 s ± 0.031 s [User: 2.562 s, System: 0.191 s] Range (min … max): 2.796 s … 2.892 s 20 runs after: Benchmark 1: nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system' Time (mean ± σ): 2.790 s ± 0.035 s [User: 2.532 s, System: 0.187 s] Range (min … max): 2.722 s … 2.836 s 20 runs
-rw-r--r--src/libexpr/eval.cc69
-rw-r--r--src/libexpr/value.hh2
2 files changed, 61 insertions, 10 deletions
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 81aa86641..61bccd6e2 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -36,6 +36,19 @@
namespace nix {
+static char * allocString(size_t size)
+{
+ char * t;
+#if HAVE_BOEHMGC
+ t = (char *) GC_MALLOC_ATOMIC(size);
+#else
+ t = malloc(size);
+#endif
+ if (!t) throw std::bad_alloc();
+ return t;
+}
+
+
static char * dupString(const char * s)
{
char * t;
@@ -771,19 +784,30 @@ void Value::mkString(std::string_view s)
}
-void Value::mkString(std::string_view s, const PathSet & context)
+static void copyContextToValue(Value & v, const PathSet & context)
{
- mkString(s);
if (!context.empty()) {
size_t n = 0;
- string.context = (const char * *)
+ v.string.context = (const char * *)
allocBytes((context.size() + 1) * sizeof(char *));
for (auto & i : context)
- string.context[n++] = dupString(i.c_str());
- string.context[n] = 0;
+ v.string.context[n++] = dupString(i.c_str());
+ v.string.context[n] = 0;
}
}
+void Value::mkString(std::string_view s, const PathSet & context)
+{
+ mkString(s);
+ copyContextToValue(*this, context);
+}
+
+void Value::mkStringMove(const char * s, const PathSet & context)
+{
+ mkString(s);
+ copyContextToValue(*this, context);
+}
+
void Value::mkPath(std::string_view s)
{
@@ -1660,13 +1684,34 @@ void EvalState::concatLists(Value & v, size_t nrLists, Value * * lists, const Po
void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
{
PathSet context;
- std::ostringstream s;
+ std::vector<std::string> s;
+ size_t sSize = 0;
NixInt n = 0;
NixFloat nf = 0;
bool first = !forceString;
ValueType firstType = nString;
+ const auto str = [&] {
+ std::string result;
+ result.reserve(sSize);
+ for (const auto & part : s) result += part;
+ return result;
+ };
+ /* c_str() is not str().c_str() because we want to create a string
+ Value. allocating a GC'd string directly and moving it into a
+ Value lets us avoid an allocation and copy. */
+ const auto c_str = [&] {
+ char * result = allocString(sSize + 1);
+ char * tmp = result;
+ for (const auto & part : s) {
+ memcpy(tmp, part.c_str(), part.size());
+ tmp += part.size();
+ }
+ *tmp = 0;
+ return result;
+ };
+
for (auto & [i_pos, i] : *es) {
Value vTmp;
i->eval(state, env, vTmp);
@@ -1696,11 +1741,15 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
nf += vTmp.fpoint;
} else
throwEvalError(i_pos, "cannot add %1% to a float", showType(vTmp));
- } else
+ } else {
+ if (s.empty()) s.reserve(es->size());
/* skip canonization of first path, which would only be not
canonized in the first place if it's coming from a ./${foo} type
path */
- s << state.coerceToString(i_pos, vTmp, context, false, firstType == nString, !first);
+ s.emplace_back(
+ state.coerceToString(i_pos, vTmp, context, false, firstType == nString, !first));
+ sSize += s.back().size();
+ }
first = false;
}
@@ -1712,9 +1761,9 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
else if (firstType == nPath) {
if (!context.empty())
throwEvalError(pos, "a string that refers to a store path cannot be appended to a path");
- v.mkPath(canonPath(s.str()));
+ v.mkPath(canonPath(str()));
} else
- v.mkString(s.str(), context);
+ v.mkStringMove(c_str(), context);
}
diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh
index 7d007ebdc..1896c7563 100644
--- a/src/libexpr/value.hh
+++ b/src/libexpr/value.hh
@@ -241,6 +241,8 @@ public:
void mkString(std::string_view s, const PathSet & context);
+ void mkStringMove(const char * s, const PathSet & context);
+
inline void mkString(const Symbol & s)
{
mkString(((const std::string &) s).c_str());