From 37821ba7c12bd2e48f09fbf22be6abb4fcd2484d Mon Sep 17 00:00:00 2001 From: Hsuan-Ting Lu Date: Fri, 6 Dec 2019 22:59:45 +0800 Subject: [PATCH 1/4] Fix bug calling non-virtual destructor via base class --- pbar/pbar.cpp | 4 +++- pbar/pbar.hpp | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/pbar/pbar.cpp b/pbar/pbar.cpp index 6c95824..6c53f05 100644 --- a/pbar/pbar.cpp +++ b/pbar/pbar.cpp @@ -8,11 +8,11 @@ #include "pbar.hpp" +#include #include #include #include -#include #include "unistd.h" namespace pbar { @@ -25,10 +25,12 @@ int window_width::operator()() const { return 0; } // static_window_width static_window_width::static_window_width(const int width) : width(width) {} +static_window_width::~static_window_width() {} int static_window_width::operator()() const { return this->width; } // dynamic_window_width dynamic_window_width::dynamic_window_width() {} +dynamic_window_width::~dynamic_window_width() {} int dynamic_window_width::operator()() const { ioctl(STDOUT_FILENO, TIOCGWINSZ, &(this->window_size)); return static_cast(this->window_size.ws_col); diff --git a/pbar/pbar.hpp b/pbar/pbar.hpp index 9d639f8..27091a7 100644 --- a/pbar/pbar.hpp +++ b/pbar/pbar.hpp @@ -29,6 +29,7 @@ namespace window_width { class window_width { public: virtual int operator()() const; + virtual ~window_width() {} }; class static_window_width final : public window_width { @@ -38,6 +39,7 @@ class static_window_width final : public window_width { public: explicit static_window_width(const int width); int operator()() const final override; + ~static_window_width(); }; class dynamic_window_width final : public window_width { @@ -47,6 +49,7 @@ class dynamic_window_width final : public window_width { public: dynamic_window_width(); int operator()() const final override; + ~dynamic_window_width(); }; } // namespace window_width From adc60eac509fb6557b3aa05c89382fca16e6ca23 Mon Sep 17 00:00:00 2001 From: Hsuan-Ting Lu Date: Thu, 9 Jan 2020 13:57:39 +0800 Subject: [PATCH 2/4] Remove Buck build configs --- examples/BUCK | 14 -------------- pbar/BUCK | 20 -------------------- 2 files changed, 34 deletions(-) delete mode 100644 examples/BUCK delete mode 100644 pbar/BUCK diff --git a/examples/BUCK b/examples/BUCK deleted file mode 100644 index 183564d..0000000 --- a/examples/BUCK +++ /dev/null @@ -1,14 +0,0 @@ -# test driver -cxx_binary( - name = "testdrive", - srcs = [ - "main.cpp", - ], - link_style = "shared", - deps = [ - "//progressbar:progressbar", - "//logger:logger", - ], - deps_query = "$declared_deps", - labels = [], -) diff --git a/pbar/BUCK b/pbar/BUCK deleted file mode 100644 index 04f1bc0..0000000 --- a/pbar/BUCK +++ /dev/null @@ -1,20 +0,0 @@ -cxx_library( - name = "progressbar", - srcs = [ - "pbar.cpp", - ], - headers = [], - exported_headers = [ - "pbar.h", - ], - header_namespace = "progressbar", - link_style = "shared", - preferred_linkage = "shared", - deps = [ - "//utils:builtin-color-util", - ], - visibility = [ - "//examples:testdrive" - ], - labels = [], -) From b724ddfb898e5a8ff2b340e7478daa2f06b60e41 Mon Sep 17 00:00:00 2001 From: Hsuan-Ting Lu Date: Thu, 9 Jan 2020 16:32:09 +0800 Subject: [PATCH 3/4] Optimised update() and altered APIs regarding shutting down a pbar --- examples/main.cpp | 15 ++++--- pbar/pbar.cpp | 100 ++++++++++++++++++++++++++++++++-------------- pbar/pbar.hpp | 31 +++++++------- 3 files changed, 92 insertions(+), 54 deletions(-) diff --git a/examples/main.cpp b/examples/main.cpp index da5c507..98e2db0 100644 --- a/examples/main.cpp +++ b/examples/main.cpp @@ -15,19 +15,18 @@ int main() { constexpr int test_size = 4; constexpr int test_size2 = 80000; - pbar::ProgressBar* progressBar = - new pbar::ProgressBar("fiiirst", test_size); + pbar::ProgressBar pBar("fiiirst", test_size); + pbar::ProgressBar pBar2("secoond", test_size2, true); for (int i = 0; i < test_size; i++) { - pbar::ProgressBar* progressBar2 = - new pbar::ProgressBar("secoond", test_size2, true); + pBar2.reset(); for (int j = 0; j < test_size2; j++) { - progressBar2->update(); + pBar2.update(); usleep(1); } - delete progressBar2; - progressBar->update(); + pBar.update(); } - delete progressBar; + pBar2.close(); + pBar.close(); std::cout << std::endl << std::endl; std::cout << "test driver END" << std::endl; diff --git a/pbar/pbar.cpp b/pbar/pbar.cpp index 6c53f05..f9a6452 100644 --- a/pbar/pbar.cpp +++ b/pbar/pbar.cpp @@ -37,12 +37,13 @@ int dynamic_window_width::operator()() const { } } // namespace window_width -ProgressBar::ProgressBar(const std::string& description, const int total, +ProgressBar::ProgressBar(const std::string& description, const long long& total, const bool leave, const int width, const std::chrono::nanoseconds min_interval_time, - const std::string& bar_format, const int initial_value, - const int position) - : description(description), + const std::string& bar_format, + const long long& initial_value, const int position) + : initial_value(initial_value), + description(description), total(total), leave(leave), min_interval_time(min_interval_time), @@ -53,7 +54,8 @@ ProgressBar::ProgressBar(const std::string& description, const int total, n(initial_value), position(position), last_update_n(initial_value), - last_update_time(std::chrono::system_clock::now()) { + last_update_time(std::chrono::system_clock::now()), + disable(false) { if (width > 0) { this->window_width = new window_width::static_window_width(width); } else { @@ -63,28 +65,14 @@ ProgressBar::ProgressBar(const std::string& description, const int total, this->display(); } -ProgressBar::ProgressBar(const std::string& description, const int total, +ProgressBar::ProgressBar(const std::string& description, const long long& total, const bool leave) : ProgressBar(description, total, leave, -1, std::chrono::duration_cast( - std::chrono::milliseconds(100)), + std::chrono::milliseconds(10)), "", 0, ProgressBar::nbars) {} -ProgressBar::~ProgressBar() { - this->display(); - /* Cleanup and (depends on config) close the progressbar - */ - // TODO: think this through - if (this->leave) { - // leave this bar as it is - } else { - this->moveto(this->position); - std::cout << "\r" << aesc::cursor::EL(aesc::cursor::clear::to_end); - this->moveto(-this->position); - } - ProgressBar::nbars -= 1; - delete window_width; -} +ProgressBar::~ProgressBar() { this->close(); } inline float ProgressBar::percentage() { // TODO: deprecate and merge @@ -110,7 +98,7 @@ void ProgressBar::moveto(const int n) { */ if (n > 0) { - // moves down + // moves down, needs NEWLINE to actually create and move to lines below for (int i = 0; i != n; ++i) { std::cout << "\n"; } @@ -186,7 +174,25 @@ void ProgressBar::display() { std::cout << std::flush; } +const std::chrono::nanoseconds ProgressBar::delta_time( + std::chrono::time_point& now) { + return std::chrono::duration_cast( + now - this->last_update_time); +} + +inline long ProgressBar::delta_iter() { + return static_cast(this->n - this->last_update_n); +} + void ProgressBar::update(const int n) { + if (this->disable) { + return; + } + + // HACK: for auto-refresh logic to work + if (n < 0) { + this->last_update_n += n; + } this->n += n; // BUG: TODO: consider last_print_n when n < 0 @@ -199,24 +205,56 @@ void ProgressBar::update(const int n) { if (this->delta_time(now) > this->min_interval_time) { this->display(); - // TODO: dynamic min_interval_iter adjustments - this->min_interval_iter = delta_iters / 3; + // TODO: find better dynamic min_interval_iter adjustments, might + // want to consider interval fluctuation Uses max number of + // iterations between two updates + this->min_interval_iter = + std::max(this->min_interval_iter, delta_iters); // TODO: how to evaluate this guess on next round + // Store old values for next call this->last_update_n = this->n; this->last_update_time = std::move(now); } } } -const std::chrono::nanoseconds ProgressBar::delta_time( - std::chrono::time_point& now) { - return std::chrono::duration_cast( - now - this->last_update_time); +void ProgressBar::close() { + /* Clean up visuals, closes pbar if set not to leave it + * then resets + */ + if (this->disable) { + return; + } + // Prevent multiple closures + this->disable = true; + + // Remove pbar from record + ProgressBar::nbars -= 1; + // TODO: implement internal set instead of simple counting to maintain + // multi-bar order + delete window_width; + + this->display(); + // TODO: write final stats + + if (this->leave) { + // leave this bar as it is + } else { + // close (clean up) the pbar + this->moveto(this->position); + std::cout << "\r" << aesc::cursor::EL(aesc::cursor::clear::to_end); + this->moveto(-this->position); + } } -long ProgressBar::delta_iter() { - return static_cast(this->n - this->last_update_n); +void ProgressBar::reset() { + /* encourages repeated use + */ + this->n = this->initial_value; + this->last_update_n = this->initial_value; + this->last_update_time = std::chrono::system_clock::now(); + this->display(); } } // namespace pbar diff --git a/pbar/pbar.hpp b/pbar/pbar.hpp index 27091a7..cfcee32 100644 --- a/pbar/pbar.hpp +++ b/pbar/pbar.hpp @@ -18,12 +18,6 @@ namespace pbar { -class Bar { - public: - Bar(); - void refresh(); -}; - namespace window_width { // Functor class window_width { @@ -51,25 +45,30 @@ class dynamic_window_width final : public window_width { int operator()() const final override; ~dynamic_window_width(); }; + } // namespace window_width class ProgressBar { - /* Not copyable, but movable + /* TODO: Not copyable, but movable */ private: static int nbars; // bar count + private: // initial values, used in reset() + long long initial_value; + private: - const std::string description; - const long long total; + std::string description; + long long total; const bool leave; std::chrono::nanoseconds min_interval_time; long min_interval_iter; - const std::string bar_format; + std::string bar_format; long long n; const int position; long long last_update_n; std::chrono::system_clock::time_point last_update_time; + bool disable; private: inline float percentage(); @@ -81,19 +80,21 @@ class ProgressBar { void display(); const std::chrono::nanoseconds delta_time( std::chrono::time_point& now); - long delta_iter(); + inline long delta_iter(); public: - explicit ProgressBar(const std::string& description, const int total, + explicit ProgressBar(const std::string& description, const long long& total, const bool leave, const int width, const std::chrono::nanoseconds min_interval_time, - const std::string& bar_format, const int initial_value, - const int position); - explicit ProgressBar(const std::string& description, const int total, + const std::string& bar_format, + const long long& initial_value, const int position); + explicit ProgressBar(const std::string& description, const long long& total, const bool leave = (ProgressBar::nbars ? false : true)); ~ProgressBar(); void update(const int n = 1); + void close(); + void reset(); }; } // namespace pbar From b484f153c23adb378dbbbdbf04cffdffce3fbfe3 Mon Sep 17 00:00:00 2001 From: Hsuan-Ting Lu Date: Thu, 9 Jan 2020 17:59:58 +0800 Subject: [PATCH 4/4] Guard critical variables and standard output procedures with mutex --- pbar/pbar.cpp | 14 ++++++++++++-- pbar/pbar.hpp | 7 +++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/pbar/pbar.cpp b/pbar/pbar.cpp index f9a6452..0f2c35a 100644 --- a/pbar/pbar.cpp +++ b/pbar/pbar.cpp @@ -161,6 +161,8 @@ std::string ProgressBar::format_meter() { void ProgressBar::display() { /** Refresh display of this bar */ + const std::lock_guard guard(this->pbar_mutex); + if (this->position) { this->moveto(this->position); } @@ -213,8 +215,14 @@ void ProgressBar::update(const int n) { // TODO: how to evaluate this guess on next round // Store old values for next call - this->last_update_n = this->n; - this->last_update_time = std::move(now); + { + std::lock_guard guard(this->pbar_mutex); + // this->last_update_n = this->n; + const long long tmp = this->n.load(); + this->last_update_n = tmp; + + this->last_update_time = std::move(now); + } } } } @@ -242,6 +250,8 @@ void ProgressBar::close() { // leave this bar as it is } else { // close (clean up) the pbar + const std::lock_guard guard(this->pbar_mutex); + this->moveto(this->position); std::cout << "\r" << aesc::cursor::EL(aesc::cursor::clear::to_end); this->moveto(-this->position); diff --git a/pbar/pbar.hpp b/pbar/pbar.hpp index cfcee32..1249824 100644 --- a/pbar/pbar.hpp +++ b/pbar/pbar.hpp @@ -11,7 +11,9 @@ #include +#include #include +#include #include #include #include @@ -64,11 +66,12 @@ class ProgressBar { std::chrono::nanoseconds min_interval_time; long min_interval_iter; std::string bar_format; - long long n; + std::atomic n; const int position; - long long last_update_n; + std::atomic last_update_n; std::chrono::system_clock::time_point last_update_time; bool disable; + std::mutex pbar_mutex; private: inline float percentage();