From 874b14e6a06dc303ed3adb5069e06fe4b13fa7ba Mon Sep 17 00:00:00 2001 From: MSebanc Date: Mon, 18 Mar 2024 16:41:01 -0700 Subject: [PATCH 1/9] Allowed for progress bar to be configurable by CALL I have read and agree to the CLA of the Kuzu repository. --- src/include/common/constants.h | 1 + src/include/common/task_system/progress_bar.h | 2 +- src/include/main/client_config.h | 2 ++ src/include/main/client_context.h | 8 +++---- src/include/main/settings.h | 12 ++++++++++ src/main/client_context.cpp | 11 ++++++++++ src/main/db_config.cpp | 3 ++- tools/shell/embedded_shell.cpp | 22 ++----------------- tools/shell/include/embedded_shell.h | 2 -- tools/shell/test/test_shell_commands.py | 3 +-- 10 files changed, 36 insertions(+), 30 deletions(-) diff --git a/src/include/common/constants.h b/src/include/common/constants.h index 96e6091a89..35a339b468 100644 --- a/src/include/common/constants.h +++ b/src/include/common/constants.h @@ -178,6 +178,7 @@ struct ClientConfigDefault { static constexpr uint64_t TIMEOUT_IN_MS = 0; static constexpr uint32_t VAR_LENGTH_MAX_DEPTH = 30; static constexpr bool ENABLE_SEMI_MASK = true; + static constexpr bool PROGRESS_BAR = true; }; struct OrderByConstants { diff --git a/src/include/common/task_system/progress_bar.h b/src/include/common/task_system/progress_bar.h index bfd0050e3a..1975979c0f 100644 --- a/src/include/common/task_system/progress_bar.h +++ b/src/include/common/task_system/progress_bar.h @@ -7,7 +7,7 @@ namespace kuzu { namespace common { /** - * TODO: PUT DESCRIPTION HERE + * @brief Progress bar for tracking the progress of a pipeline. Prints the progress of each query pipeline and the overall progress. */ class ProgressBar { diff --git a/src/include/main/client_config.h b/src/include/main/client_config.h index 8649c5bd2c..aca18fab54 100644 --- a/src/include/main/client_config.h +++ b/src/include/main/client_config.h @@ -18,6 +18,8 @@ struct ClientConfig { uint64_t timeoutInMS; // variable length maximum depth uint32_t varLengthMaxDepth; + // If using progress bar + bool progressBar; }; } // namespace main diff --git a/src/include/main/client_context.h b/src/include/main/client_context.h index 0cf4083029..5eaf80393a 100644 --- a/src/include/main/client_context.h +++ b/src/include/main/client_context.h @@ -79,6 +79,10 @@ class ClientContext { transaction::Transaction* getTx() const; KUZU_API transaction::TransactionContext* getTransactionContext() const; + // Progress bar + void setProgressBarPrinting(bool progressBarPrinting); + common::ProgressBar* getProgressBar() const; + // Replace function. inline bool hasReplaceFunc() { return replaceFunc != nullptr; } inline void setReplaceFunc(replace_func_t func) { replaceFunc = func; } @@ -106,10 +110,6 @@ class ClientContext { std::unique_ptr query(std::string_view queryStatement); void runQuery(std::string query); - void setProgressBarPrinting(bool progressBarPrinting); - - common::ProgressBar* getProgressBar() const { return progressBar.get(); } - private: std::unique_ptr query( std::string_view query, std::string_view encodedJoin, bool enumerateAllPlans = true); diff --git a/src/include/main/settings.h b/src/include/main/settings.h index a31ddd1f62..2f2fe31074 100644 --- a/src/include/main/settings.h +++ b/src/include/main/settings.h @@ -30,6 +30,18 @@ struct TimeoutSetting { } }; +struct ProgressBarSetting { + static constexpr const char* name = "progress_bar"; + static constexpr const common::LogicalTypeID inputType = common::LogicalTypeID::BOOL; + static void setContext(ClientContext* context, const common::Value& parameter) { + KU_ASSERT(parameter.getDataType()->getLogicalTypeID() == common::LogicalTypeID::BOOL); + context->getClientConfigUnsafe()->progressBar = parameter.getValue(); + } + static common::Value getSetting(ClientContext* context) { + return common::Value(context->getClientConfig()->progressBar); + } +}; + struct VarLengthExtendMaxDepthSetting { static constexpr const char* name = "var_length_extend_max_depth"; static constexpr const common::LogicalTypeID inputType = common::LogicalTypeID::INT64; diff --git a/src/main/client_context.cpp b/src/main/client_context.cpp index 1eac0c94ed..10145a4d4c 100644 --- a/src/main/client_context.cpp +++ b/src/main/client_context.cpp @@ -56,6 +56,7 @@ ClientContext::ClientContext(Database* database) : database{database} { config.numThreads = database->systemConfig.maxNumThreads; config.timeoutInMS = ClientConfigDefault::TIMEOUT_IN_MS; config.varLengthMaxDepth = ClientConfigDefault::VAR_LENGTH_MAX_DEPTH; + config.progressBar = ClientConfigDefault::PROGRESS_BAR; } uint64_t ClientContext::getTimeoutRemainingInMS() const { @@ -116,6 +117,16 @@ TransactionContext* ClientContext::getTransactionContext() const { return transactionContext.get(); } +void ClientContext::setProgressBarPrinting(bool progressBarPrinting) { + lock_t lck{mtx}; + config.progressBar = progressBarPrinting; + progressBar->toggleProgressBarPrinting(progressBarPrinting); +} + +common::ProgressBar* ClientContext::getProgressBar() const { + return progressBar.get(); +} + void ClientContext::setExtensionOption(std::string name, common::Value value) { StringUtils::toLower(name); extensionOptionValues.insert_or_assign(name, std::move(value)); diff --git a/src/main/db_config.cpp b/src/main/db_config.cpp index ad567a4320..dfcfe1756e 100644 --- a/src/main/db_config.cpp +++ b/src/main/db_config.cpp @@ -14,7 +14,8 @@ namespace main { static ConfigurationOption options[] = { // NOLINT(cert-err58-cpp): GET_CONFIGURATION(ThreadsSetting), GET_CONFIGURATION(TimeoutSetting), GET_CONFIGURATION(VarLengthExtendMaxDepthSetting), GET_CONFIGURATION(EnableSemiMaskSetting), - GET_CONFIGURATION(HomeDirectorySetting), GET_CONFIGURATION(FileSearchPathSetting)}; + GET_CONFIGURATION(HomeDirectorySetting), GET_CONFIGURATION(FileSearchPathSetting), + GET_CONFIGURATION(ProgressBarSetting)}; ConfigurationOption* DBConfig::getOptionByName(const std::string& optionName) { auto lOptionName = optionName; diff --git a/tools/shell/embedded_shell.cpp b/tools/shell/embedded_shell.cpp index 68d6ccade2..a82dd9ff05 100644 --- a/tools/shell/embedded_shell.cpp +++ b/tools/shell/embedded_shell.cpp @@ -51,8 +51,7 @@ struct ShellCommand { const char* QUIT = ":quit"; const char* MAX_ROWS = ":max_rows"; const char* MAX_WIDTH = ":max_width"; - const char* PROGRESS_BAR = ":progress_bar"; - const std::array commandList = {HELP, CLEAR, QUIT, MAX_ROWS, MAX_WIDTH, PROGRESS_BAR}; + const std::array commandList = {HELP, CLEAR, QUIT, MAX_ROWS, MAX_WIDTH}; } shellCommand; const char* TAB = " "; @@ -285,8 +284,6 @@ int EmbeddedShell::processShellCommands(std::string lineStr) { setMaxRows(lineStr.substr(strlen(shellCommand.MAX_ROWS))); } else if (lineStr.rfind(shellCommand.MAX_WIDTH) == 0) { setMaxWidth(lineStr.substr(strlen(shellCommand.MAX_WIDTH))); - } else if (lineStr.rfind(shellCommand.PROGRESS_BAR) == 0) { - toggleProgressBar(lineStr.substr(strlen(shellCommand.PROGRESS_BAR))); } else { printf("Error: Unknown command: \"%s\". Enter \":help\" for help\n", lineStr.c_str()); printf("Did you mean: \"%s\"?\n", findClosestCommand(lineStr).c_str()); @@ -430,20 +427,6 @@ void EmbeddedShell::setMaxWidth(const std::string& maxWidthString) { printf("maxWidth set as %d\n", parsedMaxWidth); } -void EmbeddedShell::toggleProgressBar(const std::string& state) { - std::string stateTrimmed = state; - stateTrimmed = stateTrimmed.erase(0, state.find_first_not_of(" \t\n\r\f\v")); - if (stateTrimmed == "on") { - conn->setProgressBarPrinting(true); - printf("Turned progress bar on.\n"); - } else if (stateTrimmed == "off") { - conn->setProgressBarPrinting(false); - printf("Turned progress bar off\n"); - } else { - printf("Cannot parse '%s' as progress bar state. Expect on|off.\n", stateTrimmed.c_str()); - } -} - void EmbeddedShell::printHelp() { printf("%s%s %sget command list\n", TAB, shellCommand.HELP, TAB); printf("%s%s %sclear shell\n", TAB, shellCommand.CLEAR, TAB); @@ -452,11 +435,10 @@ void EmbeddedShell::printHelp() { shellCommand.MAX_ROWS, TAB); printf("%s%s [max_width] %sset maximum width in characters for display\n", TAB, shellCommand.MAX_WIDTH, TAB); - printf("%s%s [on|off] %stoggle progress bar for queries\n", TAB, shellCommand.PROGRESS_BAR, TAB); printf("\n"); printf("%sNote: you can change and see several system configurations, such as num-threads, \n", TAB); - printf("%s%s timeout, and logging_level using Cypher CALL statements.\n", TAB, TAB); + printf("%s%s timeout, and progress_bar using Cypher CALL statements.\n", TAB, TAB); printf("%s%s e.g. CALL THREADS=5; or CALL current_setting('threads') return *;\n", TAB, TAB); printf("%s%s See: https://docs.kuzudb.com/cypher/configuration\n", TAB, TAB); } diff --git a/tools/shell/include/embedded_shell.h b/tools/shell/include/embedded_shell.h index 8aa44e6c04..7b5171f455 100644 --- a/tools/shell/include/embedded_shell.h +++ b/tools/shell/include/embedded_shell.h @@ -32,8 +32,6 @@ class EmbeddedShell { void setMaxWidth(const std::string& maxWidthString); - void toggleProgressBar(const std::string& state); - private: std::unique_ptr database; std::unique_ptr conn; diff --git a/tools/shell/test/test_shell_commands.py b/tools/shell/test/test_shell_commands.py index f402eba89c..ed610c2dbd 100644 --- a/tools/shell/test/test_shell_commands.py +++ b/tools/shell/test/test_shell_commands.py @@ -11,10 +11,9 @@ def test_help(temp_db) -> None: " :quit exit from shell", " :max_rows [max_rows] set maximum number of rows for display (default: 20)", " :max_width [max_width] set maximum width in characters for display", - " :progress_bar [on|off] toggle progress bar for queries", "", " Note: you can change and see several system configurations, such as num-threads, ", - " timeout, and logging_level using Cypher CALL statements.", + " timeout, and progress_bar using Cypher CALL statements.", " e.g. CALL THREADS=5; or CALL current_setting('threads') return *;", " See: https://docs.kuzudb.com/cypher/configuration", ], From 72688bba97fa11603ab2376c95fd054766d6d4b3 Mon Sep 17 00:00:00 2001 From: MSebanc Date: Mon, 18 Mar 2024 16:45:04 -0700 Subject: [PATCH 2/9] Format fixes I have read and agree to the CLA of the Kuzu repository. --- src/include/common/task_system/progress_bar.h | 3 ++- src/include/main/settings.h | 14 +++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/include/common/task_system/progress_bar.h b/src/include/common/task_system/progress_bar.h index 1975979c0f..a4e2b74329 100644 --- a/src/include/common/task_system/progress_bar.h +++ b/src/include/common/task_system/progress_bar.h @@ -7,7 +7,8 @@ namespace kuzu { namespace common { /** - * @brief Progress bar for tracking the progress of a pipeline. Prints the progress of each query pipeline and the overall progress. + * @brief Progress bar for tracking the progress of a pipeline. Prints the progress of each query + * pipeline and the overall progress. */ class ProgressBar { diff --git a/src/include/main/settings.h b/src/include/main/settings.h index 2f2fe31074..37bbca4603 100644 --- a/src/include/main/settings.h +++ b/src/include/main/settings.h @@ -31,15 +31,15 @@ struct TimeoutSetting { }; struct ProgressBarSetting { - static constexpr const char* name = "progress_bar"; - static constexpr const common::LogicalTypeID inputType = common::LogicalTypeID::BOOL; + static constexpr const char* name = "progress_bar"; + static constexpr const common::LogicalTypeID inputType = common::LogicalTypeID::BOOL; static void setContext(ClientContext* context, const common::Value& parameter) { - KU_ASSERT(parameter.getDataType()->getLogicalTypeID() == common::LogicalTypeID::BOOL); - context->getClientConfigUnsafe()->progressBar = parameter.getValue(); - } + KU_ASSERT(parameter.getDataType()->getLogicalTypeID() == common::LogicalTypeID::BOOL); + context->getClientConfigUnsafe()->progressBar = parameter.getValue(); + } static common::Value getSetting(ClientContext* context) { - return common::Value(context->getClientConfig()->progressBar); - } + return common::Value(context->getClientConfig()->progressBar); + } }; struct VarLengthExtendMaxDepthSetting { From 7e1f3614aa5d5e2a9729b3fac88ae13bfbfaa480 Mon Sep 17 00:00:00 2001 From: MSebanc Date: Mon, 18 Mar 2024 16:47:18 -0700 Subject: [PATCH 3/9] Format fixes I have read and agree to the CLA of the Kuzu repository. --- src/include/common/task_system/progress_bar.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/common/task_system/progress_bar.h b/src/include/common/task_system/progress_bar.h index a4e2b74329..f95d42a040 100644 --- a/src/include/common/task_system/progress_bar.h +++ b/src/include/common/task_system/progress_bar.h @@ -7,7 +7,7 @@ namespace kuzu { namespace common { /** - * @brief Progress bar for tracking the progress of a pipeline. Prints the progress of each query + * @brief Progress bar for tracking the progress of a pipeline. Prints the progress of each query * pipeline and the overall progress. */ class ProgressBar { From c443ae4bbe048ea74fb402b8191083c127be5b2b Mon Sep 17 00:00:00 2001 From: MSebanc Date: Tue, 19 Mar 2024 09:14:18 -0700 Subject: [PATCH 4/9] Implemented feedback and fixed call bug I have read and agree to the CLA of the Kuzu repository. --- src/common/task_system/progress_bar.cpp | 14 +++++++++----- src/include/common/constants.h | 8 -------- src/include/common/task_system/progress_bar.h | 2 ++ src/include/main/client_config.h | 10 +++++++++- src/include/main/client_context.h | 1 - src/include/main/settings.h | 5 +++-- src/main/client_context.cpp | 8 +------- src/processor/processor.cpp | 1 + test/test_files/tinysnb/call/call.test | 12 ++++++++++++ 9 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/common/task_system/progress_bar.cpp b/src/common/task_system/progress_bar.cpp index 3efb778a1e..dc01b14bbf 100644 --- a/src/common/task_system/progress_bar.cpp +++ b/src/common/task_system/progress_bar.cpp @@ -11,6 +11,14 @@ void ProgressBar::startProgress() { printProgressBar(0.0); } +void ProgressBar::endProgress() { + if (!trackProgress) { + return; + } + std::lock_guard lock(progressBarLock); + resetProgressBar(); +} + void ProgressBar::addPipeline() { if (!trackProgress) { return; @@ -23,11 +31,8 @@ void ProgressBar::finishPipeline() { return; } numPipelinesFinished++; - prevCurPipelineProgress = 0.0; + prevCurPipelineProgress = -0.01; updateProgress(0.0); - if (numPipelines == numPipelinesFinished) { - resetProgressBar(); - } } void ProgressBar::updateProgress(double curPipelineProgress) { @@ -55,7 +60,6 @@ void ProgressBar::printProgressBar(double curPipelineProgress) const { } void ProgressBar::resetProgressBar() { - std::lock_guard lock(progressBarLock); std::cout << "\033[2A\033[2K\033[1B\033[2K\033[1A"; std::cout.flush(); numPipelines = 0; diff --git a/src/include/common/constants.h b/src/include/common/constants.h index 35a339b468..2a1674387e 100644 --- a/src/include/common/constants.h +++ b/src/include/common/constants.h @@ -173,14 +173,6 @@ struct PlannerKnobs { static constexpr uint64_t SIP_RATIO = 5; }; -struct ClientConfigDefault { - // 0 means timeout is disabled by default. - static constexpr uint64_t TIMEOUT_IN_MS = 0; - static constexpr uint32_t VAR_LENGTH_MAX_DEPTH = 30; - static constexpr bool ENABLE_SEMI_MASK = true; - static constexpr bool PROGRESS_BAR = true; -}; - struct OrderByConstants { static constexpr uint64_t NUM_BYTES_FOR_PAYLOAD_IDX = 8; static constexpr uint64_t MIN_SIZE_TO_REDUCE = common::DEFAULT_VECTOR_CAPACITY * 5; diff --git a/src/include/common/task_system/progress_bar.h b/src/include/common/task_system/progress_bar.h index f95d42a040..a5e43f7579 100644 --- a/src/include/common/task_system/progress_bar.h +++ b/src/include/common/task_system/progress_bar.h @@ -21,6 +21,8 @@ class ProgressBar { void finishPipeline(); + void endProgress(); + void addJobsToPipeline(int jobs); void finishJobsInPipeline(int jobs); diff --git a/src/include/main/client_config.h b/src/include/main/client_config.h index aca18fab54..7ca23607c0 100644 --- a/src/include/main/client_config.h +++ b/src/include/main/client_config.h @@ -19,7 +19,15 @@ struct ClientConfig { // variable length maximum depth uint32_t varLengthMaxDepth; // If using progress bar - bool progressBar; + bool enableProgressBar; +}; + +struct ClientConfigDefault { + // 0 means timeout is disabled by default. + static constexpr uint64_t TIMEOUT_IN_MS = 0; + static constexpr uint32_t VAR_LENGTH_MAX_DEPTH = 30; + static constexpr bool ENABLE_SEMI_MASK = true; + static constexpr bool ENABLE_PROGRESS_BAR = true; }; } // namespace main diff --git a/src/include/main/client_context.h b/src/include/main/client_context.h index 5eaf80393a..de11e5bd05 100644 --- a/src/include/main/client_context.h +++ b/src/include/main/client_context.h @@ -80,7 +80,6 @@ class ClientContext { KUZU_API transaction::TransactionContext* getTransactionContext() const; // Progress bar - void setProgressBarPrinting(bool progressBarPrinting); common::ProgressBar* getProgressBar() const; // Replace function. diff --git a/src/include/main/settings.h b/src/include/main/settings.h index 37bbca4603..93796cca1a 100644 --- a/src/include/main/settings.h +++ b/src/include/main/settings.h @@ -35,10 +35,11 @@ struct ProgressBarSetting { static constexpr const common::LogicalTypeID inputType = common::LogicalTypeID::BOOL; static void setContext(ClientContext* context, const common::Value& parameter) { KU_ASSERT(parameter.getDataType()->getLogicalTypeID() == common::LogicalTypeID::BOOL); - context->getClientConfigUnsafe()->progressBar = parameter.getValue(); + context->getClientConfigUnsafe()->enableProgressBar = parameter.getValue(); + context->getProgressBar()->toggleProgressBarPrinting(parameter.getValue()); } static common::Value getSetting(ClientContext* context) { - return common::Value(context->getClientConfig()->progressBar); + return common::Value(context->getClientConfig()->enableProgressBar); } }; diff --git a/src/main/client_context.cpp b/src/main/client_context.cpp index 10145a4d4c..fc6c80fe03 100644 --- a/src/main/client_context.cpp +++ b/src/main/client_context.cpp @@ -56,7 +56,7 @@ ClientContext::ClientContext(Database* database) : database{database} { config.numThreads = database->systemConfig.maxNumThreads; config.timeoutInMS = ClientConfigDefault::TIMEOUT_IN_MS; config.varLengthMaxDepth = ClientConfigDefault::VAR_LENGTH_MAX_DEPTH; - config.progressBar = ClientConfigDefault::PROGRESS_BAR; + config.enableProgressBar = ClientConfigDefault::ENABLE_PROGRESS_BAR; } uint64_t ClientContext::getTimeoutRemainingInMS() const { @@ -117,12 +117,6 @@ TransactionContext* ClientContext::getTransactionContext() const { return transactionContext.get(); } -void ClientContext::setProgressBarPrinting(bool progressBarPrinting) { - lock_t lck{mtx}; - config.progressBar = progressBarPrinting; - progressBar->toggleProgressBarPrinting(progressBarPrinting); -} - common::ProgressBar* ClientContext::getProgressBar() const { return progressBar.get(); } diff --git a/src/processor/processor.cpp b/src/processor/processor.cpp index 382aa737be..c255bd03c6 100644 --- a/src/processor/processor.cpp +++ b/src/processor/processor.cpp @@ -27,6 +27,7 @@ std::shared_ptr QueryProcessor::execute( initTask(task.get()); context->clientContext->getProgressBar()->startProgress(); taskScheduler->scheduleTaskAndWaitOrError(task, context); + context->clientContext->getProgressBar()->endProgress(); return resultCollector->getResultFactorizedTable(); } diff --git a/test/test_files/tinysnb/call/call.test b/test/test_files/tinysnb/call/call.test index d9668f8d26..94a4898817 100644 --- a/test/test_files/tinysnb/call/call.test +++ b/test/test_files/tinysnb/call/call.test @@ -42,6 +42,18 @@ Binder exception: Upper bound of rel exceeds maximum: 10. ---- 1 354290 +-LOG SetGetProgressBar +-STATEMENT CALL progress_bar=true +---- ok +-STATEMENT CALL current_setting('progress_bar') RETURN * +---- 1 +True +-STATEMENT CALL progress_bar=false +---- ok +-STATEMENT CALL current_setting('progress_bar') RETURN * +---- 1 +False + -LOG disableSemihMaskOptimization -STATEMENT CALL enable_semi_mask=true ---- ok From f0c76fdb33776c860801c123298bd48f6d7592c2 Mon Sep 17 00:00:00 2001 From: MSebanc Date: Tue, 19 Mar 2024 09:31:48 -0700 Subject: [PATCH 5/9] Removed unused header and implemented feedback --- src/common/task_system/progress_bar.cpp | 3 +++ src/main/client_context.cpp | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/common/task_system/progress_bar.cpp b/src/common/task_system/progress_bar.cpp index dc01b14bbf..da975af85b 100644 --- a/src/common/task_system/progress_bar.cpp +++ b/src/common/task_system/progress_bar.cpp @@ -31,10 +31,13 @@ void ProgressBar::finishPipeline() { return; } numPipelinesFinished++; + // allow progress to print 0% for the next pipeline prevCurPipelineProgress = -0.01; updateProgress(0.0); } +// to mitigate unnecessary progress bar updates, we only update the progress bar when the progress +// changes by >= 1% void ProgressBar::updateProgress(double curPipelineProgress) { if (!trackProgress || curPipelineProgress - prevCurPipelineProgress < 0.01) { return; diff --git a/src/main/client_context.cpp b/src/main/client_context.cpp index fc6c80fe03..e6a3553c6d 100644 --- a/src/main/client_context.cpp +++ b/src/main/client_context.cpp @@ -3,7 +3,6 @@ #include #include "binder/binder.h" -#include "common/constants.h" #include "common/exception/connection.h" #include "common/exception/runtime.h" #include "common/random_engine.h" From 10d4ff28c0d8d25a45472681ba2e0235231fd49e Mon Sep 17 00:00:00 2001 From: MSebanc Date: Tue, 19 Mar 2024 11:55:33 -0700 Subject: [PATCH 6/9] Minor changes I have read and agree to the CLA of the Kuzu repository. --- src/common/task_system/progress_bar.cpp | 19 ++++++++++--------- src/include/common/task_system/progress_bar.h | 5 +++-- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/common/task_system/progress_bar.cpp b/src/common/task_system/progress_bar.cpp index da975af85b..d5319fd441 100644 --- a/src/common/task_system/progress_bar.cpp +++ b/src/common/task_system/progress_bar.cpp @@ -9,14 +9,13 @@ void ProgressBar::startProgress() { } std::lock_guard lock(progressBarLock); printProgressBar(0.0); + printing = true; } void ProgressBar::endProgress() { - if (!trackProgress) { - return; - } std::lock_guard lock(progressBarLock); resetProgressBar(); + printing = false; } void ProgressBar::addPipeline() { @@ -31,21 +30,21 @@ void ProgressBar::finishPipeline() { return; } numPipelinesFinished++; - // allow progress to print 0% for the next pipeline prevCurPipelineProgress = -0.01; updateProgress(0.0); } -// to mitigate unnecessary progress bar updates, we only update the progress bar when the progress -// changes by >= 1% void ProgressBar::updateProgress(double curPipelineProgress) { if (!trackProgress || curPipelineProgress - prevCurPipelineProgress < 0.01) { return; } std::lock_guard lock(progressBarLock); prevCurPipelineProgress = curPipelineProgress; - std::cout << "\033[2A\033[2K\033[1B\033[2K\033[1A"; + if (printing) { + std::cout << "\033[2A"; + } printProgressBar(curPipelineProgress); + printing = true; } void ProgressBar::printProgressBar(double curPipelineProgress) const { @@ -63,8 +62,10 @@ void ProgressBar::printProgressBar(double curPipelineProgress) const { } void ProgressBar::resetProgressBar() { - std::cout << "\033[2A\033[2K\033[1B\033[2K\033[1A"; - std::cout.flush(); + if (printing) { + std::cout << "\033[2A\033[2K\033[1B\033[2K\033[1A"; + std::cout.flush(); + } numPipelines = 0; numPipelinesFinished = 0; prevCurPipelineProgress = 0.0; diff --git a/src/include/common/task_system/progress_bar.h b/src/include/common/task_system/progress_bar.h index a5e43f7579..6ae3aa4c4e 100644 --- a/src/include/common/task_system/progress_bar.h +++ b/src/include/common/task_system/progress_bar.h @@ -14,8 +14,8 @@ class ProgressBar { public: ProgressBar() - : numPipelines{0}, numPipelinesFinished{0}, prevCurPipelineProgress{0.0}, trackProgress{ - false} {}; + : numPipelines{0}, numPipelinesFinished{0}, prevCurPipelineProgress{0.0}, + trackProgress{false}, printing{false} {}; void addPipeline(); @@ -48,6 +48,7 @@ class ProgressBar { double prevCurPipelineProgress; std::mutex progressBarLock; bool trackProgress; + bool printing; }; } // namespace common From 5566332e6e97bc02525344bc41aa86d0655b3bbd Mon Sep 17 00:00:00 2001 From: MSebanc Date: Tue, 19 Mar 2024 11:58:03 -0700 Subject: [PATCH 7/9] Format changes I have read and agree to the CLA of the Kuzu repository. --- src/include/common/task_system/progress_bar.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/common/task_system/progress_bar.h b/src/include/common/task_system/progress_bar.h index 6ae3aa4c4e..895dc15427 100644 --- a/src/include/common/task_system/progress_bar.h +++ b/src/include/common/task_system/progress_bar.h @@ -15,7 +15,7 @@ class ProgressBar { public: ProgressBar() : numPipelines{0}, numPipelinesFinished{0}, prevCurPipelineProgress{0.0}, - trackProgress{false}, printing{false} {}; + trackProgress{false}, printing{false} {}; void addPipeline(); From 3af97cf99dd8d91d708d478d4bd857dae52a5ff8 Mon Sep 17 00:00:00 2001 From: MSebanc Date: Tue, 19 Mar 2024 12:02:50 -0700 Subject: [PATCH 8/9] Format changes I have read and agree to the CLA of the Kuzu repository. --- src/include/common/task_system/progress_bar.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/common/task_system/progress_bar.h b/src/include/common/task_system/progress_bar.h index 895dc15427..50d65eb5fc 100644 --- a/src/include/common/task_system/progress_bar.h +++ b/src/include/common/task_system/progress_bar.h @@ -14,7 +14,7 @@ class ProgressBar { public: ProgressBar() - : numPipelines{0}, numPipelinesFinished{0}, prevCurPipelineProgress{0.0}, + : numPipelines{0}, numPipelinesFinished{0}, prevCurPipelineProgress{0.0}, trackProgress{false}, printing{false} {}; void addPipeline(); From 2c243710fd129937cd4930ad804a424ec64f45c3 Mon Sep 17 00:00:00 2001 From: MSebanc Date: Tue, 19 Mar 2024 12:47:48 -0700 Subject: [PATCH 9/9] Implemented feedback I have read and agree to the CLA of the Kuzu repository. --- src/common/task_system/progress_bar.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/common/task_system/progress_bar.cpp b/src/common/task_system/progress_bar.cpp index d5319fd441..526dde33f4 100644 --- a/src/common/task_system/progress_bar.cpp +++ b/src/common/task_system/progress_bar.cpp @@ -30,11 +30,13 @@ void ProgressBar::finishPipeline() { return; } numPipelinesFinished++; + // This ensures that the progress bar is updated back to 0% after a pipeline is finished. prevCurPipelineProgress = -0.01; updateProgress(0.0); } void ProgressBar::updateProgress(double curPipelineProgress) { + // Only update the progress bar if the progress has changed by at least 1%. if (!trackProgress || curPipelineProgress - prevCurPipelineProgress < 0.01) { return; }