From 0e0dcf2c7ec054f1b30629e275f53f56039b8257 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 14 Aug 2017 22:12:36 +0200 Subject: [PATCH] Progress indicator: Unify "copying" and "substituting" They're the same thing after all. Example: $ nix build --store local?root=/tmp/nix nixpkgs.firefox-unwrapped [0/1 built, 49/98 copied, 16.3/92.8 MiB DL, 55.8/309.2 MiB copied] downloading 'https://cache.nixos.org/nar/0pl9li1jigcj2dany47hpmn0r3r48wc4nz48v5mqhh426lgz3bz6.nar.xz' --- src/libstore/build.cc | 19 +++++++---- src/libstore/store-api.cc | 11 ++---- src/libutil/logging.hh | 11 ++---- src/nix/progress-bar.cc | 70 +++------------------------------------ 4 files changed, 24 insertions(+), 87 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index d5f48237b..9ea4e4b88 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -207,7 +207,7 @@ struct MaintainCount { T & counter; T delta; - MaintainCount(T & counter, T delta) : counter(counter), delta(delta) { counter += delta; } + MaintainCount(T & counter, T delta = 1) : counter(counter), delta(delta) { counter += delta; } ~MaintainCount() { counter -= delta; } }; @@ -256,6 +256,7 @@ private: public: const Activity act; + const Activity actSubstitutions; /* Set if at least one derivation had a BuildError (i.e. permanent failure). */ @@ -268,6 +269,8 @@ public: std::unique_ptr hook; + uint64_t expectedSubstitutions = 0; + uint64_t doneSubstitutions = 0; uint64_t expectedDownloadSize = 0; uint64_t doneDownloadSize = 0; uint64_t expectedNarSize = 0; @@ -334,6 +337,7 @@ public: void updateProgress() { + actSubstitutions.progress(doneSubstitutions, expectedSubstitutions + doneSubstitutions); logger->event(evSetExpected, act, actDownload, expectedDownloadSize + doneDownloadSize); logger->event(evSetExpected, act, actCopyPath, expectedNarSize + doneNarSize); } @@ -3328,7 +3332,8 @@ private: storePath when doing a repair. */ Path destPath; - std::unique_ptr> maintainExpectedNar, maintainExpectedDownload; + std::unique_ptr> maintainExpectedSubstitutions, + maintainExpectedNar, maintainExpectedDownload; typedef void (SubstitutionGoal::*GoalState)(); GoalState state; @@ -3364,7 +3369,6 @@ public: void amDone(ExitCode result) override { - logger->event(evSubstitutionFinished, act, result == ecSuccess); Goal::amDone(result); } }; @@ -3379,7 +3383,7 @@ SubstitutionGoal::SubstitutionGoal(const Path & storePath, Worker & worker, Repa state = &SubstitutionGoal::init; name = (format("substitution of '%1%'") % storePath).str(); trace("created"); - logger->event(evSubstitutionCreated, act, storePath); + maintainExpectedSubstitutions = std::make_unique>(worker.expectedSubstitutions); } @@ -3527,8 +3531,6 @@ void SubstitutionGoal::tryToRun() printInfo(format("fetching path '%1%'...") % storePath); - logger->event(evSubstitutionStarted, act); - outPipe.create(); promise = std::promise(); @@ -3576,6 +3578,9 @@ void SubstitutionGoal::finished() printMsg(lvlChatty, format("substitution of path '%1%' succeeded") % storePath); + maintainExpectedSubstitutions.reset(); + worker.doneSubstitutions++; + if (maintainExpectedDownload) { auto fileSize = maintainExpectedDownload->delta; maintainExpectedDownload.reset(); @@ -3610,6 +3615,7 @@ static bool working = false; Worker::Worker(LocalStore & store) : act(actRealise) + , actSubstitutions(actCopyPaths) , store(store) { /* Debugging: prevent recursive workers. */ @@ -3632,6 +3638,7 @@ Worker::~Worker() their destructors). */ topGoals.clear(); + assert(expectedSubstitutions == 0); assert(expectedDownloadSize == 0); assert(expectedNarSize == 0); } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 2b99a07e2..1e11e54eb 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -621,16 +621,13 @@ void copyPaths(ref srcStore, ref dstStore, const PathSet & storePa for (auto & path : storePaths) if (!valid.count(path)) missing.insert(path); - Activity act(actUnknown); + Activity act(actCopyPaths, fmt("copying %d paths", missing.size())); - logger->event(evCopyStarted, act); - - std::atomic nrCopied{0}; - std::atomic nrDone{storePaths.size() - missing.size()}; + std::atomic nrDone{0}; std::atomic bytesExpected{0}; auto showProgress = [&]() { - logger->event(evCopyProgress, act, storePaths.size(), nrCopied, nrDone); + act.progress(nrDone, missing.size()); }; ThreadPool pool; @@ -659,11 +656,9 @@ void copyPaths(ref srcStore, ref dstStore, const PathSet & storePa if (!dstStore->isValidPath(storePath)) { printInfo("copying '%s'...", storePath); copyStorePath(srcStore, dstStore, storePath, repair, checkSigs); - nrCopied++; } nrDone++; - showProgress(); }); } diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 7633408ea..f3ff099f0 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -18,6 +18,7 @@ typedef enum { actCopyPath = 100, actDownload = 101, actRealise = 102, + actCopyPaths = 103, } ActivityType; class Activity @@ -32,7 +33,7 @@ public: ~Activity(); template - void progress(const Args & ... args); + void progress(const Args & ... args) const; }; typedef enum { @@ -40,12 +41,6 @@ typedef enum { evBuildStarted = 1, evBuildOutput = 2, evBuildFinished = 3, - evSubstitutionCreated = 8, - evSubstitutionStarted = 9, - evSubstitutionFinished = 10, - - evCopyStarted = 100, - evCopyProgress = 101, evStartActivity = 1000, evStopActivity = 1001, @@ -152,7 +147,7 @@ void warnOnce(bool & haveWarned, const FormatOrString & fs); void writeToStderr(const string & s); template -void Activity::progress(const Args & ... args) +void Activity::progress(const Args & ... args) const { Event ev; ev.type = evProgress; diff --git a/src/nix/progress-bar.cc b/src/nix/progress-bar.cc index 736eb5ede..21d89e477 100644 --- a/src/nix/progress-bar.cc +++ b/src/nix/progress-bar.cc @@ -25,13 +25,6 @@ private: std::map expectedByType; }; - struct CopyInfo - { - uint64_t expected = 0; - uint64_t copied = 0; - uint64_t done = 0; - }; - struct ActivitiesByType { std::map::iterator> its; @@ -46,12 +39,6 @@ private: uint64_t succeededBuilds = 0; uint64_t failedBuilds = 0; - std::map substitutions; - std::set runningSubstitutions; - uint64_t succeededSubstitutions = 0; - - std::map runningCopies; - std::list activities; std::map::iterator> its; @@ -185,25 +172,7 @@ public: state.succeededBuilds, state.succeededBuilds + state.builds.size()); } - if (!state.substitutions.empty() || state.succeededSubstitutions) { - if (!res.empty()) res += ", "; - if (!state.runningSubstitutions.empty()) - res += fmt(ANSI_BLUE "%d" "/" ANSI_NORMAL, state.runningSubstitutions.size()); - res += fmt(ANSI_GREEN "%d/%d fetched" ANSI_NORMAL, - state.succeededSubstitutions, - state.succeededSubstitutions + state.substitutions.size()); - } - - if (!state.runningCopies.empty()) { - uint64_t copied = 0, expected = 0; - for (auto & i : state.runningCopies) { - copied += i.second.copied; - expected += i.second.expected - (i.second.done - i.second.copied); - } - add(fmt("%d/%d copied", copied, expected)); - } - - auto showActivity = [&](ActivityType type, const std::string & f) { + auto showActivity = [&](ActivityType type, const std::string & f, double unit) { auto & act = state.activitiesByType[type]; uint64_t done = act.done, expected = act.done; for (auto & j : act.its) { @@ -214,11 +183,12 @@ public: expected = std::max(expected, act.expected); if (done || expected) - add(fmt(f, done / MiB, expected / MiB)); + add(fmt(f, done / unit, expected / unit)); }; - showActivity(actDownload, "%1$.1f/%2$.1f MiB DL"); - showActivity(actCopyPath, "%1$.1f/%2$.1f MiB copied"); + showActivity(actCopyPaths, ANSI_GREEN "%d" ANSI_NORMAL "/%d copied", 1); + showActivity(actDownload, "%1$.1f/%2$.1f MiB DL", MiB); + showActivity(actCopyPath, "%1$.1f/%2$.1f MiB copied", MiB); return res; } @@ -287,36 +257,6 @@ public: updateActivity(*state, act, ev.getS(1)); } - if (ev.type == evSubstitutionCreated) { - state->substitutions[ev.getI(0)] = ev.getS(1); - } - - if (ev.type == evSubstitutionStarted) { - Activity::t act = ev.getI(0); - state->runningSubstitutions.insert(act); - auto name = storePathToName(state->substitutions[act]); - createActivity(*state, act, fmt("fetching " ANSI_BOLD "%s" ANSI_NORMAL, name)); - } - - if (ev.type == evSubstitutionFinished) { - Activity::t act = ev.getI(0); - if (ev.getI(1)) { - if (state->runningSubstitutions.count(act)) - state->succeededSubstitutions++; - } - state->runningSubstitutions.erase(act); - state->substitutions.erase(act); - deleteActivity(*state, act); - } - - if (ev.type == evCopyProgress) { - Activity::t act = ev.getI(0); - auto & i = state->runningCopies[act]; - i.expected = ev.getI(1); - i.copied = ev.getI(2); - i.done = ev.getI(3); - } - update(*state); } };