Merge pull request #3822 from obsidiansystems/dump-thrice-fixme

Optimize `addToStoreSlow` and remove `TeeParseSink`

src/libstore/daemon.cc

@@ -173,31 +173,6 @@ struct TunnelSource : BufferedSource
     }
 };
 
-/* If the NAR archive contains a single file at top-level, then save
-   the contents of the file to `s'.  Otherwise barf. */
-struct RetrieveRegularNARSink : ParseSink
-{
-    bool regular;
-    string s;
-
-    RetrieveRegularNARSink() : regular(true) { }
-
-    void createDirectory(const Path & path)
-    {
-        regular = false;
-    }
-
-    void receiveContents(unsigned char * data, unsigned int len)
-    {
-        s.append((const char *) data, len);
-    }
-
-    void createSymlink(const Path & path, const string & target)
-    {
-        regular = false;
-    }
-};
-
 struct ClientSettings
 {
     bool keepFailed;
@@ -391,9 +366,9 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         }
         HashType hashAlgo = parseHashType(s);
 
-        StringSink savedNAR;
-        TeeSource savedNARSource(from, savedNAR);
-        RetrieveRegularNARSink savedRegular;
+        StringSink saved;
+        TeeSource savedNARSource(from, saved);
+        RetrieveRegularNARSink savedRegular { saved };
 
         if (method == FileIngestionMethod::Recursive) {
             /* Get the entire NAR dump from the client and save it to
@@ -407,11 +382,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         logger->startWork();
         if (!savedRegular.regular) throw Error("regular file expected");
 
-        auto path = store->addToStoreFromDump(
-            method == FileIngestionMethod::Recursive ? *savedNAR.s : savedRegular.s,
-            baseName,
-            method,
-            hashAlgo);
+        auto path = store->addToStoreFromDump(*saved.s, baseName, method, hashAlgo);
         logger->stopWork();
 
         to << store->printStorePath(path);
@@ -727,15 +698,15 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         if (!trusted)
             info.ultimate = false;
 
-        std::string saved;
         std::unique_ptr<Source> source;
         if (GET_PROTOCOL_MINOR(clientVersion) >= 21)
             source = std::make_unique<TunnelSource>(from, to);
         else {
-            TeeParseSink tee(from);
-            parseDump(tee, tee.source);
-            saved = std::move(*tee.saved.s);
-            source = std::make_unique<StringSource>(saved);
+            StringSink saved;
+            TeeSource tee { from, saved };
+            ParseSink ether;
+            parseDump(ether, tee);
+            source = std::make_unique<StringSource>(std::move(*saved.s));
         }
 
         logger->startWork();

src/libstore/export-import.cc

@@ -60,8 +60,10 @@ StorePaths Store::importPaths(Source & source, CheckSigsFlag checkSigs)
         if (n != 1) throw Error("input doesn't look like something created by 'nix-store --export'");
 
         /* Extract the NAR from the source. */
-        TeeParseSink tee(source);
-        parseDump(tee, tee.source);
+        StringSink saved;
+        TeeSource tee { source, saved };
+        ParseSink ether;
+        parseDump(ether, tee);
 
         uint32_t magic = readInt(source);
         if (magic != exportMagic)
@@ -77,15 +79,15 @@ StorePaths Store::importPaths(Source & source, CheckSigsFlag checkSigs)
         if (deriver != "")
             info.deriver = parseStorePath(deriver);
 
-        info.narHash = hashString(htSHA256, *tee.saved.s);
-        info.narSize = tee.saved.s->size();
+        info.narHash = hashString(htSHA256, *saved.s);
+        info.narSize = saved.s->size();
 
         // Ignore optional legacy signature.
         if (readInt(source) == 1)
             readString(source);
 
         // Can't use underlying source, which would have been exhausted
-        auto source = StringSource { *tee.saved.s };
+        auto source = StringSource { *saved.s };
         addToStore(info, source, NoRepair, checkSigs);
 
         res.push_back(info.path);

src/libstore/store-api.cc

@@ -226,16 +226,41 @@ ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath,
     FileIngestionMethod method, HashType hashAlgo,
     std::optional<Hash> expectedCAHash)
 {
-    /* FIXME: inefficient: we're reading/hashing 'tmpFile' three
+    /* FIXME: inefficient: we're reading/hashing 'tmpFile' two
        times. */
+    HashSink narHashSink { htSHA256 };
+    HashSink caHashSink { hashAlgo };
+    RetrieveRegularNARSink fileSink { caHashSink };
 
-    auto [narHash, narSize] = hashPath(htSHA256, srcPath);
+    TeeSink sinkIfNar { narHashSink, caHashSink };
 
-    auto hash = method == FileIngestionMethod::Recursive
-        ? hashAlgo == htSHA256
-          ? narHash
-          : hashPath(hashAlgo, srcPath).first
-        : hashFile(hashAlgo, srcPath);
+    /* We use the tee sink if we need to hash the nar twice */
+    auto & sink = method == FileIngestionMethod::Recursive && hashAlgo != htSHA256
+        ? static_cast<Sink &>(sinkIfNar)
+        : narHashSink;
+
+    auto fileSource = sinkToSource([&](Sink & sink) {
+        dumpPath(srcPath, sink);
+    });
+
+    TeeSource tapped { *fileSource, sink };
+
+    ParseSink blank;
+    auto & parseSink = method == FileIngestionMethod::Flat
+        ? fileSink
+        : blank;
+
+    parseDump(
+        parseSink,
+        method == FileIngestionMethod::Recursive && hashAlgo == htSHA256
+            ? *fileSource // don't need to hash twice if we just can use the `narHash` twice
+            : tapped);
+
+    auto [narHash, narSize] = narHashSink.finish();
+
+    auto hash = method == FileIngestionMethod::Recursive && hashAlgo == htSHA256
+        ? narHash
+        : caHashSink.finish().first;
 
     if (expectedCAHash && expectedCAHash != hash)
         throw Error("hash mismatch for '%s'", srcPath);

src/libutil/archive.hh

@@ -63,12 +63,29 @@ struct ParseSink
     virtual void createSymlink(const Path & path, const string & target) { };
 };
 
-struct TeeParseSink : ParseSink
+/* If the NAR archive contains a single file at top-level, then save
+   the contents of the file to `s'.  Otherwise barf. */
+struct RetrieveRegularNARSink : ParseSink
 {
-    StringSink saved;
-    TeeSource source;
+    bool regular = true;
+    Sink & sink;
 
-    TeeParseSink(Source & source) : source(source, saved) { }
+    RetrieveRegularNARSink(Sink & sink) : sink(sink) { }
+
+    void createDirectory(const Path & path)
+    {
+        regular = false;
+    }
+
+    void receiveContents(unsigned char * data, unsigned int len)
+    {
+        sink(data, len);
+    }
+
+    void createSymlink(const Path & path, const string & target)
+    {
+        regular = false;
+    }
 };
 
 void parseDump(ParseSink & sink, Source & source);