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
This commit is contained in:
parent
26a8b220eb
commit
5838354d34
2 changed files with 64 additions and 13 deletions
|
@ -36,6 +36,19 @@
|
||||||
namespace nix {
|
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)
|
static char * dupString(const char * s)
|
||||||
{
|
{
|
||||||
char * t;
|
char * t;
|
||||||
|
@ -771,17 +784,28 @@ void Value::mkString(std::string_view s)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
static void copyContextToValue(Value & v, const PathSet & context)
|
||||||
|
{
|
||||||
|
if (!context.empty()) {
|
||||||
|
size_t n = 0;
|
||||||
|
v.string.context = (const char * *)
|
||||||
|
allocBytes((context.size() + 1) * sizeof(char *));
|
||||||
|
for (auto & i : context)
|
||||||
|
v.string.context[n++] = dupString(i.c_str());
|
||||||
|
v.string.context[n] = 0;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
void Value::mkString(std::string_view s, const PathSet & context)
|
void Value::mkString(std::string_view s, const PathSet & context)
|
||||||
{
|
{
|
||||||
mkString(s);
|
mkString(s);
|
||||||
if (!context.empty()) {
|
copyContextToValue(*this, context);
|
||||||
size_t n = 0;
|
|
||||||
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;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void Value::mkStringMove(const char * s, const PathSet & context)
|
||||||
|
{
|
||||||
|
mkString(s);
|
||||||
|
copyContextToValue(*this, context);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@ -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)
|
void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
|
||||||
{
|
{
|
||||||
PathSet context;
|
PathSet context;
|
||||||
std::ostringstream s;
|
std::vector<std::string> s;
|
||||||
|
size_t sSize = 0;
|
||||||
NixInt n = 0;
|
NixInt n = 0;
|
||||||
NixFloat nf = 0;
|
NixFloat nf = 0;
|
||||||
|
|
||||||
bool first = !forceString;
|
bool first = !forceString;
|
||||||
ValueType firstType = nString;
|
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) {
|
for (auto & [i_pos, i] : *es) {
|
||||||
Value vTmp;
|
Value vTmp;
|
||||||
i->eval(state, env, vTmp);
|
i->eval(state, env, vTmp);
|
||||||
|
@ -1696,11 +1741,15 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
|
||||||
nf += vTmp.fpoint;
|
nf += vTmp.fpoint;
|
||||||
} else
|
} else
|
||||||
throwEvalError(i_pos, "cannot add %1% to a float", showType(vTmp));
|
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
|
/* skip canonization of first path, which would only be not
|
||||||
canonized in the first place if it's coming from a ./${foo} type
|
canonized in the first place if it's coming from a ./${foo} type
|
||||||
path */
|
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;
|
first = false;
|
||||||
}
|
}
|
||||||
|
@ -1712,9 +1761,9 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
|
||||||
else if (firstType == nPath) {
|
else if (firstType == nPath) {
|
||||||
if (!context.empty())
|
if (!context.empty())
|
||||||
throwEvalError(pos, "a string that refers to a store path cannot be appended to a path");
|
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
|
} else
|
||||||
v.mkString(s.str(), context);
|
v.mkStringMove(c_str(), context);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -241,6 +241,8 @@ public:
|
||||||
|
|
||||||
void mkString(std::string_view s, const PathSet & context);
|
void mkString(std::string_view s, const PathSet & context);
|
||||||
|
|
||||||
|
void mkStringMove(const char * s, const PathSet & context);
|
||||||
|
|
||||||
inline void mkString(const Symbol & s)
|
inline void mkString(const Symbol & s)
|
||||||
{
|
{
|
||||||
mkString(((const std::string &) s).c_str());
|
mkString(((const std::string &) s).c_str());
|
||||||
|
|
Loading…
Reference in a new issue