Skip to content

Modernize code by C++17 syntax #1116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions libkineto/src/ActivityProfilerController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@ ActivityProfilerController::ActivityProfilerController(

ActivityProfilerController::~ActivityProfilerController() {
configLoader_.removeHandler(ConfigLoader::ConfigKind::ActivityProfiler, this);
for (int thread_type = 0; thread_type < ThreadType::THREAD_MAX_COUNT;
thread_type++) {
std::thread* profilerThread = profilerThreads_[thread_type];
for (auto profilerThread : profilerThreads_) {
if (profilerThread) {
// signaling termination of the profiler loop
stopRunloop_ = true;
Expand Down
4 changes: 2 additions & 2 deletions libkineto/src/ApproximateClock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ ApproximateClockToUnixTimeConverter::measurePairs() {
}

time_pairs out;
for (int i = 0; i < out.size(); i++) {
out[i] = measurePair();
for (auto & i : out) {
i = measurePair();
}
return out;
}
Expand Down
6 changes: 3 additions & 3 deletions libkineto/src/Config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@

#include "Config.h"

#include <stdlib.h>
#include <cstdlib>

#include <fmt/chrono.h>
#include <fmt/format.h>
#include <fmt/ostream.h>
#include <fmt/ranges.h>
#include <time.h>
#include <ctime>
#include <chrono>
#include <functional>
#include <mutex>
Expand Down Expand Up @@ -615,7 +615,7 @@ void Config::printActivityProfilerConfig(std::ostream& s) const {
std::vector<std::string> activities;
activities.reserve(selectedActivityTypes_.size());
for (const auto& activity : selectedActivityTypes_) {
activities.push_back(toString(activity));
activities.emplace_back(toString(activity));
}
fmt::print(s, " Enabled activities: {}\n", fmt::join(activities, ","));

Expand Down
4 changes: 2 additions & 2 deletions libkineto/src/ConfigLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
#include "ConfigLoader.h"

#ifdef __linux__
#include <signal.h>
#include <csignal>
#endif

#include <stdlib.h>
#include <cstdlib>
#include <chrono>
#include <fstream>
#include <functional>
Expand Down
35 changes: 16 additions & 19 deletions libkineto/src/CuptiActivityProfiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "CuptiActivityProfiler.h"
#include <fmt/format.h>
#include <fmt/ranges.h>
#include <algorithm>
#include <atomic>
#include <cstdint>
#include <functional>
Expand Down Expand Up @@ -303,7 +304,7 @@ void CuptiActivityProfiler::processTraceInternal(ActivityLogger& logger) {
addMetadata(pair.first, pair.second);
}
std::vector<std::string> device_properties;
if (auto props = devicePropertiesJson(); !props.empty()) {
if (const auto& props = devicePropertiesJson(); !props.empty()) {
device_properties.push_back(props);
}
for (const auto& session : sessions_) {
Expand Down Expand Up @@ -413,14 +414,14 @@ CuptiActivityProfiler::CpuGpuSpanPair& CuptiActivityProfiler::recordTraceSpan(
int gpuOpCount) {
TraceSpan gpu_span(gpuOpCount, span.iteration, span.name, "GPU: ");
auto& iterations = traceSpans_[span.name];
iterations.push_back({span, gpu_span});
iterations.emplace_back(span, gpu_span);
return iterations.back();
}

void CuptiActivityProfiler::processCpuTrace(
libkineto::CpuTraceBuffer& cpuTrace,
ActivityLogger& logger) {
if (cpuTrace.activities.size() == 0) {
if (cpuTrace.activities.empty()) {
LOG(WARNING) << "CPU trace is empty!";
return;
}
Expand All @@ -433,9 +434,9 @@ void CuptiActivityProfiler::processCpuTrace(
VLOG(2) << act->correlationId() << ": OP " << act->activityName;
if (derivedConfig_->profileActivityTypes().count(act->type())) {
static_assert(
std::is_same<
std::remove_reference<decltype(act)>::type,
const std::unique_ptr<GenericTraceActivity>>::value,
std::is_same_v<
std::remove_reference_t<decltype(act)>,
const std::unique_ptr<GenericTraceActivity>>,
"handleActivity is unsafe and relies on the caller to maintain not "
"only lifetime but also address stability.");
if (act->duration() < 0) {
Expand Down Expand Up @@ -525,9 +526,7 @@ void CuptiActivityProfiler::GpuUserEventMap::insertOrExtendEvent(
}
int64_t gpu_activity_end =
gpuTraceActivity.timestamp() + gpuTraceActivity.duration();
if (gpu_activity_end > span.endTime) {
span.endTime = gpu_activity_end;
}
span.endTime = std::max(gpu_activity_end, span.endTime);
}

const CuptiActivityProfiler::CpuGpuSpanPair&
Expand Down Expand Up @@ -653,7 +652,7 @@ void CuptiActivityProfiler::handleOverheadActivity(
setGpuActivityPresent(true);
}

std::optional<WaitEventInfo> getWaitEventInfo(uint32_t ctx, uint32_t eventId) {
static std::optional<WaitEventInfo> getWaitEventInfo(uint32_t ctx, uint32_t eventId) {
auto key = CtxEventPair{ctx, eventId};
auto it = waitEventMap().find(key);
if (it != waitEventMap().end()) {
Expand Down Expand Up @@ -772,9 +771,7 @@ inline void CuptiActivityProfiler::updateGpuNetSpan(
if (gpuOp.timestamp() < gpu_span.startTime || gpu_span.startTime == 0) {
gpu_span.startTime = gpuOp.timestamp();
}
if ((gpuOp.timestamp() + gpuOp.duration()) > gpu_span.endTime) {
gpu_span.endTime = gpuOp.timestamp() + gpuOp.duration();
}
gpu_span.endTime = std::max(gpuOp.timestamp() + gpuOp.duration(), gpu_span.endTime);
}

// I've observed occasional broken timestamps attached to GPU events...
Expand Down Expand Up @@ -865,7 +862,7 @@ inline void CuptiActivityProfiler::handleGpuActivity(
}

template <class T>
inline void updateCtxToDeviceId(const T* act) {
static inline void updateCtxToDeviceId(const T* act) {
if (ctxToDeviceId().count(act->contextId) == 0) {
ctxToDeviceId()[act->contextId] = act->deviceId;
}
Expand Down Expand Up @@ -1127,7 +1124,7 @@ void CuptiActivityProfiler::configure(
}
#endif // HAS_CUPTI || HAS_ROCTRACER

if (profilers_.size() > 0) {
if (!profilers_.empty()) {
configureChildProfilers();
}
rangeProfilingActive_ = config_->selectedActivityTypes().count(
Expand Down Expand Up @@ -1270,7 +1267,7 @@ void CuptiActivityProfiler::stopTraceInternal(
VLOG(0) << "CollectTrace -> ProcessTrace";
} else {
LOG(WARNING) << "Called stopTrace with state == "
<< static_cast<std::underlying_type<RunloopState>::type>(
<< static_cast<std::underlying_type_t<RunloopState>>(
currentRunloopState_.load());
}
for (auto& session : sessions_) {
Expand All @@ -1285,7 +1282,7 @@ void CuptiActivityProfiler::resetInternal() {
currentRunloopState_ = RunloopState::WaitForRequest;
}

const time_point<system_clock> CuptiActivityProfiler::performRunLoopStep(
time_point<system_clock> CuptiActivityProfiler::performRunLoopStep(
const time_point<system_clock>& now,
const time_point<system_clock>& nextWakeupTime,
int64_t currentIter) {
Expand Down Expand Up @@ -1436,7 +1433,7 @@ const time_point<system_clock> CuptiActivityProfiler::performRunLoopStep(
return new_wakeup_time;
}

const void CuptiActivityProfiler::performMemoryLoop(
void CuptiActivityProfiler::performMemoryLoop(
const string& path,
uint32_t profile_time,
ActivityLogger* logger,
Expand Down Expand Up @@ -1521,7 +1518,7 @@ void CuptiActivityProfiler::finalizeTrace(

#ifdef HAS_CUPTI
// Overhead info
overheadInfo_.push_back(ActivityLogger::OverheadInfo("CUPTI Overhead"));
overheadInfo_.emplace_back("CUPTI Overhead");
for (const auto& info : overheadInfo_) {
logger.handleOverheadInfo(info, captureWindowStartTime_);
}
Expand Down
5 changes: 2 additions & 3 deletions libkineto/src/CuptiActivityProfiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include <map>
#include <memory>
#include <mutex>
#include <queue>
#include <string>
#include <thread>
#include <unordered_map>
Expand Down Expand Up @@ -138,12 +137,12 @@ class CuptiActivityProfiler {
// depending on required warm-up time and delayed start time.
// When active, it's a good idea to invoke more frequently to stay below
// memory usage limit (ACTIVITIES_MAX_GPU_BUFFER_SIZE_MB) during warmup.
const std::chrono::time_point<std::chrono::system_clock> performRunLoopStep(
std::chrono::time_point<std::chrono::system_clock> performRunLoopStep(
const std::chrono::time_point<std::chrono::system_clock>& now,
const std::chrono::time_point<std::chrono::system_clock>& nextWakeupTime,
int64_t currentIter = -1);

const void performMemoryLoop(
void performMemoryLoop(
const std::string& path,
uint32_t profile_time,
ActivityLogger* logger,
Expand Down
2 changes: 1 addition & 1 deletion libkineto/src/CuptiCallbackApi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#include "CuptiCallbackApi.h"
#include "CuptiActivityApi.h"

#include <assert.h>
#include <cassert>
#include <algorithm>
#include <chrono>
#include <mutex>
Expand Down
6 changes: 3 additions & 3 deletions libkineto/src/CuptiRangeProfiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,9 @@ void CuptiRangeProfilerSession::processTrace(ActivityLogger& logger) {

for (const auto& event : traceBuffer_.activities) {
static_assert(
std::is_same<
std::remove_reference<decltype(event)>::type,
const std::unique_ptr<GenericTraceActivity>>::value,
std::is_same_v<
std::remove_reference_t<decltype(event)>,
const std::unique_ptr<GenericTraceActivity>>,
"handleActivity is unsafe and relies on the caller to maintain not "
"only lifetime but also address stability.");
logger.handleActivity(*event);
Expand Down
6 changes: 3 additions & 3 deletions libkineto/src/CuptiRangeProfilerApi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
* LICENSE file in the root directory of this source tree.
*/

#include <stdio.h>
#include <stdlib.h>
#include <cstdio>
#include <cstdlib>
#include "ILoggerObserver.h"
#ifdef HAS_CUPTI
#include <cupti.h>
Expand Down Expand Up @@ -742,7 +742,7 @@ CuptiRBProfilerSession::CuptiRBProfilerSession(
LOG(WARNING) << "CUPTI Range Profiler is not available on this CUDA version:"
<< STRINGIFY(CUDA_VERSION);
};
CuptiRBProfilerSession::~CuptiRBProfilerSession() {}
CuptiRBProfilerSession::~CuptiRBProfilerSession() = default;
void CuptiRBProfilerSession::stop() {}
void CuptiRBProfilerSession::enable() {}
void CuptiRBProfilerSession::disable() {}
Expand Down
7 changes: 3 additions & 4 deletions libkineto/src/CuptiRangeProfilerConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include <CuptiRangeProfilerConfig.h>
#include <Logger.h>

#include <stdlib.h>
#include <cstdlib>

#include <fmt/format.h>
#include <fmt/ostream.h>
Expand All @@ -29,9 +29,8 @@ constexpr char kCuptiProfilerPerKernelKey[] =
constexpr char kCuptiProfilerMaxRangesKey[] = "CUPTI_PROFILER_MAX_RANGES";

CuptiRangeProfilerConfig::CuptiRangeProfilerConfig(Config& cfg)
: parent_(&cfg),
cuptiProfilerPerKernel_(false),
cuptiProfilerMaxRanges_(0) {}
: parent_(&cfg)
{}

bool CuptiRangeProfilerConfig::handleOption(
const std::string& name,
Expand Down
2 changes: 1 addition & 1 deletion libkineto/src/Demangle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ _Pragma("GCC diagnostic pop");
#endif
#endif // _MSC_VER

#include <string.h>
#include <cstring>
#include <string>

namespace KINETO_NAMESPACE {
Expand Down
5 changes: 2 additions & 3 deletions libkineto/src/EventProfiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#include <fmt/format.h>
#include <fmt/ostream.h>
#include <fmt/ranges.h>
#include <time.h>
#include <ctime>
#include <algorithm>
#include <cmath>
#include <iomanip>
Expand Down Expand Up @@ -311,8 +311,7 @@ bool EventProfiler::initEventGroups() {
eventGroupSets_ = cuptiEvents_->createGroupSets(ids);
VLOG(0) << "Number of group sets: " << eventGroupSets_->numSets;
for (int i = 0; i < eventGroupSets_->numSets; i++) {
sets_.push_back(
EventGroupSet(eventGroupSets_->sets[i], events_, *cuptiEvents_));
sets_.emplace_back(eventGroupSets_->sets[i], events_, *cuptiEvents_);
}
return !sets_.empty();
}
Expand Down
2 changes: 1 addition & 1 deletion libkineto/src/IpcFabricConfigClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

#include "IpcFabricConfigClient.h"

#include <stdlib.h>
#include <cstdlib>
#include <random>
#include <sstream>

Expand Down
2 changes: 1 addition & 1 deletion libkineto/src/Logger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

#ifndef USE_GOOGLE_LOG

#include <time.h>
#include <ctime>
#include <chrono>
#include <cstring>
#include <iomanip>
Expand Down
2 changes: 1 addition & 1 deletion libkineto/src/ThreadUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ std::vector<std::pair<int32_t, std::string>> pidCommandPairsOfAncestors() {
for (int i = 0; i <= kMaxParentPids && (i == 0 || curr_pid > 1); i++) {
std::pair<int32_t, std::string> ppid_and_comm =
parentPidAndCommand(curr_pid);
pairs.push_back(std::make_pair(curr_pid, ppid_and_comm.second));
pairs.emplace_back(curr_pid, ppid_and_comm.second);
curr_pid = ppid_and_comm.first;
}
return pairs;
Expand Down
2 changes: 1 addition & 1 deletion libkineto/src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ void libkineto_init(bool cpuOnly, bool logOnError) {
const char* logLevelEnv = getenv("KINETO_LOG_LEVEL");
if (logLevelEnv) {
// atoi returns 0 on error, so that's what we want - default to VERBOSE
static_assert(static_cast<int>(VERBOSE) == 0, "");
static_assert(static_cast<int>(VERBOSE) == 0 );
SET_LOG_SEVERITY_LEVEL(atoi(logLevelEnv));
}

Expand Down
8 changes: 4 additions & 4 deletions libkineto/src/output_json.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

#include <fmt/format.h>
#include <fmt/ostream.h>
#include <time.h>
#include <ctime>
#include <fstream>
#include <iterator>
#include "Config.h"
Expand Down Expand Up @@ -450,7 +450,7 @@ void ChromeTraceLogger::handleActivity(const libkineto::ITraceActivity& op) {
arg_values.append(",");
}
arg_values.append(fmt::format(
" \"{}\": {}, \"{}\": {}, \"{}\": {}, \"{}\": {}, \"{}\": {}",
R"( "{}": {}, "{}": {}, "{}": {}, "{}": {}, "{}": {})",
kCollectiveName,
collectiveName,
kInMsgNelems,
Expand Down Expand Up @@ -488,7 +488,7 @@ void ChromeTraceLogger::handleActivity(const libkineto::ITraceActivity& op) {
arg_values.append(",");
}
arg_values.append(fmt::format(
" \"{}\": {}, \"{}\": {}",
R"( "{}": {}, "{}": {})",
kInSplit,
inSplitSize,
kOutSplit,
Expand Down Expand Up @@ -623,7 +623,7 @@ void ChromeTraceLogger::handleLink(
// Flow events much bind to specific slices in order to exist.
// Only Flow end needs to specify a binding point to enclosing slice.
// Flow start automatically sets binding point to enclosing slice.
const auto binding = (type == kFlowEnd) ? ", \"bp\": \"e\"" : "";
const auto binding = (type == kFlowEnd) ? R"(, "bp": "e")" : "";
// clang-format off
uint64_t ts = transToRelativeTime(e.timestamp());
fmt::print(traceOf_, R"JSON(
Expand Down