From 42bc395b63260e13f42e4bf348823799e78e445f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 4 Mar 2016 16:49:56 +0100 Subject: [PATCH] Eliminate some large string copying --- src/libstore/binary-cache-store.cc | 26 +++++++++++++------------- src/libstore/build.cc | 6 +++--- src/libstore/local-store.cc | 8 ++++---- src/libutil/serialise.cc | 4 ++-- src/libutil/serialise.hh | 4 +++- 5 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 02e73d2ce..01d937f2e 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -28,7 +28,7 @@ BinaryCacheStore::BinaryCacheStore(std::shared_ptr localStore, StringSink sink; sink << narVersionMagic1; - narMagic = sink.s; + narMagic = *sink.s; } void BinaryCacheStore::init() @@ -200,14 +200,16 @@ Paths BinaryCacheStore::importPaths(bool requireSignature, Source & source, struct TeeSource : Source { Source & readSource; - std::string data; - TeeSource(Source & readSource) : readSource(readSource) + ref data; + TeeSource(Source & readSource) + : readSource(readSource) + , data(make_ref()) { } size_t read(unsigned char * data, size_t len) { size_t n = readSource.read(data, len); - this->data.append((char *) data, n); + this->data->append((char *) data, n); return n; } }; @@ -257,7 +259,7 @@ Path BinaryCacheStore::addToStore(const string & name, const Path & srcPath, Hash h; if (recursive) { dumpPath(srcPath, sink, filter); - h = hashString(hashAlgo, sink.s); + h = hashString(hashAlgo, *sink.s); } else { auto s = readFile(srcPath); dumpString(s, sink); @@ -268,7 +270,7 @@ Path BinaryCacheStore::addToStore(const string & name, const Path & srcPath, info.path = makeFixedOutputPath(recursive, hashAlgo, h, name); if (repair || !isValidPath(info.path)) - addToCache(info, sink.s); + addToCache(info, *sink.s); return info.path; } @@ -283,7 +285,7 @@ Path BinaryCacheStore::addTextToStore(const string & name, const string & s, if (repair || !isValidPath(info.path)) { StringSink sink; dumpString(s, sink); - addToCache(info, sink.s); + addToCache(info, *sink.s); } return info.path; @@ -313,7 +315,7 @@ void BinaryCacheStore::buildPaths(const PathSet & paths, BuildMode buildMode) StringSink sink; dumpPath(storePath, sink); - addToCache(info, sink.s); + addToCache(info, *sink.s); } } @@ -352,8 +354,7 @@ struct BinaryCacheStoreAccessor : public FSAccessor StringSink sink; store->exportPath(storePath, false, sink); - // FIXME: gratuitous string copying. - auto accessor = makeNarAccessor(make_ref(sink.s)); + auto accessor = makeNarAccessor(sink.s); nars.emplace(storePath, accessor); return {accessor, restPath}; } @@ -412,12 +413,11 @@ Path BinaryCacheStore::importPath(Source & source, std::shared_ptr a bool haveSignature = readInt(source) == 1; assert(!haveSignature); - addToCache(info, tee.data); + addToCache(info, *tee.data); auto accessor_ = std::dynamic_pointer_cast(accessor); if (accessor_) - // FIXME: more gratuitous string copying - accessor_->nars.emplace(info.path, makeNarAccessor(make_ref(tee.data))); + accessor_->nars.emplace(info.path, makeNarAccessor(tee.data)); return info.path; } diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 7d124f6f0..ed4e0f659 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2009,7 +2009,7 @@ void DerivationGoal::startBuilder() throw SysError(format("linking ‘%1%’ to ‘%2%’") % p % i); StringSink sink; dumpPath(i, sink); - StringSource source(sink.s); + StringSource source(*sink.s); restorePath(p, source); } } @@ -2666,8 +2666,8 @@ void DerivationGoal::registerOutputs() StringSink sink; dumpPath(actualPath, sink); deletePath(actualPath); - sink.s = rewriteHashes(sink.s, rewritesFromTmp); - StringSource source(sink.s); + sink.s = make_ref(rewriteHashes(*sink.s, rewritesFromTmp)); + StringSource source(*sink.s); restorePath(actualPath, source); rewritten = true; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 9a5706681..8a2b7bb91 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1415,9 +1415,9 @@ Path LocalStore::addToStore(const string & name, const Path & _srcPath, if (recursive) dumpPath(srcPath, sink, filter); else - sink.s = readFile(srcPath); + sink.s = make_ref(readFile(srcPath)); - return addToStoreFromDump(sink.s, name, recursive, hashAlgo, repair); + return addToStoreFromDump(*sink.s, name, recursive, hashAlgo, repair); } @@ -1442,14 +1442,14 @@ Path LocalStore::addTextToStore(const string & name, const string & s, StringSink sink; dumpString(s, sink); - auto hash = hashString(htSHA256, sink.s); + auto hash = hashString(htSHA256, *sink.s); optimisePath(dstPath); ValidPathInfo info; info.path = dstPath; info.narHash = hash; - info.narSize = sink.s.size(); + info.narSize = sink.s->size(); info.references = references; registerValidPath(info); } diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index a3cd4ff0d..5c45c890f 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -288,11 +288,11 @@ template PathSet readStrings(Source & source); void StringSink::operator () (const unsigned char * data, size_t len) { static bool warned = false; - if (!warned && s.size() > threshold) { + if (!warned && s->size() > threshold) { warnLargeDump(); warned = true; } - s.append((const char *) data, len); + s->append((const char *) data, len); } diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 0fdc4037b..9ba6391f8 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -110,7 +110,9 @@ private: /* A sink that writes data to a string. */ struct StringSink : Sink { - string s; + ref s; + StringSink() : s(make_ref()) { }; + StringSink(ref s) : s(s) { }; void operator () (const unsigned char * data, size_t len) override; };