From 34f25124ba2ab32b8a95d6b37cd68d7bb85ff2d4 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 9 Apr 2018 23:46:20 +0200 Subject: Make LocalStore::addToStore(srcPath) run in constant memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reduces memory consumption of nix-instantiate \ -E 'with import {}; runCommand "foo" { src = ./blender; } "echo foo"' \ --option nar-buffer-size 10000 (where ./blender is a 1.1 GiB tree) from 1716 to 36 MiB, while still ensuring that we don't do any write I/O for small source paths (up to 'nar-buffer-size' bytes). The downside is that large paths are now always written to a temporary location in the store, even if they produce an already valid store path. Thus, adding large paths might be slower and run out of disk space. ¯\_(ツ)_/¯ Of course, you can always restore the old behaviour by setting 'nar-buffer-size' to a very high value. --- src/libstore/local-store.cc | 123 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 113 insertions(+), 10 deletions(-) (limited to 'src/libstore/local-store.cc') diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index eed225349..b9176ec38 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1098,16 +1098,119 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, { Path srcPath(absPath(_srcPath)); - /* Read the whole path into memory. This is not a very scalable - method for very large paths, but `copyPath' is mainly used for - small files. */ - StringSink sink; - if (method == FileIngestionMethod::Recursive) - dumpPath(srcPath, sink, filter); - else - sink.s = make_ref(readFile(srcPath)); - - return addToStoreFromDump(*sink.s, name, method, hashAlgo, repair); + if (method != FileIngestionMethod::Recursive) + return addToStoreFromDump(readFile(srcPath), name, method, hashAlgo, repair); + + /* For computing the NAR hash. */ + auto sha256Sink = std::make_unique(htSHA256); + + /* For computing the store path. In recursive SHA-256 mode, this + is the same as the NAR hash, so no need to do it again. */ + std::unique_ptr hashSink = + hashAlgo == htSHA256 + ? nullptr + : std::make_unique(hashAlgo); + + /* Read the source path into memory, but only if it's up to + narBufferSize bytes. If it's larger, write it to a temporary + location in the Nix store. If the subsequently computed + destination store path is already valid, we just delete the + temporary path. Otherwise, we move it to the destination store + path. */ + bool inMemory = true; + std::string nar; + + auto source = sinkToSource([&](Sink & sink) { + + LambdaSink sink2([&](const unsigned char * buf, size_t len) { + (*sha256Sink)(buf, len); + if (hashSink) (*hashSink)(buf, len); + + if (inMemory) { + if (nar.size() + len > settings.narBufferSize) { + inMemory = false; + sink << 1; + sink((const unsigned char *) nar.data(), nar.size()); + nar.clear(); + } else { + nar.append((const char *) buf, len); + } + } + + if (!inMemory) sink(buf, len); + }); + + dumpPath(srcPath, sink2, filter); + }); + + std::unique_ptr delTempDir; + Path tempPath; + + try { + /* Wait for the source coroutine to give us some dummy + data. This is so that we don't create the temporary + directory if the NAR fits in memory. */ + readInt(*source); + + auto tempDir = createTempDir(realStoreDir, "add"); + delTempDir = std::make_unique(tempDir); + tempPath = tempDir + "/x"; + + restorePath(tempPath, *source); + + } catch (EndOfFile &) { + if (!inMemory) throw; + /* The NAR fits in memory, so we didn't do restorePath(). */ + } + + auto sha256 = sha256Sink->finish(); + + Hash hash = hashSink ? hashSink->finish().first : sha256.first; + + Path dstPath = makeFixedOutputPath(method, hash, name); + + addTempRoot(dstPath); + + if (repair || !isValidPath(dstPath)) { + + /* The first check above is an optimisation to prevent + unnecessary lock acquisition. */ + + Path realPath = realStoreDir + "/" + baseNameOf(dstPath); + + PathLocks outputLock({realPath}); + + if (repair || !isValidPath(dstPath)) { + + deletePath(realPath); + + autoGC(); + + if (inMemory) { + /* Restore from the NAR in memory. */ + StringSource source(nar); + restorePath(realPath, source); + } else { + /* Move the temporary path we restored above. */ + if (rename(tempPath.c_str(), realPath.c_str())) + throw Error("renaming '%s' to '%s'", tempPath, realPath); + } + + canonicalisePathMetaData(realPath, -1); // FIXME: merge into restorePath + + optimisePath(realPath); + + ValidPathInfo info(dstPath); + info.narHash = sha256.first; + info.narSize = sha256.second; + info.ca = FixedOutputHash { .method = method, .hash = hash }; + registerValidPath(info); + } + + outputLock.setDeletion(true); + } + + return dstPath; } -- cgit v1.2.3 From b981e5aacf3848424264f4a84826f8f9ca33da14 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 8 Jul 2020 22:07:10 +0200 Subject: Cleanup --- src/libstore/local-store.cc | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'src/libstore/local-store.cc') diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index b9176ec38..fa66c1c90 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -976,7 +976,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, PathLocks outputLock; - Path realPath = realStoreDir + "/" + std::string(info.path.to_string()); + auto realPath = Store::toRealPath(info.path); /* Lock the output path. But don't lock if we're being called from a build hook (whose parent process already acquired a @@ -1047,8 +1047,7 @@ StorePath LocalStore::addToStoreFromDump(const string & dump, const string & nam /* The first check above is an optimisation to prevent unnecessary lock acquisition. */ - Path realPath = realStoreDir + "/"; - realPath += dstPath.to_string(); + auto realPath = Store::toRealPath(dstPath); PathLocks outputLock({realPath}); @@ -1167,7 +1166,7 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, Hash hash = hashSink ? hashSink->finish().first : sha256.first; - Path dstPath = makeFixedOutputPath(method, hash, name); + auto dstPath = makeFixedOutputPath(method, hash, name); addTempRoot(dstPath); @@ -1176,7 +1175,7 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, /* The first check above is an optimisation to prevent unnecessary lock acquisition. */ - Path realPath = realStoreDir + "/" + baseNameOf(dstPath); + auto realPath = Store::toRealPath(dstPath); PathLocks outputLock({realPath}); @@ -1224,8 +1223,7 @@ StorePath LocalStore::addTextToStore(const string & name, const string & s, if (repair || !isValidPath(dstPath)) { - Path realPath = realStoreDir + "/"; - realPath += dstPath.to_string(); + auto realPath = Store::toRealPath(dstPath); PathLocks outputLock({realPath}); -- cgit v1.2.3 From a2c27022e9afc394e08d34d349587c8903fc1a97 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 9 Jul 2020 15:54:32 +0200 Subject: LocalStore::addToStore(srcPath): Handle the flat case This helps nix-prefetch-url when using a local store. --- src/libstore/local-store.cc | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'src/libstore/local-store.cc') diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index fa66c1c90..69f149841 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1097,16 +1097,13 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, { Path srcPath(absPath(_srcPath)); - if (method != FileIngestionMethod::Recursive) - return addToStoreFromDump(readFile(srcPath), name, method, hashAlgo, repair); - /* For computing the NAR hash. */ auto sha256Sink = std::make_unique(htSHA256); /* For computing the store path. In recursive SHA-256 mode, this is the same as the NAR hash, so no need to do it again. */ std::unique_ptr hashSink = - hashAlgo == htSHA256 + method == FileIngestionMethod::Recursive && hashAlgo == htSHA256 ? nullptr : std::make_unique(hashAlgo); @@ -1139,7 +1136,10 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, if (!inMemory) sink(buf, len); }); - dumpPath(srcPath, sink2, filter); + if (method == FileIngestionMethod::Recursive) + dumpPath(srcPath, sink2, filter); + else + readFile(srcPath, sink2); }); std::unique_ptr delTempDir; @@ -1155,7 +1155,10 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, delTempDir = std::make_unique(tempDir); tempPath = tempDir + "/x"; - restorePath(tempPath, *source); + if (method == FileIngestionMethod::Recursive) + restorePath(tempPath, *source); + else + writeFile(tempPath, *source); } catch (EndOfFile &) { if (!inMemory) throw; @@ -1188,7 +1191,10 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, if (inMemory) { /* Restore from the NAR in memory. */ StringSource source(nar); - restorePath(realPath, source); + if (method == FileIngestionMethod::Recursive) + restorePath(realPath, source); + else + writeFile(realPath, source); } else { /* Move the temporary path we restored above. */ if (rename(tempPath.c_str(), realPath.c_str())) -- cgit v1.2.3 From 545bb2ed03001cd7a80a90f73eb500f396c043a1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Jul 2020 17:37:44 +0200 Subject: Remove 'accessor' from addToStore() This is only used by hydra-queue-runner and it's better to implement it there. --- src/libstore/local-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/local-store.cc') diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 69f149841..26b226fe8 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -962,7 +962,7 @@ const PublicKeys & LocalStore::getPublicKeys() void LocalStore::addToStore(const ValidPathInfo & info, Source & source, - RepairFlag repair, CheckSigsFlag checkSigs, std::shared_ptr accessor) + RepairFlag repair, CheckSigsFlag checkSigs) { if (!info.narHash) throw Error("cannot add path '%s' because it lacks a hash", printStorePath(info.path)); -- cgit v1.2.3 From cff2157185912025c24a1b9dc99056161634176c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 15 Jul 2020 12:49:03 +0200 Subject: Revert "LocalStore::addToStore(srcPath): Handle the flat case" This reverts commit a2c27022e9afc394e08d34d349587c8903fc1a97. See addToStoreSlow(), we don't need to handle this case efficiently anymore. In fact, we can almost remove the method/hashAlgo arguments since the non-recursive and/or non-SHA256 are almost not used anymore. --- src/libstore/local-store.cc | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) (limited to 'src/libstore/local-store.cc') diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 26b226fe8..b3f4b3f7d 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1097,13 +1097,16 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, { Path srcPath(absPath(_srcPath)); + if (method != FileIngestionMethod::Recursive) + return addToStoreFromDump(readFile(srcPath), name, method, hashAlgo, repair); + /* For computing the NAR hash. */ auto sha256Sink = std::make_unique(htSHA256); /* For computing the store path. In recursive SHA-256 mode, this is the same as the NAR hash, so no need to do it again. */ std::unique_ptr hashSink = - method == FileIngestionMethod::Recursive && hashAlgo == htSHA256 + hashAlgo == htSHA256 ? nullptr : std::make_unique(hashAlgo); @@ -1136,10 +1139,7 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, if (!inMemory) sink(buf, len); }); - if (method == FileIngestionMethod::Recursive) - dumpPath(srcPath, sink2, filter); - else - readFile(srcPath, sink2); + dumpPath(srcPath, sink2, filter); }); std::unique_ptr delTempDir; @@ -1155,10 +1155,7 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, delTempDir = std::make_unique(tempDir); tempPath = tempDir + "/x"; - if (method == FileIngestionMethod::Recursive) - restorePath(tempPath, *source); - else - writeFile(tempPath, *source); + restorePath(tempPath, *source); } catch (EndOfFile &) { if (!inMemory) throw; @@ -1191,10 +1188,7 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, if (inMemory) { /* Restore from the NAR in memory. */ StringSource source(nar); - if (method == FileIngestionMethod::Recursive) - restorePath(realPath, source); - else - writeFile(realPath, source); + restorePath(realPath, source); } else { /* Move the temporary path we restored above. */ if (rename(tempPath.c_str(), realPath.c_str())) -- cgit v1.2.3