Skip to content

Commit

Permalink
Make LocalBinaryCacheStore::narFromPath() run in constant memory
Browse files Browse the repository at this point in the history
This reduces memory consumption of

  nix copy --from file://... --to ~/my-nix /nix/store/95cwv4q54dc6giaqv6q6p4r02ia2km35-blender-2.79

from 514 MiB to 18 MiB for an uncompressed binary cache, and from 192
MiB to 53 MiB for a bzipped binary cache. It may also be faster
because fetching can happen concurrently with decompression/writing.

Continuation of 48662d1.

Issue NixOS#1681.
  • Loading branch information
edolstra committed Mar 27, 2018
1 parent d2bc632 commit 3c3cb96
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 31 deletions.
36 changes: 27 additions & 9 deletions src/libstore/binary-cache-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,15 @@ void BinaryCacheStore::init()
}
}

std::shared_ptr<std::string> BinaryCacheStore::getFile(const std::string & path)
void BinaryCacheStore::getFile(const std::string & path,
Callback<std::shared_ptr<std::string>> callback)
{
try {
callback(getFile(path));
} catch (...) { callback.rethrow(); }
}

void BinaryCacheStore::getFile(const std::string & path, Sink & sink)
{
std::promise<std::shared_ptr<std::string>> promise;
getFile(path,
Expand All @@ -65,7 +73,19 @@ std::shared_ptr<std::string> BinaryCacheStore::getFile(const std::string & path)
promise.set_exception(std::current_exception());
}
}});
return promise.get_future().get();
auto data = promise.get_future().get();
sink((unsigned char *) data->data(), data->size());
}

std::shared_ptr<std::string> BinaryCacheStore::getFile(const std::string & path)
{
StringSink sink;
try {
getFile(path, sink);
} catch (NoSuchBinaryCacheFile &) {
return nullptr;
}
return sink.s;
}

Path BinaryCacheStore::narInfoFileFor(const Path & storePath)
Expand Down Expand Up @@ -197,23 +217,21 @@ void BinaryCacheStore::narFromPath(const Path & storePath, Sink & sink)
{
auto info = queryPathInfo(storePath).cast<const NarInfo>();

auto nar = getFile(info->url);

if (!nar) throw Error(format("file '%s' missing from binary cache") % info->url);
auto source = sinkToSource([this, url{info->url}](Sink & sink) {
getFile(url, sink);
});

stats.narRead++;
stats.narReadCompressedBytes += nar->size();
//stats.narReadCompressedBytes += nar->size(); // FIXME

uint64_t narSize = 0;

StringSource source(*nar);

LambdaSink wrapperSink([&](const unsigned char * data, size_t len) {
sink(data, len);
narSize += len;
});

decompress(info->compression, source, wrapperSink);
decompress(info->compression, *source, wrapperSink);

stats.narReadBytes += narSize;
}
Expand Down
14 changes: 11 additions & 3 deletions src/libstore/binary-cache-store.hh
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,16 @@ public:
const std::string & data,
const std::string & mimeType) = 0;

/* Return the contents of the specified file, or null if it
doesn't exist. */
/* Note: subclasses must implement at least one of the two
following getFile() methods. */

/* Dump the contents of the specified file to a sink. */
virtual void getFile(const std::string & path, Sink & sink);

/* Fetch the specified file and call the specified callback with
the result. A subclass may implement this asynchronously. */
virtual void getFile(const std::string & path,
Callback<std::shared_ptr<std::string>> callback) = 0;
Callback<std::shared_ptr<std::string>> callback);

std::shared_ptr<std::string> getFile(const std::string & path);

Expand Down Expand Up @@ -129,4 +135,6 @@ public:

};

MakeError(NoSuchBinaryCacheFile, Error);

}
11 changes: 5 additions & 6 deletions src/libstore/local-binary-cache-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,14 @@ class LocalBinaryCacheStore : public BinaryCacheStore
const std::string & data,
const std::string & mimeType) override;

void getFile(const std::string & path,
Callback<std::shared_ptr<std::string>> callback) override
void getFile(const std::string & path, Sink & sink) override
{
try {
// FIXME: O(n) space
callback(std::make_shared<std::string>(readFile(binaryCacheDir + "/" + path)));
readFile(binaryCacheDir + "/" + path, sink);
} catch (SysError & e) {
if (e.errNo == ENOENT) callback(nullptr); else callback.rethrow();
} catch (...) { callback.rethrow(); }
if (e.errNo == ENOENT)
throw NoSuchBinaryCacheFile("file '%s' does not exist in binary cache", path);
}
}

PathSet queryAllValidPaths() override
Expand Down
24 changes: 12 additions & 12 deletions src/libstore/s3-binary-cache-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -316,23 +316,23 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore
uploadFile(path, data, mimeType, "");
}

void getFile(const std::string & path,
Callback<std::shared_ptr<std::string>> callback) override
void getFile(const std::string & path, Sink & sink) override
{
try {
stats.get++;
stats.get++;

auto res = s3Helper.getObject(bucketName, path);
// FIXME: stream output to sink.
auto res = s3Helper.getObject(bucketName, path);

stats.getBytes += res.data ? res.data->size() : 0;
stats.getTimeMs += res.durationMs;
stats.getBytes += res.data ? res.data->size() : 0;
stats.getTimeMs += res.durationMs;

if (res.data)
printTalkative("downloaded 's3://%s/%s' (%d bytes) in %d ms",
bucketName, path, res.data->size(), res.durationMs);
if (res.data) {
printTalkative("downloaded 's3://%s/%s' (%d bytes) in %d ms",
bucketName, path, res.data->size(), res.durationMs);

callback(std::move(res.data));
} catch (...) { callback.rethrow(); }
sink((unsigned char *) res.data->data(), res.data->size());
} else
throw NoSuchBinaryCacheFile("file '%s' does not exist in binary cache '%s'", path, getUri());
}

PathSet queryAllValidPaths() override
Expand Down
10 changes: 9 additions & 1 deletion src/libutil/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,14 @@ string readFile(const Path & path, bool drain)
}


void readFile(const Path & path, Sink & sink)
{
AutoCloseFD fd = open(path.c_str(), O_RDONLY | O_CLOEXEC);
if (!fd) throw SysError("opening file '%s'", path);
drainFD(fd.get(), sink);
}


void writeFile(const Path & path, const string & s, mode_t mode)
{
AutoCloseFD fd = open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, mode);
Expand Down Expand Up @@ -593,7 +601,7 @@ void drainFD(int fd, Sink & sink, bool block)
throw SysError("making file descriptor non-blocking");
}

std::vector<unsigned char> buf(4096);
std::vector<unsigned char> buf(64 * 1024);
while (1) {
checkInterrupt();
ssize_t rd = read(fd, buf.data(), buf.size());
Expand Down
1 change: 1 addition & 0 deletions src/libutil/util.hh
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ unsigned char getFileType(const Path & path);
/* Read the contents of a file into a string. */
string readFile(int fd);
string readFile(const Path & path, bool drain = false);
void readFile(const Path & path, Sink & sink);

/* Write a string to a file. */
void writeFile(const Path & path, const string & s, mode_t mode = 0666);
Expand Down

0 comments on commit 3c3cb96

Please sign in to comment.