From 99a691c8a1abffd30077bd5f005cb8d4bbafae5c Mon Sep 17 00:00:00 2001
From: pennae <github@quasiparticle.net>
Date: Sun, 24 Dec 2023 21:14:08 +0100
Subject: [PATCH] don't use istreams in hot paths
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

istream sentry objects are very expensive for single-character
operations, and since we don't configure exception masks for the
istreams used here they don't even do anything. all we need is
end-of-string checks and an advancing position in an immutable memory
buffer, both of which can be had for much cheaper than istreams allow.

the effect of this change is most apparent on empty stores.

before:

Benchmark 1: nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
  Time (mean ± σ):      7.167 s ±  0.013 s    [User: 5.528 s, System: 1.431 s]
  Range (min … max):    7.147 s …  7.182 s    10 runs

after:

Benchmark 1: nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
  Time (mean ± σ):      6.963 s ±  0.011 s    [User: 5.330 s, System: 1.421 s]
  Range (min … max):    6.943 s …  6.974 s    10 runs
---
 src/libstore/derivations.cc | 47 +++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
index 8a7d660ff..973ce5211 100644
--- a/src/libstore/derivations.cc
+++ b/src/libstore/derivations.cc
@@ -154,18 +154,39 @@ StorePath writeDerivation(Store & store,
 }
 
 
-/* Read string `s' from stream `str'. */
-static void expect(std::istream & str, std::string_view s)
-{
-    for (auto & c : s) {
-        if (str.get() != c)
-            throw FormatError("expected string '%1%'", s);
+namespace {
+/**
+ * This mimics std::istream to some extent. We use this much smaller implementation
+ * instead of plain istreams because the sentry object overhead is too high.
+ */
+struct StringViewStream {
+    std::string_view remaining;
+
+    int peek() const {
+        return remaining.empty() ? EOF : remaining[0];
     }
+
+    int get() {
+        if (remaining.empty()) return EOF;
+        char c = remaining[0];
+        remaining.remove_prefix(1);
+        return c;
+    }
+};
+}
+
+
+/* Read string `s' from stream `str'. */
+static void expect(StringViewStream & str, std::string_view s)
+{
+    if (!str.remaining.starts_with(s))
+        throw FormatError("expected string '%1%'", s);
+    str.remaining.remove_prefix(s.size());
 }
 
 
 /* Read a C-style string from stream `str'. */
-static std::string parseString(std::istream & str)
+static std::string parseString(StringViewStream & str)
 {
     std::string res;
     expect(str, "\"");
@@ -187,7 +208,7 @@ static void validatePath(std::string_view s) {
         throw FormatError("bad path '%1%' in derivation", s);
 }
 
-static Path parsePath(std::istream & str)
+static Path parsePath(StringViewStream & str)
 {
     auto s = parseString(str);
     validatePath(s);
@@ -195,7 +216,7 @@ static Path parsePath(std::istream & str)
 }
 
 
-static bool endOfList(std::istream & str)
+static bool endOfList(StringViewStream & str)
 {
     if (str.peek() == ',') {
         str.get();
@@ -209,7 +230,7 @@ static bool endOfList(std::istream & str)
 }
 
 
-static StringSet parseStrings(std::istream & str, bool arePaths)
+static StringSet parseStrings(StringViewStream & str, bool arePaths)
 {
     StringSet res;
     expect(str, "[");
@@ -267,7 +288,7 @@ static DerivationOutput parseDerivationOutput(
 }
 
 static DerivationOutput parseDerivationOutput(
-    const StoreDirConfig & store, std::istringstream & str,
+    const StoreDirConfig & store, StringViewStream & str,
     const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings)
 {
     expect(str, ","); const auto pathS = parseString(str);
@@ -297,7 +318,7 @@ enum struct DerivationATermVersion {
 
 static DerivedPathMap<StringSet>::ChildNode parseDerivedPathMapNode(
     const StoreDirConfig & store,
-    std::istringstream & str,
+    StringViewStream & str,
     DerivationATermVersion version)
 {
     DerivedPathMap<StringSet>::ChildNode node;
@@ -349,7 +370,7 @@ Derivation parseDerivation(
     Derivation drv;
     drv.name = name;
 
-    std::istringstream str(std::move(s));
+    StringViewStream str{s};
     expect(str, "D");
     DerivationATermVersion version;
     switch (str.peek()) {