From dec774811922d7ee220bb8e2488bc2f7abf61844 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 19 Jan 2022 15:20:46 +0100 Subject: [PATCH] Replace withBuffer by boost small_vector Although this will leave gaps in the stack, the performance impact of those should be insignificant and we get a simpler solution this way. --- src/libstore/derivations.cc | 35 ++++++++++++++++++++++------------- src/libutil/util.hh | 25 ------------------------- 2 files changed, 22 insertions(+), 38 deletions(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index f8fe6a59c..5233cfe67 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -4,6 +4,7 @@ #include "util.hh" #include "worker-protocol.hh" #include "fs-accessor.hh" +#include namespace nix { @@ -272,19 +273,27 @@ Derivation parseDerivation(const Store & store, std::string && s, std::string_vi static void printString(string & res, std::string_view s) { - size_t bufSize = s.size() * 2 + 2; - withBuffer(bufSize, [&](char *buf) { - char * p = buf; - *p++ = '"'; - for (auto c : s) - if (c == '\"' || c == '\\') { *p++ = '\\'; *p++ = c; } - else if (c == '\n') { *p++ = '\\'; *p++ = 'n'; } - else if (c == '\r') { *p++ = '\\'; *p++ = 'r'; } - else if (c == '\t') { *p++ = '\\'; *p++ = 't'; } - else *p++ = c; - *p++ = '"'; - res.append(buf, p - buf); - }); + // Large stack allocations can skip past the stack protection page. + const size_t stack_protection_size = 4096; + // We reduce the max stack allocated buffer by an extra amount to increase + // the chance of hitting it, even when `fun`'s first access is some distance + // into its *further* stack frame, particularly if the call was inlined and + // therefore not writing a frame pointer. + const size_t play = 64 * sizeof(char *); // 512B on 64b archs + + boost::container::small_vector buffer; + buffer.reserve(s.size() * 2 + 2); + char * buf = buffer.data(); + char * p = buf; + *p++ = '"'; + for (auto c : s) + if (c == '\"' || c == '\\') { *p++ = '\\'; *p++ = c; } + else if (c == '\n') { *p++ = '\\'; *p++ = 'n'; } + else if (c == '\r') { *p++ = '\\'; *p++ = 'r'; } + else if (c == '\t') { *p++ = '\\'; *p++ = 't'; } + else *p++ = c; + *p++ = '"'; + res.append(buf, p - buf); } diff --git a/src/libutil/util.hh b/src/libutil/util.hh index e2192987c..369c44f78 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -671,30 +671,5 @@ template overloaded(Ts...) -> overloaded; std::string showBytes(uint64_t bytes); -/** - `withBuffer(size, fun)` applies `fun` to a temporary `char` array of size `size`. - The array will be allocated either on the stack or on the heap depending on its size -*/ -template -inline auto withBuffer(size_t n, Fn fun) - -> std::invoke_result_t -{ - // Large stack allocations can skip past the stack protection page. - const size_t stack_protection_size = 4096; - // We reduce the max stack allocated buffer by an extra amount to increase - // the chance of hitting it, even when `fun`'s first access is some distance - // into its *further* stack frame, particularly if the call was inlined and - // therefore not writing a frame pointer. - const size_t play = 64 * sizeof(char *); // 512B on 64b archs - size_t size_bytes = n * sizeof(T); - - if (size_bytes < stack_protection_size - play) { - T buf[n]; - return fun(buf); - } else { - auto buf = std::unique_ptr(new T[n]); - return fun(buf.get()); - } -} }