Skip to content
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

#2150: Types: Make Time a strong type and use it across the codebase #2151

Merged
merged 4 commits into from
Oct 11, 2023
Merged
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
32 changes: 16 additions & 16 deletions src/vt/elm/elm_lb_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@
namespace vt { namespace elm {

void ElementLBData::start(TimeType time) {
TimeTypeWrapper const start_time = time;
cur_time_ = start_time.seconds();
auto const start_time = time;
cur_time_ = start_time;
cur_time_started_ = true;

vt_debug_print(
Expand All @@ -63,13 +63,13 @@ void ElementLBData::start(TimeType time) {
}

void ElementLBData::stop(TimeType time) {
TimeTypeWrapper const stop_time = time;
TimeTypeWrapper const total_time = stop_time.seconds() - cur_time_;
auto const stop_time = time;
auto const total_time = stop_time - cur_time_;
//vtAssert(cur_time_started_, "Must have started time");
auto const started = cur_time_started_;
if (started) {
cur_time_started_ = false;
addTime(total_time);
addTime(total_time.seconds());
}

vt_debug_print(
Expand Down Expand Up @@ -124,17 +124,17 @@ void ElementLBData::recvToNode(
recvComm(key, bytes);
}

void ElementLBData::addTime(TimeTypeWrapper const& time) {
phase_timings_[cur_phase_] += time.seconds();
void ElementLBData::addTime(LoadType const timeLoad) {
phase_timings_[cur_phase_] += timeLoad;

subphase_timings_[cur_phase_].resize(cur_subphase_ + 1);
subphase_timings_[cur_phase_].at(cur_subphase_) += time.seconds();
subphase_timings_[cur_phase_].at(cur_subphase_) += timeLoad;

vt_debug_print(
verbose,lb,
"ElementLBData: addTime: time={}, cur_load={}\n",
time,
TimeTypeWrapper(phase_timings_[cur_phase_])
phase_timings_[cur_phase_]
);
}

Expand Down Expand Up @@ -163,43 +163,43 @@ PhaseType ElementLBData::getPhase() const {
return cur_phase_;
}

TimeType ElementLBData::getLoad(PhaseType const& phase) const {
LoadType ElementLBData::getLoad(PhaseType const& phase) const {
auto iter = phase_timings_.find(phase);
if (iter != phase_timings_.end()) {
TimeTypeWrapper const total_load = phase_timings_.at(phase);
auto const total_load = phase_timings_.at(phase);

vt_debug_print(
verbose, lb,
"ElementLBData: getLoad: load={}, phase={}, size={}\n",
total_load, phase, phase_timings_.size()
);

return total_load.seconds();
return total_load;
} else {
return 0.0;
}
}

TimeType
LoadType
ElementLBData::getLoad(PhaseType phase, SubphaseType subphase) const {
if (subphase == no_subphase)
return getLoad(phase);

auto const& subphase_loads = subphase_timings_.at(phase);

vtAssert(subphase_loads.size() > subphase, "Must have subphase");
TimeTypeWrapper const total_load = subphase_loads.at(subphase);
auto const total_load = subphase_loads.at(subphase);

vt_debug_print(
verbose, lb,
"ElementLBData: getLoad: load={}, phase={}, subphase={}\n",
total_load, phase, subphase
);

return total_load.seconds();
return total_load;
}

std::vector<TimeType> const& ElementLBData::getSubphaseTimes(PhaseType phase) {
std::vector<LoadType> const& ElementLBData::getSubphaseTimes(PhaseType phase) {
return subphase_timings_[phase];
}

Expand Down
14 changes: 7 additions & 7 deletions src/vt/elm/elm_lb_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ struct ElementLBData {

void start(TimeType time);
void stop(TimeType time);
void addTime(TimeTypeWrapper const& time);
void addTime(LoadType const timeLoad);

void sendToEntity(ElementIDStruct to, ElementIDStruct from, double bytes);
void sendComm(elm::CommKey key, double bytes);
Expand All @@ -84,12 +84,12 @@ struct ElementLBData {
void updatePhase(PhaseType const& inc = 1);
void resetPhase();
PhaseType getPhase() const;
TimeType getLoad(PhaseType const& phase) const;
TimeType getLoad(PhaseType phase, SubphaseType subphase) const;
LoadType getLoad(PhaseType const& phase) const;
LoadType getLoad(PhaseType phase, SubphaseType subphase) const;

CommMapType const& getComm(PhaseType const& phase);
std::vector<CommMapType> const& getSubphaseComm(PhaseType phase);
std::vector<TimeType> const& getSubphaseTimes(PhaseType phase);
std::vector<LoadType> const& getSubphaseTimes(PhaseType phase);
void setSubPhase(SubphaseType subphase);
SubphaseType getSubPhase() const;

Expand Down Expand Up @@ -124,13 +124,13 @@ struct ElementLBData {

protected:
bool cur_time_started_ = false;
TimeType cur_time_ = 0.0;
TimeType cur_time_ = TimeType{0.0};
PhaseType cur_phase_ = fst_lb_phase;
std::unordered_map<PhaseType, TimeType> phase_timings_ = {};
std::unordered_map<PhaseType, LoadType> phase_timings_ = {};
std::unordered_map<PhaseType, CommMapType> phase_comm_ = {};

SubphaseType cur_subphase_ = 0;
std::unordered_map<PhaseType, std::vector<TimeType>> subphase_timings_ = {};
std::unordered_map<PhaseType, std::vector<LoadType>> subphase_timings_ = {};
std::unordered_map<PhaseType, std::vector<CommMapType>> subphase_comm_ = {};
};

Expand Down
2 changes: 1 addition & 1 deletion src/vt/event/event.cc
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ AsyncEvent::EventStateType AsyncEvent::testEventComplete(EventType const& event)
void AsyncEvent::testEventsTrigger(int const& num_events) {
# if vt_check_enabled(trace_enabled)
int32_t num_completed = 0;
TimeType tr_begin = 0.0;
auto tr_begin = TimeType{0.0};

if (theConfig()->vt_trace_event_polling) {
tr_begin = timing::getCurrentTime();
Expand Down
2 changes: 1 addition & 1 deletion src/vt/event/event_record.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ struct EventRecord {

# if vt_check_enabled(diagnostics)
/// the time this event record was created
TimeType creation_time_stamp_ = 0.;
TimeType creation_time_stamp_ = TimeType{0.};
# endif
};

Expand Down
8 changes: 4 additions & 4 deletions src/vt/messaging/active.cc
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ EventType ActiveMessenger::sendMsgMPI(
{
VT_ALLOW_MPI_CALLS;
#if vt_check_enabled(trace_enabled)
double tr_begin = 0;
auto tr_begin = TimeType{0.};
if (theConfig()->vt_trace_mpi) {
tr_begin = vt::timing::getCurrentTime();
}
Expand Down Expand Up @@ -579,7 +579,7 @@ std::tuple<EventType, int> ActiveMessenger::sendDataMPI(
);
{
#if vt_check_enabled(trace_enabled)
double tr_begin = 0;
auto tr_begin = TimeType{0.};
if (theConfig()->vt_trace_mpi) {
tr_begin = vt::timing::getCurrentTime();
}
Expand Down Expand Up @@ -769,7 +769,7 @@ void ActiveMessenger::recvDataDirect(
);

#if vt_check_enabled(trace_enabled)
double tr_begin = 0;
auto tr_begin = TimeType{0.};
if (theConfig()->vt_trace_mpi) {
tr_begin = vt::timing::getCurrentTime();
}
Expand Down Expand Up @@ -1006,7 +1006,7 @@ bool ActiveMessenger::tryProcessIncomingActiveMsg() {

{
#if vt_check_enabled(trace_enabled)
double tr_begin = 0;
auto tr_begin = TimeType{0.};
if (theConfig()->vt_trace_mpi) {
tr_begin = vt::timing::getCurrentTime();
}
Expand Down
2 changes: 1 addition & 1 deletion src/vt/messaging/request_holder.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ struct RequestHolder {
bool testAll(Callable c, int& num_mpi_tests) {
# if vt_check_enabled(trace_enabled)
std::size_t const holder_size_start = holder_.size();
TimeType tr_begin = 0.0;
auto tr_begin = TimeType{0.0};
if (theConfig()->vt_trace_irecv_polling) {
tr_begin = vt::timing::getCurrentTime();
}
Expand Down
12 changes: 6 additions & 6 deletions src/vt/phase/phase_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -296,17 +296,17 @@ void PhaseManager::printSummary(vrt::collection::lb::PhaseInfo* last_phase_info)
auto lb_name = vrt::collection::balance::get_lb_names()[
last_phase_info->lb_type
];
TimeTypeWrapper const total_time = timing::getCurrentTime() - start_time_;
auto const total_time = timing::getCurrentTime() - start_time_;
vt_print(
phase,
"phase={}, duration={}, rank_max_compute_time={}, rank_avg_compute_time={}, imbalance={:.3f}, "
"grain_max_time={}, migration count={}, lb_name={}\n",
cur_phase_,
total_time,
TimeTypeWrapper(last_phase_info->max_load),
TimeTypeWrapper(last_phase_info->avg_load),
TimeType(last_phase_info->max_load),
TimeType(last_phase_info->avg_load),
last_phase_info->imb_load,
TimeTypeWrapper(last_phase_info->max_obj),
TimeType(last_phase_info->max_obj),
last_phase_info->migration_count,
lb_name
);
Expand All @@ -315,8 +315,8 @@ void PhaseManager::printSummary(vrt::collection::lb::PhaseInfo* last_phase_info)
// "POST phase={}, total time={}, max_load={}, avg_load={}, imbalance={:.3f}, migration count={}\n",
// cur_phase_,
// total_time,
// TimeTypeWrapper(last_phase_info->max_load_post_lb),
// TimeTypeWrapper(last_phase_info->avg_load_post_lb),
// TimeType(last_phase_info->max_load_post_lb),
// TimeType(last_phase_info->avg_load_post_lb),
// last_phase_info->imb_load_post_lb,
// last_phase_info->migration_count
// );
Expand Down
4 changes: 2 additions & 2 deletions src/vt/runnable/runnable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ void RunnableNew::run() {
{
needs_time = contexts_.lb.needsTime();
}
TimeType start_time = needs_time ? theSched()->getRecentTime() : NAN;
TimeType start_time = needs_time ? theSched()->getRecentTime() : TimeType{NAN};

#if vt_check_enabled(fcontext)
if (suspended_) {
Expand Down Expand Up @@ -175,7 +175,7 @@ void RunnableNew::run() {
#endif
}
theSched()->setRecentTimeToStale();
TimeType end_time = needs_time ? theSched()->getRecentTime() : NAN;
TimeType end_time = needs_time ? theSched()->getRecentTime() : TimeType{NAN};



Expand Down
4 changes: 2 additions & 2 deletions src/vt/runtime/component/diagnostic.impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ meter::Timer<T> Diagnostic::registerTimerT(
) {
auto sum = registerDiagnostic<T>(
key + " [sum]", desc, DiagnosticUpdate::Sum, unit,
DiagnosticTypeEnum::PerformanceDiagnostic, 0
DiagnosticTypeEnum::PerformanceDiagnostic, T{0}
);
auto min = registerDiagnostic<T>(
key + " [min]", desc, DiagnosticUpdate::Min, unit,
Expand All @@ -105,7 +105,7 @@ meter::Timer<T> Diagnostic::registerTimerT(
);
auto avg = registerDiagnostic<T>(
key + " [avg]", desc, DiagnosticUpdate::Avg, unit,
DiagnosticTypeEnum::PerformanceDiagnostic, 0
DiagnosticTypeEnum::PerformanceDiagnostic, T{0}
);
return meter::Timer<T>{sum, avg, max, min};
}
Expand Down
3 changes: 2 additions & 1 deletion src/vt/runtime/component/diagnostic_erased_value.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@

#include "vt/runtime/component/diagnostic_types.h"
#include "vt/utils/adt/histogram_approx.h"
#include "vt/timing/timing_type.h"
#include "vt/utils/strong/strong_type.h"

#include <string>
Expand All @@ -62,7 +63,7 @@ namespace vt { namespace runtime { namespace component {
struct DiagnosticErasedValue {
/// These are the set of valid diagnostic value types after being erased from
/// \c DiagnosticValue<T> get turned into this union for saving the value.
using UnionValueType = std::variant<double, int64_t>;
using UnionValueType = std::variant<double, int64_t, TimeType >;

// The trio (min, max, sum) save the actual type with the value to print it
// correctly
Expand Down
4 changes: 3 additions & 1 deletion src/vt/runtime/component/diagnostic_value.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include "vt/collective/reduce/operators/default_msg.h"
#include "vt/collective/reduce/reduce.h"
#include "vt/pipe/pipe_manager.h"
#include "vt/timing/timing_type.h"

#include <limits>

Expand Down Expand Up @@ -80,7 +81,7 @@ void reduceHelper(
if (update == DiagnosticUpdate::Min) {
out->is_valid_value_ = reduced_val.min() != std::numeric_limits<T>::max();
} else {
out->is_valid_value_ = reduced_val.sum() != 0;
out->is_valid_value_ = reduced_val.sum() != T{0};
}
}
);
Expand Down Expand Up @@ -110,5 +111,6 @@ void reduceHelper(

DIAGNOSTIC_VALUE_INSTANCE(int64_t)
DIAGNOSTIC_VALUE_INSTANCE(double)
DIAGNOSTIC_VALUE_INSTANCE(TimeType)

}}}} /* end namespace vt::runtime::component::detail */
12 changes: 6 additions & 6 deletions src/vt/runtime/component/diagnostic_value.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,15 @@ struct DiagnosticValueWrapper {
N_(in_N),
min_(updated ? in_value : std::numeric_limits<T>::max()),
max_(updated ? in_value : std::numeric_limits<T>::lowest()),
sum_(updated ? in_value : 0),
avg_(updated ? in_value : 0),
sum_(updated ? in_value : T{0}),
avg_(updated ? static_cast<double>(in_value) : 0),
M2_(0.),
M3_(0.),
M4_(0.),
hist_(16)
{
// add to histogram when starting the reduction
hist_.add(value_);
hist_.add(static_cast<double>(value_));
}

/**
Expand Down Expand Up @@ -168,7 +168,7 @@ struct DiagnosticValueWrapper {
*
* \return the max value
*/
T max() const { return N_ == 0 ? 0 : max_; }
T max() const { return N_ == 0 ? T{0} : max_; }

/**
* \internal \brief Get sum of values (use after reduction)
Expand All @@ -182,7 +182,7 @@ struct DiagnosticValueWrapper {
*
* \return the min value
*/
T min() const { return N_ == 0 ? 0 : min_; }
T min() const { return N_ == 0 ? T{0} : min_; }

/**
* \internal \brief Get the arithmetic mean value (use after reduction)
Expand Down Expand Up @@ -280,7 +280,7 @@ struct DiagnosticValueWrapper {
private:
T value_; /**< The raw value */
std::size_t N_ = 0; /**< The cardinality */
T min_ = {}, max_ = {}, sum_ = {}; /**< The min/max/sum for reduction */
T min_ = T{}, max_ = T{}, sum_ = T{}; /**< The min/max/sum for reduction */
/**
* The avg and 2/3/4 moments for reduction
*/
Expand Down
6 changes: 3 additions & 3 deletions src/vt/runtime/component/meter/timer.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ struct Timer : DiagnosticStatsPack<T> {
* \brief Stop the timer and record the interval
*/
void stop() {
if (start_time_ != 0) {
if (start_time_ != TimeType{0.}) {
update(start_time_, timing::getCurrentTime());
start_time_ = 0;
start_time_ = TimeType{0.};
}
}

Expand All @@ -116,7 +116,7 @@ struct Timer : DiagnosticStatsPack<T> {
}

private:
T start_time_ = 0;
T start_time_ = T{0};
};

}}}} /* end namespace vt::runtime::component::meter */
Expand Down
2 changes: 1 addition & 1 deletion src/vt/scheduler/scheduler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ bool Scheduler::shouldCallProgress(
processed_since_last_progress >= theConfig()->vt_sched_progress_han;
bool enough_time_passed =
progress_time_enabled_ and
time_since_last_progress > theConfig()->vt_sched_progress_sec;
time_since_last_progress > TimeType{theConfig()->vt_sched_progress_sec};

return
(not progress_time_enabled_ and not k_handler_enabled) or
Expand Down
Loading
Loading