From 11f4a5bc7eca8a4cca2ae9f3d83b69cd497933f8 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Fri, 3 May 2024 22:53:24 +0200 Subject: [PATCH] libutil: return a source from readFile don't consume a sink, return a source instead. the only reason to not do this is a very slight reduction in dynamic allocations, but since we are going to *at least* do disk io that will not be a lot of overhead anyway Change-Id: Iae2f879ec64c3c3ac1d5310eeb6a85e696d4614a --- src/libstore/binary-cache-store.cc | 2 +- src/libstore/build/local-derivation-goal.cc | 4 ++-- src/libstore/local-binary-cache-store.cc | 2 +- src/libstore/local-store.cc | 4 ++-- src/libstore/store-api.cc | 2 +- src/libutil/file-system.cc | 9 +++++++-- src/libutil/file-system.hh | 3 ++- src/libutil/hash.cc | 2 +- src/nix/add-to-store.cc | 2 +- src/nix/hash.cc | 2 +- 10 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 07c722016..d6ded0a24 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -385,7 +385,7 @@ StorePath BinaryCacheStore::addToStore( if (method == FileIngestionMethod::Recursive) { dumpPath(srcPath, sink, filter); } else { - readFile(srcPath, sink); + readFileSource(srcPath)->drainInto(sink); } auto h = sink.finish().first; diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 7066f5c93..bae821266 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2486,7 +2486,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() HashModuloSink caSink { outputHash.hashType, oldHashPart }; std::visit(overloaded { [&](const TextIngestionMethod &) { - readFile(actualPath, caSink); + readFileSource(actualPath)->drainInto(caSink); }, [&](const FileIngestionMethod & m2) { switch (m2) { @@ -2494,7 +2494,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() dumpPath(actualPath, caSink); break; case FileIngestionMethod::Flat: - readFile(actualPath, caSink); + readFileSource(actualPath)->drainInto(caSink); break; } }, diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index 5684dcd80..5f6730476 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -71,7 +71,7 @@ protected: void getFile(const std::string & path, Sink & sink) override { try { - readFile(binaryCacheDir + "/" + path, sink); + readFileSource(binaryCacheDir + "/" + path)->drainInto(sink); } catch (SysError & e) { if (e.errNo == ENOENT) throw NoSuchBinaryCacheFile("file '%s' does not exist in binary cache", path); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 5e79630c6..6f441a0a1 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1890,7 +1890,7 @@ ContentAddress LocalStore::hashCAPath( HashModuloSink caSink ( hashType, std::string(pathHash) ); std::visit(overloaded { [&](const TextIngestionMethod &) { - readFile(path, caSink); + readFileSource(path)->drainInto(caSink); }, [&](const FileIngestionMethod & m2) { switch (m2) { @@ -1898,7 +1898,7 @@ ContentAddress LocalStore::hashCAPath( dumpPath(path, caSink); break; case FileIngestionMethod::Flat: - readFile(path, caSink); + readFileSource(path)->drainInto(caSink); break; } }, diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 55083e834..e954acff3 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -279,7 +279,7 @@ StorePath Store::addToStore( if (method == FileIngestionMethod::Recursive) dumpPath(srcPath, sink, filter); else - readFile(srcPath, sink); + readFileSource(srcPath)->drainInto(sink); }); return addToStoreFromDump(*source, name, method, hashAlgo, repair, references); } diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index d573b22b4..f51f3c092 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -289,12 +289,17 @@ std::string readFile(const Path & path) } -void readFile(const Path & path, Sink & sink) +box_ptr readFileSource(const Path & path) { AutoCloseFD fd{open(path.c_str(), O_RDONLY | O_CLOEXEC)}; if (!fd) throw SysError("opening file '%s'", path); - drainFD(fd.get(), sink); + + struct FileSource : FdSource { + AutoCloseFD fd; + explicit FileSource(AutoCloseFD fd) : FdSource(fd.get()), fd(std::move(fd)) {} + }; + return make_box_ptr(std::move(fd)); } diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh index 6c1923d55..64d884227 100644 --- a/src/libutil/file-system.hh +++ b/src/libutil/file-system.hh @@ -5,6 +5,7 @@ * Utiltities for working with the file sytem and file paths. */ +#include "box_ptr.hh" #include "types.hh" #include "file-descriptor.hh" @@ -142,7 +143,7 @@ unsigned char getFileType(const Path & path); * Read the contents of a file into a string. */ std::string readFile(const Path & path); -void readFile(const Path & path, Sink & sink); +box_ptr readFileSource(const Path & path); /** * Write a string to a file. diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 006b5000c..822fa150e 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -324,7 +324,7 @@ Hash hashString(HashType ht, std::string_view s) Hash hashFile(HashType ht, const Path & path) { HashSink sink(ht); - readFile(path, sink); + readFileSource(path)->drainInto(sink); return sink.finish().first; } diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index 39e5cc99d..7dbbbcc56 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -37,7 +37,7 @@ struct CmdAddToStore : MixDryRun, StoreCommand Hash hash = narHash; if (ingestionMethod == FileIngestionMethod::Flat) { HashSink hsink(htSHA256); - readFile(path, hsink); + readFileSource(path)->drainInto(hsink); hash = hsink.finish().first; } diff --git a/src/nix/hash.cc b/src/nix/hash.cc index 9feca9345..66c5516e7 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -85,7 +85,7 @@ struct CmdHashBase : Command switch (mode) { case FileIngestionMethod::Flat: - readFile(path, *hashSink); + readFileSource(path)->drainInto(*hashSink); break; case FileIngestionMethod::Recursive: dumpPath(path, *hashSink);