Skip to content

Commit

Permalink
#2150: Types: Make Time a strong type and use it across the codebase
Browse files Browse the repository at this point in the history
  • Loading branch information
JacobDomagala committed Sep 26, 2023
1 parent 9b8df4f commit 5c07be5
Show file tree
Hide file tree
Showing 50 changed files with 354 additions and 216 deletions.
26 changes: 13 additions & 13 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,8 +63,8 @@ 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) {
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(TimeType const& time) {
phase_timings_[cur_phase_] += time;

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

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 @@ -166,17 +166,17 @@ PhaseType ElementLBData::getPhase() const {
TimeType 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;
return TimeType{0.0};
}
}

Expand All @@ -188,15 +188,15 @@ ElementLBData::getLoad(PhaseType phase, SubphaseType subphase) const {
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) {
Expand Down
4 changes: 2 additions & 2 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(TimeType const& time);

void sendToEntity(ElementIDStruct to, ElementIDStruct from, double bytes);
void sendComm(elm::CommKey key, double bytes);
Expand Down Expand Up @@ -124,7 +124,7 @@ 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, CommMapType> phase_comm_ = {};
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
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
4 changes: 2 additions & 2 deletions src/vt/scheduler/scheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ struct Scheduler : runtime::component::Component<Scheduler> {
*
* \param[in] current_time current time
*/
void runProgress(bool msg_only = false, TimeType current_time = 0.0 );
void runProgress(bool msg_only = false, TimeType current_time = TimeType{0.0} );

/**
* \brief Runs the scheduler until a condition is met.
Expand Down Expand Up @@ -438,7 +438,7 @@ struct Scheduler : runtime::component::Component<Scheduler> {
EventTriggerContType event_triggers;
EventTriggerContType event_triggers_once;

TimeType last_progress_time_ = 0.0;
TimeType last_progress_time_ = TimeType{0.0};
bool progress_time_enabled_ = false;
int32_t processed_after_last_progress_ = 0;

Expand Down
5 changes: 3 additions & 2 deletions src/vt/timetrigger/trigger.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ struct Trigger {
* \return the next time this should be triggered
*/
TimeType nextTriggerTime() const {
return (last_trigger_time_*1000.0 + period_.count())/1000.0;
return TimeType{
(last_trigger_time_.milliseconds() + period_.count()) / 1000.0};
}

/**
Expand Down Expand Up @@ -141,7 +142,7 @@ struct Trigger {
private:
std::chrono::milliseconds period_; /**< The trigger's period */
ActionType trigger_ = nullptr; /**< The action to trigger */
TimeType last_trigger_time_ = 0.; /**< The last time it was triggered */
TimeType last_trigger_time_ = TimeType{0.}; /**< The last time it was triggered */
int id_ = -1; /**< The trigger's id */
};

Expand Down
2 changes: 1 addition & 1 deletion src/vt/timing/timing.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
namespace vt { namespace timing {

TimeType getCurrentTime() {
return MPI_Wtime();
return TimeType{MPI_Wtime()};
}

}} /* end namespace vt::timing */
Loading

0 comments on commit 5c07be5

Please sign in to comment.