refs #14599 - moved ShowTime logic out of timer-related code

Makefile

@@ -695,7 +695,7 @@ $(libcppdir)/vfvalue.o: lib/vfvalue.cpp lib/config.h lib/errortypes.h lib/mathli
 frontend/frontend.o: frontend/frontend.cpp frontend/frontend.h lib/addoninfo.h lib/checkers.h lib/config.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/regex.h lib/settings.h lib/standards.h lib/utils.h
 	$(CXX) ${INCLUDE_FOR_FE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ frontend/frontend.cpp
 
-cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/filelister.h externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/regex.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h lib/xml.h
+cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/filelister.h externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/regex.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h lib/xml.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cmdlineparser.cpp
 
 cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/executor.h cli/processexecutor.h cli/sehwrapper.h cli/signalhandler.h cli/singleexecutor.h cli/threadexecutor.h externals/picojson/picojson.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkers.h lib/checkersreport.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/json.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/regex.h lib/sarifreport.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
@@ -776,7 +776,7 @@ test/testclangimport.o: test/testclangimport.cpp lib/addoninfo.h lib/check.h lib
 test/testclass.o: test/testclass.cpp lib/addoninfo.h lib/check.h lib/checkclass.h lib/checkers.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/regex.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h
 	$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testclass.cpp
 
-test/testcmdlineparser.o: test/testcmdlineparser.cpp cli/cmdlinelogger.h cli/cmdlineparser.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/regex.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h test/redirect.h
+test/testcmdlineparser.o: test/testcmdlineparser.cpp cli/cmdlinelogger.h cli/cmdlineparser.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/regex.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h test/redirect.h
 	$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testcmdlineparser.cpp
 
 test/testcolor.o: test/testcolor.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/regex.h lib/settings.h lib/standards.h lib/utils.h test/fixture.h

cli/cmdlineparser.cpp

@@ -36,7 +36,6 @@
 #include "settings.h"
 #include "standards.h"
 #include "suppressions.h"
-#include "timer.h"
 #include "utils.h"
 
 #include "frontend.h"
@@ -1430,17 +1429,17 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
         else if (std::strncmp(argv[i], "--showtime=", 11) == 0) {
             const std::string showtimeMode = argv[i] + 11;
             if (showtimeMode == "file")
-                mSettings.showtime = ShowTime::FILE;
+                mSettings.showtime = Settings::ShowTime::FILE;
             else if (showtimeMode == "file-total")
-                mSettings.showtime = ShowTime::FILE_TOTAL;
+                mSettings.showtime = Settings::ShowTime::FILE_TOTAL;
             else if (showtimeMode == "summary")
-                mSettings.showtime = ShowTime::SUMMARY;
+                mSettings.showtime = Settings::ShowTime::SUMMARY;
             else if (showtimeMode == "top5_file")
-                mSettings.showtime = ShowTime::TOP5_FILE;
+                mSettings.showtime = Settings::ShowTime::TOP5_FILE;
             else if (showtimeMode == "top5_summary")
-                mSettings.showtime = ShowTime::TOP5_SUMMARY;
+                mSettings.showtime = Settings::ShowTime::TOP5_SUMMARY;
             else if (showtimeMode == "none")
-                mSettings.showtime = ShowTime::NONE;
+                mSettings.showtime = Settings::ShowTime::NONE;
             else if (showtimeMode.empty()) {
                 mLogger.printError("no mode provided for --showtime");
                 return Result::Fail;

cli/cppcheckexecutor.cpp

@@ -56,6 +56,7 @@
 #include <iostream>
 #include <list>
 #include <map>
+#include <memory>
 #include <set>
 #include <sstream>
 #include <unordered_set>
@@ -268,8 +269,8 @@ int CppCheckExecutor::check(int argc, const char* const argv[])
     }
 
     std::unique_ptr<OneShotTimer> overallTimer;
-    if (settings.showtime == ShowTime::SUMMARY || settings.showtime == ShowTime::TOP5_SUMMARY)
-        overallTimer.reset(new OneShotTimer("Overall time", settings.showtime));
+    if (settings.showtime == Settings::ShowTime::SUMMARY || settings.showtime == Settings::ShowTime::TOP5_SUMMARY)
+        overallTimer.reset(new OneShotTimer("Overall time"));
 
     settings.loadSummaries();
 
@@ -420,7 +421,9 @@ bool CppCheckExecutor::reportUnmatchedSuppressions(const Settings &settings, con
 int CppCheckExecutor::check_internal(const Settings& settings, Suppressions& supprs) const
 {
     StdLogger stdLogger(settings);
-    TimerResults timerResults;
+    std::unique_ptr<TimerResults> timerResults;
+    if (settings.showtime != Settings::ShowTime::NONE)
+        timerResults.reset(new TimerResults);
 
     if (settings.reportProgress >= 0)
         stdLogger.resetLatestProgressOutputTime();
@@ -441,31 +444,35 @@ int CppCheckExecutor::check_internal(const Settings& settings, Suppressions& sup
     if (!settings.checkersReportFilename.empty())
         std::remove(settings.checkersReportFilename.c_str());
 
-    CppCheck cppcheck(settings, supprs, stdLogger, &timerResults, true, executeCommand);
+    CppCheck cppcheck(settings, supprs, stdLogger, timerResults.get(), true, executeCommand);
 
     unsigned int returnValue = 0;
     if (settings.useSingleJob()) {
         // Single process
-        SingleExecutor executor(cppcheck, mFiles, mFileSettings, settings, supprs, stdLogger, &timerResults);
+        SingleExecutor executor(cppcheck, mFiles, mFileSettings, settings, supprs, stdLogger, timerResults.get());
         returnValue = executor.check();
     } else {
 #if defined(HAS_THREADING_MODEL_THREAD)
         if (settings.executor == Settings::ExecutorType::Thread) {
-            ThreadExecutor executor(mFiles, mFileSettings, settings, supprs, stdLogger, &timerResults, CppCheckExecutor::executeCommand);
+            ThreadExecutor executor(mFiles, mFileSettings, settings, supprs, stdLogger, timerResults.get(), CppCheckExecutor::executeCommand);
             returnValue = executor.check();
         }
 #endif
 #if defined(HAS_THREADING_MODEL_FORK)
         if (settings.executor == Settings::ExecutorType::Process) {
-            ProcessExecutor executor(mFiles, mFileSettings, settings, supprs, stdLogger, &timerResults, CppCheckExecutor::executeCommand);
+            ProcessExecutor executor(mFiles, mFileSettings, settings, supprs, stdLogger, timerResults.get(), CppCheckExecutor::executeCommand);
             returnValue = executor.check();
         }
 #endif
     }
 
     // TODO: show time *after* the whole program analysis
-    if (settings.showtime == ShowTime::SUMMARY || settings.showtime == ShowTime::TOP5_SUMMARY)
-        timerResults.showResults(settings.showtime);
+    if (timerResults) {
+        if (settings.showtime == Settings::ShowTime::SUMMARY)
+            timerResults->showResults();
+        else if (settings.showtime == Settings::ShowTime::TOP5_SUMMARY)
+            timerResults->showResults(5);
+    }
 
     // TODO: is this run again instead of using previously cached results?
     returnValue |= cppcheck.analyseWholeProgram(settings.buildDir, mFiles, mFileSettings, stdLogger.getCtuInfo());

lib/cppcheck.cpp

@@ -924,8 +924,8 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
         return mLogger->exitcode();
 
     std::unique_ptr<OneShotTimer> checkTimeTimer;
-    if (mSettings.showtime == ShowTime::FILE || mSettings.showtime == ShowTime::FILE_TOTAL || mSettings.showtime == ShowTime::TOP5_FILE)
-        checkTimeTimer.reset(new OneShotTimer("Check time: " + file.spath(), mSettings.showtime));
+    if (mSettings.showtime == Settings::ShowTime::FILE || mSettings.showtime == Settings::ShowTime::FILE_TOTAL || mSettings.showtime == Settings::ShowTime::TOP5_FILE)
+        checkTimeTimer.reset(new OneShotTimer("Check time: " + file.spath()));
 
     if (!mSettings.quiet) {
         std::string fixedpath = Path::toNativeSeparators(file.spath());
@@ -1045,7 +1045,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
         // Get configurations..
         std::set<std::string> configurations;
         if (maxConfigs > 1) {
-            Timer::run("Preprocessor::getConfigs", mSettings.showtime, mTimerResults, [&]() {
+            Timer::run("Preprocessor::getConfigs", mTimerResults, [&]() {
                 configurations = preprocessor.getConfigs();
             });
         } else {
@@ -1130,7 +1130,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
 
             if (mSettings.preprocessOnly) {
                 std::string codeWithoutCfg;
-                Timer::run("Preprocessor::getcode", mSettings.showtime, mTimerResults, [&]() {
+                Timer::run("Preprocessor::getcode", mTimerResults, [&]() {
                     codeWithoutCfg = preprocessor.getcode(currentConfig, files, true);
                 });
 
@@ -1154,7 +1154,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
             {
                 bool skipCfg = false;
                 // Create tokens, skip rest of iteration if failed
-                Timer::run("Tokenizer::createTokens", mSettings.showtime, mTimerResults, [&]() {
+                Timer::run("Tokenizer::createTokens", mTimerResults, [&]() {
                     simplecpp::OutputList outputList_cfg;
                     simplecpp::TokenList tokensP = preprocessor.preprocess(currentConfig, files, outputList_cfg);
                     const simplecpp::Output* o = preprocessor.handleErrors(outputList_cfg);
@@ -1180,7 +1180,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
 
             Tokenizer tokenizer(std::move(tokenlist), mErrorLogger);
             try {
-                if (mSettings.showtime != ShowTime::NONE)
+                if (mSettings.showtime != Settings::ShowTime::NONE)
                     tokenizer.setTimerResults(mTimerResults);
                 tokenizer.setDirectives(directives); // TODO: how to avoid repeated copies?
 
@@ -1294,8 +1294,12 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
     // TODO: clear earlier?
     mLogger->clear();
 
-    if (mTimerResults && (mSettings.showtime == ShowTime::FILE || mSettings.showtime == ShowTime::TOP5_FILE))
-        mTimerResults->showResults(mSettings.showtime);
+    if (mTimerResults) {
+        if (mSettings.showtime == Settings::ShowTime::FILE)
+            mTimerResults->showResults();
+        else if (mSettings.showtime == Settings::ShowTime::TOP5_FILE)
+            mTimerResults->showResults(5);
+    }
 
     return mLogger->exitcode();
 }
@@ -1354,7 +1358,7 @@ void CppCheck::checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation
                 return;
             }
 
-            Timer::run(check->name() + "::runChecks", mSettings.showtime, mTimerResults, [&]() {
+            Timer::run(check->name() + "::runChecks", mTimerResults, [&]() {
                 check->runChecks(tokenizer, &mErrorLogger);
             });
         }
@@ -1489,7 +1493,7 @@ void CppCheck::executeAddons(const std::string& dumpFile, const FileWithDetails&
 {
     if (!dumpFile.empty()) {
         std::vector<std::string> f{dumpFile};
-        Timer::run("CppCheck::executeAddons", mSettings.showtime, mTimerResults, [&]() {
+        Timer::run("CppCheck::executeAddons", mTimerResults, [&]() {
             executeAddons(f, file.spath());
         });
     }

lib/settings.h

@@ -53,7 +53,6 @@ class Regex;
 #endif
 
 struct Suppressions;
-enum class ShowTime : std::uint8_t;
 namespace ValueFlow {
     class Value;
 }
@@ -442,6 +441,15 @@ class CPPCHECKLIB WARN_UNUSED Settings {
     SimpleEnableGroup<Certainty> certainty;
     SimpleEnableGroup<Checks> checks;
 
+    enum class ShowTime : std::uint8_t {
+        NONE,
+        FILE,
+        FILE_TOTAL,
+        SUMMARY,
+        TOP5_SUMMARY,
+        TOP5_FILE
+    };
+
     /** @brief show timing information (--showtime=file|summary|top5) */
     ShowTime showtime{};
 

lib/timer.cpp

@@ -19,8 +19,6 @@
 #include "timer.h"
 
 #include <algorithm>
-#include <cassert>
-#include <cstddef>
 #include <iostream>
 #include <utility>
 #include <vector>
@@ -38,12 +36,9 @@ namespace {
 
 // TODO: this does not include any file context when SHOWTIME_FILE thus rendering it useless - should we include the logging with the progress logging?
 // that could also get rid of the broader locking
-void TimerResults::showResults(ShowTime mode, bool metrics) const
+void TimerResults::showResults(size_t max_results, bool metrics) const
 {
-    if (mode == ShowTime::NONE)
-        return;
     std::vector<dataElementType> data;
-
     {
         std::lock_guard<std::mutex> l(mResultsSync);
 
@@ -57,7 +52,7 @@ void TimerResults::showResults(ShowTime mode, bool metrics) const
 
     size_t ordinal = 1; // maybe it would be nice to have an ordinal in output later!
     for (auto iter=data.cbegin(); iter!=data.cend(); ++iter) {
-        if ((mode != ShowTime::TOP5_FILE && mode != ShowTime::TOP5_SUMMARY) || (ordinal<=5)) {
+        if (ordinal <= max_results) {
             const double sec = iter->second.getSeconds().count();
             std::cout << iter->first << ": " << sec << "s";
             if (metrics) {
@@ -85,12 +80,14 @@ void TimerResults::reset()
     mResults.clear();
 }
 
-Timer::Timer(std::string str, ShowTime showtimeMode, TimerResultsIntf* timerResults)
+Timer::Timer(std::string str, TimerResultsIntf* timerResults)
     : mName(std::move(str))
-    , mMode(showtimeMode)
-    , mStart(Clock::now())
     , mResults(timerResults)
-{}
+{
+    if (!mResults)
+        return;
+    mStart = Clock::now();
+}
 
 Timer::~Timer()
 {
@@ -99,18 +96,13 @@ Timer::~Timer()
 
 void Timer::stop()
 {
-    if (mMode == ShowTime::NONE)
+    if (mStart == TimePoint{})
         return;
-    if (mStart != TimePoint{}) {
-        if (!mResults) {
-            assert(false);
-        }
-        else {
-            auto diff = std::chrono::duration_cast<std::chrono::milliseconds>(Clock::now() - mStart);
-            mResults->addResults(mName, diff);
-        }
-    }
-    mMode = ShowTime::NONE; // prevent multiple stops
+
+    const auto diff = std::chrono::duration_cast<std::chrono::milliseconds>(Clock::now() - mStart);
+    mResults->addResults(mName, diff);
+
+    mStart = TimePoint{}; // prevent multiple stops
 }
 
 static std::string durationToString(std::chrono::milliseconds duration)
@@ -138,11 +130,8 @@ static std::string durationToString(std::chrono::milliseconds duration)
     return (ellapsedTime + secondsStr + "s");
 }
 
-OneShotTimer::OneShotTimer(std::string name, ShowTime showtime)
+OneShotTimer::OneShotTimer(std::string name)
 {
-    if (showtime == ShowTime::NONE)
-        return;
-
     class MyResults : public TimerResultsIntf
     {
     private:
@@ -156,5 +145,5 @@ OneShotTimer::OneShotTimer(std::string name, ShowTime showtime)
     };
 
     mResults.reset(new MyResults);
-    mTimer.reset(new Timer(std::move(name), showtime, mResults.get()));
+    mTimer.reset(new Timer(std::move(name), mResults.get()));
 }

lib/timer.h

@@ -23,7 +23,8 @@
 #include "config.h"
 
 #include <chrono>
-#include <cstdint>
+#include <cstddef>
+#include <limits>
 #include <map>
 #include <memory>
 #include <mutex>
@@ -32,15 +33,6 @@
 #include <utility>
 #include <vector>
 
-enum class ShowTime : std::uint8_t {
-    NONE,
-    FILE,
-    FILE_TOTAL,
-    SUMMARY,
-    TOP5_SUMMARY,
-    TOP5_FILE
-};
-
 class CPPCHECKLIB TimerResultsIntf {
 public:
     virtual ~TimerResultsIntf() = default;
@@ -66,7 +58,7 @@ class CPPCHECKLIB WARN_UNUSED TimerResults : public TimerResultsIntf {
 public:
     TimerResults() = default;
 
-    void showResults(ShowTime mode, bool metrics = true) const;
+    void showResults(size_t max_results = std::numeric_limits<size_t>::max(), bool metrics = true) const;
     void addResults(const std::string& name, std::chrono::milliseconds duration) override;
 
     void reset();
@@ -81,7 +73,7 @@ class CPPCHECKLIB Timer {
     using Clock = std::chrono::high_resolution_clock;
     using TimePoint = std::chrono::time_point<Clock>;
 
-    Timer(std::string str, ShowTime showtimeMode, TimerResultsIntf* timerResults = nullptr);
+    Timer(std::string str, TimerResultsIntf* timerResults = nullptr);
     ~Timer();
 
     Timer(const Timer&) = delete;
@@ -90,22 +82,21 @@ class CPPCHECKLIB Timer {
     void stop();
 
     template<class TFunc>
-    static void run(std::string str, ShowTime showtimeMode, TimerResultsIntf* timerResults, const TFunc& f) {
-        Timer t(std::move(str), showtimeMode, timerResults);
+    static void run(std::string str, TimerResultsIntf* timerResults, const TFunc& f) {
+        Timer t(std::move(str), timerResults);
         f();
     }
 
 private:
     const std::string mName;
-    ShowTime mMode{};
     TimePoint mStart;
     TimerResultsIntf* mResults{};
 };
 
 class CPPCHECKLIB OneShotTimer
 {
 public:
-    OneShotTimer(std::string name, ShowTime showtime);
+    OneShotTimer(std::string name);
 private:
     std::unique_ptr<TimerResultsIntf> mResults;
     std::unique_ptr<Timer> mTimer;