From 40bffe0a43e5f2f320c6bae7e39ea9c26906451d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 16 Aug 2017 16:38:23 +0200 Subject: [PATCH] Progress indicator: Cleanup --- src/libstore/build.cc | 14 ++--- src/libstore/download.cc | 2 +- src/libstore/store-api.cc | 7 +-- src/libutil/logging.cc | 22 +------ src/libutil/logging.hh | 98 ++++++++++--------------------- src/nix-daemon/nix-daemon.cc | 4 -- src/nix/progress-bar.cc | 109 +++++++++++++++++++---------------- 7 files changed, 105 insertions(+), 151 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 257f02a49..1a1fc1dee 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -335,8 +335,8 @@ public: { actDerivations.progress(doneBuilds, expectedBuilds + doneBuilds, runningBuilds, failedBuilds); actSubstitutions.progress(doneSubstitutions, expectedSubstitutions + doneSubstitutions, runningSubstitutions, failedSubstitutions); - logger->event(evSetExpected, act, actDownload, expectedDownloadSize + doneDownloadSize); - logger->event(evSetExpected, act, actCopyPath, expectedNarSize + doneNarSize); + act.setExpected(actDownload, expectedDownloadSize + doneDownloadSize); + act.setExpected(actCopyPath, expectedNarSize + doneNarSize); } }; @@ -1386,7 +1386,7 @@ void DerivationGoal::tryToBuild() bool buildLocally = buildMode != bmNormal || drv->willBuildLocally(); auto started = [&]() { - act = std::make_unique(actBuild, fmt("building '%s'", drvPath)); + act = std::make_unique(*logger, actBuild, fmt("building '%s'", drvPath)); mcRunningBuilds = std::make_unique>(worker.runningBuilds); worker.updateProgress(); }; @@ -3257,7 +3257,7 @@ void DerivationGoal::flushLine() logTail.push_back(currentLogLine); if (logTail.size() > settings.logLines) logTail.pop_front(); } - logger->event(evBuildOutput, *act, currentLogLine); + act->progress(currentLogLine); currentLogLine = ""; currentLogLinePos = 0; } @@ -3647,9 +3647,9 @@ static bool working = false; Worker::Worker(LocalStore & store) - : act(actRealise) - , actDerivations(actBuilds) - , actSubstitutions(actCopyPaths) + : act(*logger, actRealise) + , actDerivations(*logger, actBuilds) + , actSubstitutions(*logger, actCopyPaths) , store(store) { /* Debugging: prevent recursive workers. */ diff --git a/src/libstore/download.cc b/src/libstore/download.cc index 9ac4a65d5..71b720a3b 100644 --- a/src/libstore/download.cc +++ b/src/libstore/download.cc @@ -85,7 +85,7 @@ struct CurlDownloader : public Downloader DownloadItem(CurlDownloader & downloader, const DownloadRequest & request) : downloader(downloader) , request(request) - , act(actDownload, fmt("downloading '%s'", request.uri)) + , act(*logger, actDownload, fmt("downloading '%s'", request.uri)) { if (!request.expectedETag.empty()) requestHeaders = curl_slist_append(requestHeaders, ("If-None-Match: " + request.expectedETag).c_str()); diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 5aa3dcfd2..f52021061 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -565,7 +565,7 @@ void Store::buildPaths(const PathSet & paths, BuildMode buildMode) void copyStorePath(ref srcStore, ref dstStore, const Path & storePath, RepairFlag repair, CheckSigsFlag checkSigs) { - Activity act(actCopyPath, fmt("copying path '%s'", storePath)); + Activity act(*logger, actCopyPath, fmt("copying path '%s'", storePath)); auto info = srcStore->queryPathInfo(storePath); @@ -621,7 +621,7 @@ void copyPaths(ref srcStore, ref dstStore, const PathSet & storePa for (auto & path : storePaths) if (!valid.count(path)) missing.insert(path); - Activity act(actCopyPaths, fmt("copying %d paths", missing.size())); + Activity act(*logger, actCopyPaths, fmt("copying %d paths", missing.size())); std::atomic nrDone{0}; std::atomic bytesExpected{0}; @@ -646,7 +646,7 @@ void copyPaths(ref srcStore, ref dstStore, const PathSet & storePa auto info = srcStore->queryPathInfo(storePath); bytesExpected += info->narSize; - logger->event(evSetExpected, act, actCopyPath, bytesExpected); + act.setExpected(actCopyPath, bytesExpected); return info->references; }, @@ -655,7 +655,6 @@ void copyPaths(ref srcStore, ref dstStore, const PathSet & storePa checkInterrupt(); if (!dstStore->isValidPath(storePath)) { - printInfo("copying '%s'...", storePath); MaintainCount mc(nrRunning); showProgress(); copyStorePath(srcStore, dstStore, storePath, repair, checkSigs); diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index ed83770a1..900f24e4c 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -43,10 +43,6 @@ public: writeToStderr(prefix + (tty ? fs.s : filterANSIEscapes(fs.s)) + "\n"); } - - void event(const Event & ev) override - { - } }; Verbosity verbosity = lvlInfo; @@ -78,22 +74,10 @@ Logger * makeDefaultLogger() std::atomic nextId{(uint64_t) getpid() << 32}; -Activity::Activity() : id(nextId++) { }; - -Activity::Activity(ActivityType type, std::string msg) - : Activity() +Activity::Activity(Logger & logger, ActivityType type, const std::string & s) + : logger(logger), id(nextId++) { - logger->event(evStartActivity, id, type, msg); -} - -Activity::~Activity() -{ - logger->event(evStopActivity, id); -} - -void Activity::progress(uint64_t done, uint64_t expected, uint64_t running, uint64_t failed) const -{ - logger->event(evProgress, id, done, expected, running, failed); + logger.startActivity(id, type, s); } } diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 097d486ef..edc3553f6 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -23,63 +23,7 @@ typedef enum { actBuild = 105, } ActivityType; -class Activity -{ -public: - typedef uint64_t t; - const t id; - Activity(); - Activity(const Activity & act) : id(act.id) { }; - Activity(uint64_t id) : id(id) { }; - Activity(ActivityType type, std::string msg = ""); - ~Activity(); - - void progress(uint64_t done = 0, uint64_t expected = 0, uint64_t running = 0, uint64_t failed = 0) const; -}; - -typedef enum { - evBuildOutput = 2, - - evStartActivity = 1000, - evStopActivity = 1001, - evProgress = 1002, - evSetExpected = 1003, - -} EventType; - -struct Event -{ - struct Field - { - // FIXME: use std::variant. - enum { tInt, tString } type; - uint64_t i = 0; - std::string s; - Field(const std::string & s) : type(tString), s(s) { } - Field(const char * s) : type(tString), s(s) { } - Field(const uint64_t & i) : type(tInt), i(i) { } - Field(const Activity & act) : type(tInt), i(act.id) { } - }; - - typedef std::vector Fields; - - EventType type; - Fields fields; - - std::string getS(size_t n) const - { - assert(n < fields.size()); - assert(fields[n].type == Field::tString); - return fields[n].s; - } - - uint64_t getI(size_t n) const - { - assert(n < fields.size()); - assert(fields[n].type == Field::tInt); - return fields[n].i; - } -}; +typedef uint64_t ActivityId; class Logger { @@ -98,16 +42,38 @@ public: virtual void warn(const std::string & msg); - template - void event(EventType type, const Args & ... args) - { - Event ev; - ev.type = type; - nop{(ev.fields.emplace_back(Event::Field(args)), 1)...}; - event(ev); - } + virtual void startActivity(ActivityId act, ActivityType type, const std::string & s) { }; - virtual void event(const Event & ev) = 0; + virtual void stopActivity(ActivityId act) { }; + + virtual void progress(ActivityId act, uint64_t done = 0, uint64_t expected = 0, uint64_t running = 0, uint64_t failed = 0) { }; + + virtual void progress(ActivityId act, const std::string & s) { }; + + virtual void setExpected(ActivityId act, ActivityType type, uint64_t expected) { }; +}; + +struct Activity +{ + Logger & logger; + + const ActivityId id; + + Activity(Logger & logger, ActivityType type, const std::string & s = ""); + + ~Activity() + { logger.stopActivity(id); } + + void progress(uint64_t done = 0, uint64_t expected = 0, uint64_t running = 0, uint64_t failed = 0) const + { logger.progress(id, done, expected, running, failed); } + + void progress(const std::string & s) const + { logger.progress(id, s); } + + void setExpected(ActivityType type2, uint64_t expected) const + { logger.setExpected(id, type2, expected); } + + friend class Logger; }; extern Logger * logger; diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc index 2b223c882..7e6f3aa25 100644 --- a/src/nix-daemon/nix-daemon.cc +++ b/src/nix-daemon/nix-daemon.cc @@ -81,10 +81,6 @@ class TunnelLogger : public Logger } else defaultLogger->log(lvl, fs); } - - void event(const Event & ev) override - { - } }; diff --git a/src/nix/progress-bar.cc b/src/nix/progress-bar.cc index c79a0d6d1..628873708 100644 --- a/src/nix/progress-bar.cc +++ b/src/nix/progress-bar.cc @@ -29,7 +29,7 @@ private: struct ActivitiesByType { - std::map::iterator> its; + std::map::iterator> its; uint64_t done = 0; uint64_t expected = 0; uint64_t failed = 0; @@ -38,7 +38,7 @@ private: struct State { std::list activities; - std::map::iterator> its; + std::map::iterator> its; std::map activitiesByType; }; @@ -77,7 +77,61 @@ public: update(state); } - void createActivity(State & state, Activity::t activity, const std::string & s, ActivityType type = actUnknown) + void startActivity(ActivityId act, ActivityType type, const std::string & s) override + { + auto state(state_.lock()); + createActivity(*state, act, s, type); + update(*state); + } + + void stopActivity(ActivityId act) override + { + auto state(state_.lock()); + deleteActivity(*state, act); + update(*state); + } + + void progress(ActivityId act, uint64_t done = 0, uint64_t expected = 0, uint64_t running = 0, uint64_t failed = 0) override + { + auto state(state_.lock()); + + auto i = state->its.find(act); + assert(i != state->its.end()); + ActInfo & actInfo = *i->second; + actInfo.done = done; + actInfo.expected = expected; + actInfo.running = running; + actInfo.failed = failed; + + update(*state); + } + + void progress(ActivityId act, const std::string & s) override + { + auto state(state_.lock()); + auto s2 = trim(s); + if (!s2.empty()) { + updateActivity(*state, act, s2); + update(*state); + } + } + + void setExpected(ActivityId act, ActivityType type, uint64_t expected) override + { + auto state(state_.lock()); + + auto i = state->its.find(act); + assert(i != state->its.end()); + ActInfo & actInfo = *i->second; + auto & j = actInfo.expectedByType[type]; + state->activitiesByType[type].expected -= j; + j = expected; + state->activitiesByType[type].expected += j; + + update(*state); + } + + void createActivity(State & state, ActivityId activity, const std::string & s, ActivityType type = actUnknown) { state.activities.emplace_back(ActInfo{s, "", type}); auto i = std::prev(state.activities.end()); @@ -85,7 +139,7 @@ public: state.activitiesByType[type].its.emplace(activity, i); } - void deleteActivity(State & state, Activity::t activity) + void deleteActivity(State & state, ActivityId activity) { auto i = state.its.find(activity); if (i != state.its.end()) { @@ -102,7 +156,7 @@ public: } } - void updateActivity(State & state, Activity::t activity, const std::string & s2) + void updateActivity(State & state, ActivityId activity, const std::string & s2) { auto i = state.its.find(activity); assert(i != state.its.end()); @@ -210,51 +264,6 @@ public: return res; } - - void event(const Event & ev) override - { - auto state(state_.lock()); - - if (ev.type == evStartActivity) { - Activity::t act = ev.getI(0); - createActivity(*state, act, ev.getS(2), (ActivityType) ev.getI(1)); - } - - if (ev.type == evStopActivity) { - Activity::t act = ev.getI(0); - deleteActivity(*state, act); - } - - if (ev.type == evProgress) { - auto i = state->its.find(ev.getI(0)); - assert(i != state->its.end()); - ActInfo & actInfo = *i->second; - actInfo.done = ev.getI(1); - actInfo.expected = ev.getI(2); - actInfo.running = ev.getI(3); - actInfo.failed = ev.getI(4); - } - - if (ev.type == evSetExpected) { - auto i = state->its.find(ev.getI(0)); - assert(i != state->its.end()); - ActInfo & actInfo = *i->second; - auto type = (ActivityType) ev.getI(1); - auto & j = actInfo.expectedByType[type]; - state->activitiesByType[type].expected -= j; - j = ev.getI(2); - state->activitiesByType[type].expected += j; - } - - if (ev.type == evBuildOutput) { - Activity::t act = ev.getI(0); - auto s = trim(ev.getS(1)); - if (!s.empty()) - updateActivity(*state, act, s); - } - - update(*state); - } }; StartProgressBar::StartProgressBar()