Tidy code (#1120)

Summary:
Apply some clang-tidy rules of performance and readability.

Pull Request resolved: #1120

Reviewed By: davidberard98

Differential Revision: D80110562

Pulled By: sraikund16

fbshipit-source-id: aaff444b3202690dca571d2b688d9e4a3d760d99

Author: cyyever

libkineto/src/AbstractConfig.cpp

@@ -44,7 +44,7 @@ static inline string trim(string& s) {
 // Return the index of char d in string s.
 // If not found, returns the length of the string.
 static int find(const char* s, char delim) {
-  int i;
+  int i = 0;
   for (i = 0; s[i]; i++) {
     if (s[i] == delim) {
       break;
@@ -97,7 +97,7 @@ vector<string> AbstractConfig::splitAndTrim(const string& s, char delim) const {
 
 int64_t AbstractConfig::toIntRange(const string& val, int64_t min, int64_t max)
     const {
-  char* invalid;
+  char* invalid = nullptr;
   int64_t res = strtoll(val.c_str(), &invalid, 10);
   if (val.empty() || *invalid) {
     throw std::invalid_argument(fmt::format("Invalid integer: {}", val));

libkineto/src/ActivityLoggerFactory.h

@@ -14,6 +14,7 @@
 #include <functional>
 #include <map>
 #include <string>
+#include <utility>
 
 namespace KINETO_NAMESPACE {
 
@@ -26,7 +27,7 @@ class ActivityLoggerFactory {
 
   // Add logger factory for a protocol prefix
   void addProtocol(const std::string& protocol, FactoryFunc f) {
-    factories_[tolower(protocol)] = f;
+    factories_[tolower(protocol)] = std::move(f);
   }
 
   // Create a logger, invoking the factory for the protocol specified in url
@@ -49,11 +50,11 @@ class ActivityLoggerFactory {
     return s;
   }
 
-  static std::string extractProtocol(std::string url) {
+  static std::string extractProtocol(const std::string& url) {
     return url.substr(0, url.find("://"));
   }
 
-  static std::string stripProtocol(std::string url) {
+  static std::string stripProtocol(const std::string& url) {
     size_t pos = url.find("://");
     return pos == url.npos ? url : url.substr(pos + 3);
   }

libkineto/src/ActivityProfilerController.cpp

@@ -11,6 +11,7 @@
 #include <chrono>
 #include <functional>
 #include <thread>
+#include <utility>
 
 #include "ActivityLoggerFactory.h"
 #include "ActivityTrace.h"
@@ -37,7 +38,7 @@ static std::shared_ptr<LoggerCollector>& loggerCollectorFactory() {
 }
 
 void ActivityProfilerController::setLoggerCollectorFactory(
-    std::function<std::shared_ptr<LoggerCollector>()> factory) {
+    const std::function<std::shared_ptr<LoggerCollector>()>& factory) {
   loggerCollectorFactory() = factory();
 }
 
@@ -110,7 +111,7 @@ static ActivityLoggerFactory& loggerFactory() {
 void ActivityProfilerController::addLoggerFactory(
     const std::string& protocol,
     ActivityLoggerFactory::FactoryFunc factory) {
-  loggerFactory().addProtocol(protocol, factory);
+  loggerFactory().addProtocol(protocol, std::move(factory));
 }
 
 static std::unique_ptr<ActivityLogger> makeLogger(const Config& config) {
@@ -253,28 +254,21 @@ void ActivityProfilerController::profilerLoop() {
 }
 
 void ActivityProfilerController::memoryProfilerLoop() {
-  std::string path = asyncRequestConfig_->activitiesLogFile();
-  auto profile_time = asyncRequestConfig_->profileMemoryDuration();
-  std::unique_ptr<Config> config = asyncRequestConfig_->clone();
   while (!stopRunloop_) {
     // Perform Double-checked locking to reduce overhead of taking lock.
     if (asyncRequestConfig_ && !profiler_->isActive()) {
       std::lock_guard<std::mutex> lock(asyncConfigLock_);
       if (asyncRequestConfig_ && !profiler_->isActive() &&
           asyncRequestConfig_->memoryProfilerEnabled()) {
         logger_ = makeLogger(*asyncRequestConfig_);
-        path = asyncRequestConfig_->activitiesLogFile();
-        profile_time = asyncRequestConfig_->profileMemoryDuration();
-        config = asyncRequestConfig_->clone();
+        auto path = asyncRequestConfig_->activitiesLogFile();
+        auto profile_time = asyncRequestConfig_->profileMemoryDuration();
+        auto config = asyncRequestConfig_->clone();
         asyncRequestConfig_ = nullptr;
-      } else {
-        continue;
+        profiler_->performMemoryLoop(
+            path, profile_time, logger_.get(), *config);
       }
-    } else {
-      continue;
     }
-
-    profiler_->performMemoryLoop(path, profile_time, logger_.get(), *config);
   }
 }
 

libkineto/src/ActivityProfilerController.h

@@ -47,7 +47,7 @@ class ActivityProfilerController : public ConfigLoader::ConfigHandler {
 #if !USE_GOOGLE_LOG
   static std::shared_ptr<LoggerCollector> getLoggerCollector();
   static void setLoggerCollectorFactory(
-      std::function<std::shared_ptr<LoggerCollector>()> factory);
+      const std::function<std::shared_ptr<LoggerCollector>()>& factory);
 #endif // !USE_GOOGLE_LOG
 
   static void addLoggerFactory(

libkineto/src/Config.cpp

@@ -20,6 +20,7 @@
 #include <functional>
 #include <mutex>
 #include <ostream>
+#include <utility>
 
 #include "Logger.h"
 #include "ThreadUtil.h"
@@ -182,7 +183,7 @@ struct FactoryMap {
       std::string name,
       std::function<AbstractConfig*(Config&)> factory) {
     std::lock_guard<std::mutex> lock(lock_);
-    factories_[name] = factory;
+    factories_.emplace(std::move(name), std::move(factory));
   }
 
   void addFeatureConfigs(Config& cfg) {
@@ -214,7 +215,7 @@ void Config::addConfigFactory(
     std::function<AbstractConfig*(Config&)> factory) {
   auto factories = configFactories();
   if (factories) {
-    factories->addFactory(name, factory);
+    factories->addFactory(std::move(name), std::move(factory));
   }
 }
 
@@ -346,9 +347,9 @@ static time_point<system_clock> handleProfileStartTime(int64_t start_time_ms) {
 void Config::setActivityTypes(
     const std::vector<std::string>& selected_activities) {
   selectedActivityTypes_.clear();
-  if (selected_activities.size() > 0) {
+  if (!selected_activities.empty()) {
     for (const auto& activity : selected_activities) {
-      if (activity == "") {
+      if (activity.empty()) {
         continue;
       }
       selectedActivityTypes_.insert(toActivityType(activity));
@@ -568,7 +569,7 @@ void Config::validate(
     profileStartIteration_ = 0;
   }
 
-  if (selectedActivityTypes_.size() == 0) {
+  if (selectedActivityTypes_.empty()) {
     selectDefaultActivityTypes();
   }
   setActivityDependentConfig();

libkineto/src/ConfigLoader.cpp

@@ -17,6 +17,7 @@
 #include <fstream>
 #include <functional>
 #include <memory>
+#include <utility>
 
 #include "DaemonConfigLoader.h"
 
@@ -117,7 +118,7 @@ daemonConfigLoaderFactory() {
 
 void ConfigLoader::setDaemonConfigLoaderFactory(
     std::function<std::unique_ptr<IDaemonConfigLoader>()> factory) {
-  daemonConfigLoaderFactory() = factory;
+  daemonConfigLoaderFactory() = std::move(factory);
 }
 
 ConfigLoader& ConfigLoader::instance() {

libkineto/src/DaemonConfigLoader.cpp

@@ -71,7 +71,7 @@ void DaemonConfigLoader::setCommunicationFabric(bool enabled) {
     // This is probably a temporary problem - return -1 to indicate error.
     return;
   }
-  return configClient->setIpcFabricEnabled(enabled);
+  configClient->setIpcFabricEnabled(enabled);
 }
 
 void DaemonConfigLoader::registerFactory() {

libkineto/src/Demangle.cpp

@@ -37,7 +37,7 @@ std::string demangle(const char* name) {
     return name;
   }
 
-  int status;
+  int status = 0;
   size_t len = 0;
   char* demangled = abi::__cxa_demangle(name, nullptr, &len, &status);
   if (status != 0) {

libkineto/src/DeviceProperties.cpp

@@ -128,7 +128,7 @@ int smCount(uint32_t deviceId) {
 }
 #else
 const std::string& devicePropertiesJson() {
-  static std::string devicePropsJson = "";
+  static std::string devicePropsJson;
   return devicePropsJson;
 }
 

libkineto/src/IpcFabricConfigClient.cpp

@@ -28,7 +28,7 @@ std::string generate_uuid_v4() {
   static std::uniform_int_distribution<> dis2(8, 11);
 
   std::stringstream ss;
-  int i;
+  int i = 0;
   ss << std::hex;
   for (i = 0; i < 8; i++) {
     ss << dis(gen);