Skip to content

Commit 96f533a

Browse files
authored
Merge pull request #2151 from DARMA-tasking/2150-use-strong-time-type
#2150: Types: Make `Time` a strong type and use it across the codebase
2 parents 2ff5cc2 + edd172e commit 96f533a

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+315
-176
lines changed

src/vt/elm/elm_lb_data.cc

+16-16
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@
5151
namespace vt { namespace elm {
5252

5353
void ElementLBData::start(TimeType time) {
54-
TimeTypeWrapper const start_time = time;
55-
cur_time_ = start_time.seconds();
54+
auto const start_time = time;
55+
cur_time_ = start_time;
5656
cur_time_started_ = true;
5757

5858
vt_debug_print(
@@ -63,13 +63,13 @@ void ElementLBData::start(TimeType time) {
6363
}
6464

6565
void ElementLBData::stop(TimeType time) {
66-
TimeTypeWrapper const stop_time = time;
67-
TimeTypeWrapper const total_time = stop_time.seconds() - cur_time_;
66+
auto const stop_time = time;
67+
auto const total_time = stop_time - cur_time_;
6868
//vtAssert(cur_time_started_, "Must have started time");
6969
auto const started = cur_time_started_;
7070
if (started) {
7171
cur_time_started_ = false;
72-
addTime(total_time);
72+
addTime(total_time.seconds());
7373
}
7474

7575
vt_debug_print(
@@ -124,17 +124,17 @@ void ElementLBData::recvToNode(
124124
recvComm(key, bytes);
125125
}
126126

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

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

133133
vt_debug_print(
134134
verbose,lb,
135135
"ElementLBData: addTime: time={}, cur_load={}\n",
136136
time,
137-
TimeTypeWrapper(phase_timings_[cur_phase_])
137+
phase_timings_[cur_phase_]
138138
);
139139
}
140140

@@ -163,43 +163,43 @@ PhaseType ElementLBData::getPhase() const {
163163
return cur_phase_;
164164
}
165165

166-
TimeType ElementLBData::getLoad(PhaseType const& phase) const {
166+
LoadType ElementLBData::getLoad(PhaseType const& phase) const {
167167
auto iter = phase_timings_.find(phase);
168168
if (iter != phase_timings_.end()) {
169-
TimeTypeWrapper const total_load = phase_timings_.at(phase);
169+
auto const total_load = phase_timings_.at(phase);
170170

171171
vt_debug_print(
172172
verbose, lb,
173173
"ElementLBData: getLoad: load={}, phase={}, size={}\n",
174174
total_load, phase, phase_timings_.size()
175175
);
176176

177-
return total_load.seconds();
177+
return total_load;
178178
} else {
179179
return 0.0;
180180
}
181181
}
182182

183-
TimeType
183+
LoadType
184184
ElementLBData::getLoad(PhaseType phase, SubphaseType subphase) const {
185185
if (subphase == no_subphase)
186186
return getLoad(phase);
187187

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

190190
vtAssert(subphase_loads.size() > subphase, "Must have subphase");
191-
TimeTypeWrapper const total_load = subphase_loads.at(subphase);
191+
auto const total_load = subphase_loads.at(subphase);
192192

193193
vt_debug_print(
194194
verbose, lb,
195195
"ElementLBData: getLoad: load={}, phase={}, subphase={}\n",
196196
total_load, phase, subphase
197197
);
198198

199-
return total_load.seconds();
199+
return total_load;
200200
}
201201

202-
std::vector<TimeType> const& ElementLBData::getSubphaseTimes(PhaseType phase) {
202+
std::vector<LoadType> const& ElementLBData::getSubphaseTimes(PhaseType phase) {
203203
return subphase_timings_[phase];
204204
}
205205

src/vt/elm/elm_lb_data.h

+7-7
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ struct ElementLBData {
6363

6464
void start(TimeType time);
6565
void stop(TimeType time);
66-
void addTime(TimeTypeWrapper const& time);
66+
void addTime(LoadType const timeLoad);
6767

6868
void sendToEntity(ElementIDStruct to, ElementIDStruct from, double bytes);
6969
void sendComm(elm::CommKey key, double bytes);
@@ -84,12 +84,12 @@ struct ElementLBData {
8484
void updatePhase(PhaseType const& inc = 1);
8585
void resetPhase();
8686
PhaseType getPhase() const;
87-
TimeType getLoad(PhaseType const& phase) const;
88-
TimeType getLoad(PhaseType phase, SubphaseType subphase) const;
87+
LoadType getLoad(PhaseType const& phase) const;
88+
LoadType getLoad(PhaseType phase, SubphaseType subphase) const;
8989

9090
CommMapType const& getComm(PhaseType const& phase);
9191
std::vector<CommMapType> const& getSubphaseComm(PhaseType phase);
92-
std::vector<TimeType> const& getSubphaseTimes(PhaseType phase);
92+
std::vector<LoadType> const& getSubphaseTimes(PhaseType phase);
9393
void setSubPhase(SubphaseType subphase);
9494
SubphaseType getSubPhase() const;
9595

@@ -124,13 +124,13 @@ struct ElementLBData {
124124

125125
protected:
126126
bool cur_time_started_ = false;
127-
TimeType cur_time_ = 0.0;
127+
TimeType cur_time_ = TimeType{0.0};
128128
PhaseType cur_phase_ = fst_lb_phase;
129-
std::unordered_map<PhaseType, TimeType> phase_timings_ = {};
129+
std::unordered_map<PhaseType, LoadType> phase_timings_ = {};
130130
std::unordered_map<PhaseType, CommMapType> phase_comm_ = {};
131131

132132
SubphaseType cur_subphase_ = 0;
133-
std::unordered_map<PhaseType, std::vector<TimeType>> subphase_timings_ = {};
133+
std::unordered_map<PhaseType, std::vector<LoadType>> subphase_timings_ = {};
134134
std::unordered_map<PhaseType, std::vector<CommMapType>> subphase_comm_ = {};
135135
};
136136

src/vt/event/event.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ AsyncEvent::EventStateType AsyncEvent::testEventComplete(EventType const& event)
304304
void AsyncEvent::testEventsTrigger(int const& num_events) {
305305
# if vt_check_enabled(trace_enabled)
306306
int32_t num_completed = 0;
307-
TimeType tr_begin = 0.0;
307+
auto tr_begin = TimeType{0.0};
308308

309309
if (theConfig()->vt_trace_event_polling) {
310310
tr_begin = timing::getCurrentTime();

src/vt/event/event_record.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ struct EventRecord {
138138

139139
# if vt_check_enabled(diagnostics)
140140
/// the time this event record was created
141-
TimeType creation_time_stamp_ = 0.;
141+
TimeType creation_time_stamp_ = TimeType{0.};
142142
# endif
143143
};
144144

src/vt/messaging/active.cc

+4-4
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ EventType ActiveMessenger::sendMsgMPI(
347347
{
348348
VT_ALLOW_MPI_CALLS;
349349
#if vt_check_enabled(trace_enabled)
350-
double tr_begin = 0;
350+
auto tr_begin = TimeType{0.};
351351
if (theConfig()->vt_trace_mpi) {
352352
tr_begin = vt::timing::getCurrentTime();
353353
}
@@ -579,7 +579,7 @@ std::tuple<EventType, int> ActiveMessenger::sendDataMPI(
579579
);
580580
{
581581
#if vt_check_enabled(trace_enabled)
582-
double tr_begin = 0;
582+
auto tr_begin = TimeType{0.};
583583
if (theConfig()->vt_trace_mpi) {
584584
tr_begin = vt::timing::getCurrentTime();
585585
}
@@ -769,7 +769,7 @@ void ActiveMessenger::recvDataDirect(
769769
);
770770

771771
#if vt_check_enabled(trace_enabled)
772-
double tr_begin = 0;
772+
auto tr_begin = TimeType{0.};
773773
if (theConfig()->vt_trace_mpi) {
774774
tr_begin = vt::timing::getCurrentTime();
775775
}
@@ -1006,7 +1006,7 @@ bool ActiveMessenger::tryProcessIncomingActiveMsg() {
10061006

10071007
{
10081008
#if vt_check_enabled(trace_enabled)
1009-
double tr_begin = 0;
1009+
auto tr_begin = TimeType{0.};
10101010
if (theConfig()->vt_trace_mpi) {
10111011
tr_begin = vt::timing::getCurrentTime();
10121012
}

src/vt/messaging/request_holder.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ struct RequestHolder {
9898
bool testAll(Callable c, int& num_mpi_tests) {
9999
# if vt_check_enabled(trace_enabled)
100100
std::size_t const holder_size_start = holder_.size();
101-
TimeType tr_begin = 0.0;
101+
auto tr_begin = TimeType{0.0};
102102
if (theConfig()->vt_trace_irecv_polling) {
103103
tr_begin = vt::timing::getCurrentTime();
104104
}

src/vt/phase/phase_manager.cc

+6-6
Original file line numberDiff line numberDiff line change
@@ -296,17 +296,17 @@ void PhaseManager::printSummary(vrt::collection::lb::PhaseInfo* last_phase_info)
296296
auto lb_name = vrt::collection::balance::get_lb_names()[
297297
last_phase_info->lb_type
298298
];
299-
TimeTypeWrapper const total_time = timing::getCurrentTime() - start_time_;
299+
auto const total_time = timing::getCurrentTime() - start_time_;
300300
vt_print(
301301
phase,
302302
"phase={}, duration={}, rank_max_compute_time={}, rank_avg_compute_time={}, imbalance={:.3f}, "
303303
"grain_max_time={}, migration count={}, lb_name={}\n",
304304
cur_phase_,
305305
total_time,
306-
TimeTypeWrapper(last_phase_info->max_load),
307-
TimeTypeWrapper(last_phase_info->avg_load),
306+
TimeType(last_phase_info->max_load),
307+
TimeType(last_phase_info->avg_load),
308308
last_phase_info->imb_load,
309-
TimeTypeWrapper(last_phase_info->max_obj),
309+
TimeType(last_phase_info->max_obj),
310310
last_phase_info->migration_count,
311311
lb_name
312312
);
@@ -315,8 +315,8 @@ void PhaseManager::printSummary(vrt::collection::lb::PhaseInfo* last_phase_info)
315315
// "POST phase={}, total time={}, max_load={}, avg_load={}, imbalance={:.3f}, migration count={}\n",
316316
// cur_phase_,
317317
// total_time,
318-
// TimeTypeWrapper(last_phase_info->max_load_post_lb),
319-
// TimeTypeWrapper(last_phase_info->avg_load_post_lb),
318+
// TimeType(last_phase_info->max_load_post_lb),
319+
// TimeType(last_phase_info->avg_load_post_lb),
320320
// last_phase_info->imb_load_post_lb,
321321
// last_phase_info->migration_count
322322
// );

src/vt/runnable/runnable.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ void RunnableNew::run() {
124124
{
125125
needs_time = contexts_.lb.needsTime();
126126
}
127-
TimeType start_time = needs_time ? theSched()->getRecentTime() : NAN;
127+
TimeType start_time = needs_time ? theSched()->getRecentTime() : TimeType{NAN};
128128

129129
#if vt_check_enabled(fcontext)
130130
if (suspended_) {
@@ -175,7 +175,7 @@ void RunnableNew::run() {
175175
#endif
176176
}
177177
theSched()->setRecentTimeToStale();
178-
TimeType end_time = needs_time ? theSched()->getRecentTime() : NAN;
178+
TimeType end_time = needs_time ? theSched()->getRecentTime() : TimeType{NAN};
179179

180180

181181

src/vt/runtime/component/diagnostic.impl.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ meter::Timer<T> Diagnostic::registerTimerT(
9393
) {
9494
auto sum = registerDiagnostic<T>(
9595
key + " [sum]", desc, DiagnosticUpdate::Sum, unit,
96-
DiagnosticTypeEnum::PerformanceDiagnostic, 0
96+
DiagnosticTypeEnum::PerformanceDiagnostic, T{0}
9797
);
9898
auto min = registerDiagnostic<T>(
9999
key + " [min]", desc, DiagnosticUpdate::Min, unit,
@@ -105,7 +105,7 @@ meter::Timer<T> Diagnostic::registerTimerT(
105105
);
106106
auto avg = registerDiagnostic<T>(
107107
key + " [avg]", desc, DiagnosticUpdate::Avg, unit,
108-
DiagnosticTypeEnum::PerformanceDiagnostic, 0
108+
DiagnosticTypeEnum::PerformanceDiagnostic, T{0}
109109
);
110110
return meter::Timer<T>{sum, avg, max, min};
111111
}

src/vt/runtime/component/diagnostic_erased_value.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646

4747
#include "vt/runtime/component/diagnostic_types.h"
4848
#include "vt/utils/adt/histogram_approx.h"
49+
#include "vt/timing/timing_type.h"
4950
#include "vt/utils/strong/strong_type.h"
5051

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

6768
// The trio (min, max, sum) save the actual type with the value to print it
6869
// correctly

src/vt/runtime/component/diagnostic_value.cc

+3-1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
#include "vt/collective/reduce/operators/default_msg.h"
4848
#include "vt/collective/reduce/reduce.h"
4949
#include "vt/pipe/pipe_manager.h"
50+
#include "vt/timing/timing_type.h"
5051

5152
#include <limits>
5253

@@ -80,7 +81,7 @@ void reduceHelper(
8081
if (update == DiagnosticUpdate::Min) {
8182
out->is_valid_value_ = reduced_val.min() != std::numeric_limits<T>::max();
8283
} else {
83-
out->is_valid_value_ = reduced_val.sum() != 0;
84+
out->is_valid_value_ = reduced_val.sum() != T{0};
8485
}
8586
}
8687
);
@@ -110,5 +111,6 @@ void reduceHelper(
110111

111112
DIAGNOSTIC_VALUE_INSTANCE(int64_t)
112113
DIAGNOSTIC_VALUE_INSTANCE(double)
114+
DIAGNOSTIC_VALUE_INSTANCE(TimeType)
113115

114116
}}}} /* end namespace vt::runtime::component::detail */

src/vt/runtime/component/diagnostic_value.h

+6-6
Original file line numberDiff line numberDiff line change
@@ -93,15 +93,15 @@ struct DiagnosticValueWrapper {
9393
N_(in_N),
9494
min_(updated ? in_value : std::numeric_limits<T>::max()),
9595
max_(updated ? in_value : std::numeric_limits<T>::lowest()),
96-
sum_(updated ? in_value : 0),
97-
avg_(updated ? in_value : 0),
96+
sum_(updated ? in_value : T{0}),
97+
avg_(updated ? static_cast<double>(in_value) : 0),
9898
M2_(0.),
9999
M3_(0.),
100100
M4_(0.),
101101
hist_(16)
102102
{
103103
// add to histogram when starting the reduction
104-
hist_.add(value_);
104+
hist_.add(static_cast<double>(value_));
105105
}
106106

107107
/**
@@ -168,7 +168,7 @@ struct DiagnosticValueWrapper {
168168
*
169169
* \return the max value
170170
*/
171-
T max() const { return N_ == 0 ? 0 : max_; }
171+
T max() const { return N_ == 0 ? T{0} : max_; }
172172

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

187187
/**
188188
* \internal \brief Get the arithmetic mean value (use after reduction)
@@ -280,7 +280,7 @@ struct DiagnosticValueWrapper {
280280
private:
281281
T value_; /**< The raw value */
282282
std::size_t N_ = 0; /**< The cardinality */
283-
T min_ = {}, max_ = {}, sum_ = {}; /**< The min/max/sum for reduction */
283+
T min_ = T{}, max_ = T{}, sum_ = T{}; /**< The min/max/sum for reduction */
284284
/**
285285
* The avg and 2/3/4 moments for reduction
286286
*/

src/vt/runtime/component/meter/timer.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,9 @@ struct Timer : DiagnosticStatsPack<T> {
102102
* \brief Stop the timer and record the interval
103103
*/
104104
void stop() {
105-
if (start_time_ != 0) {
105+
if (start_time_ != TimeType{0.}) {
106106
update(start_time_, timing::getCurrentTime());
107-
start_time_ = 0;
107+
start_time_ = TimeType{0.};
108108
}
109109
}
110110

@@ -116,7 +116,7 @@ struct Timer : DiagnosticStatsPack<T> {
116116
}
117117

118118
private:
119-
T start_time_ = 0;
119+
T start_time_ = T{0};
120120
};
121121

122122
}}}} /* end namespace vt::runtime::component::meter */

src/vt/scheduler/scheduler.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ bool Scheduler::shouldCallProgress(
179179
processed_since_last_progress >= theConfig()->vt_sched_progress_han;
180180
bool enough_time_passed =
181181
progress_time_enabled_ and
182-
time_since_last_progress > theConfig()->vt_sched_progress_sec;
182+
time_since_last_progress > TimeType{theConfig()->vt_sched_progress_sec};
183183

184184
return
185185
(not progress_time_enabled_ and not k_handler_enabled) or

0 commit comments

Comments
 (0)