Skip to content

Commit

Permalink
Review requests: dup type error and test; rename field; more comments…
Browse files Browse the repository at this point in the history 
… and checks
  • Loading branch information
@derekbruening
derekbruening committed Dec 18, 2023
1 parent 30c20fa commit 7d422e9
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 9 deletions.
13 changes: 9 additions & 4 deletions clients/drcachesim/scheduler/scheduler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,7 @@ scheduler_tmpl_t<RecordType, ReaderType>::read_switch_sequences()
}
reader = std::move(options_.kernel_switch_reader);
reader_end = std::move(options_.kernel_switch_reader_end);
// We own calling init() as it can block.
if (!reader->init()) {
error_string_ += "Failed to init kernel switch reader";
return STATUS_ERROR_INVALID_PARAMETER;
Expand All @@ -1201,6 +1202,10 @@ scheduler_tmpl_t<RecordType, ReaderType>::read_switch_sequences()
if (record_type_is_marker(record, marker_type, marker_value) &&
marker_type == TRACE_MARKER_TYPE_CONTEXT_SWITCH_START) {
switch_type = static_cast<sched_type_t::switch_type_t>(marker_value);
if (!switch_sequence_[switch_type].empty()) {
error_string_ += "Duplicate context switch sequence type found";
return STATUS_ERROR_INVALID_PARAMETER;
}
}
if (switch_type != SWITCH_INVALID)
switch_sequence_[switch_type].push_back(record);
Expand Down Expand Up @@ -1375,7 +1380,7 @@ scheduler_tmpl_t<RecordType, ReaderType>::is_record_synthetic(output_ordinal_t o
int index = outputs_[output].cur_input;
if (index < 0)
return false;
if (outputs_[output].in_switch_code)
if (outputs_[output].in_context_switch_code)
return true;
return inputs_[index].reader->is_record_synthetic();
}
Expand Down Expand Up @@ -2400,8 +2405,8 @@ scheduler_tmpl_t<RecordType, ReaderType>::next_record(output_ordinal_t output,
}
}
if (outputs_[output].hit_switch_code_end) {
// We have to delay so the end marker is still in_switch_code.
outputs_[output].in_switch_code = false;
// We have to delay so the end marker is still in_context_switch_code.
outputs_[output].in_context_switch_code = false;
outputs_[output].hit_switch_code_end = false;
// We're now back "on the clock".
if (options_.quantum_unit == QUANTUM_TIME)
Expand All @@ -2426,7 +2431,7 @@ scheduler_tmpl_t<RecordType, ReaderType>::next_record(output_ordinal_t output,
marker_type == TRACE_MARKER_TYPE_CONTEXT_SWITCH_START)) {
outputs_[output].in_kernel_code = true;
if (marker_type == TRACE_MARKER_TYPE_CONTEXT_SWITCH_START)
outputs_[output].in_switch_code = true;
outputs_[output].in_context_switch_code = true;
} else if (record_type_is_marker(record, marker_type, marker_value) &&
(marker_type == TRACE_MARKER_TYPE_SYSCALL_TRACE_END ||
marker_type == TRACE_MARKER_TYPE_CONTEXT_SWITCH_END)) {
Expand Down
7 changes: 5 additions & 2 deletions clients/drcachesim/scheduler/scheduler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -562,8 +562,11 @@ template <typename RecordType, typename ReaderType> class scheduler_tmpl_t {
* #block_time_scale.
*/
uint64_t block_time_max = 25000000;
// XXX: Should we share the file-to-reader code currently in the scheduler
// with the analyzer and only then need reader interfaces and not pass paths
// to the scheduler?
/**
* Input file containing canned sequences of kernel context switch code.
* Input file containing template sequences of kernel context switch code.
* Each sequence must start with a #TRACE_MARKER_TYPE_CONTEXT_SWITCH_START
* marker and end with #TRACE_MARKER_TYPE_CONTEXT_SWITCH_END.
* The values of each marker must hold a #switch_type_t enum value
Expand Down Expand Up @@ -1179,7 +1182,7 @@ template <typename RecordType, typename ReaderType> class scheduler_tmpl_t {
bool waiting = false; // Waiting or idling.
bool active = true;
bool in_kernel_code = false;
bool in_switch_code = false;
bool in_context_switch_code = false;
bool hit_switch_code_end = false;
// Used for time-based quanta.
uint64_t cur_time = 0;
Expand Down
48 changes: 45 additions & 3 deletions clients/drcachesim/tests/scheduler_unit_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3564,7 +3564,6 @@ test_kernel_switch_sequences()
auto switch_reader =
std::unique_ptr<mock_reader_t>(new mock_reader_t(switch_sequence));
auto switch_reader_end = std::unique_ptr<mock_reader_t>(new mock_reader_t());

static constexpr int NUM_WORKLOADS = 3;
static constexpr int NUM_INPUTS_PER_WORKLOAD = 3;
static constexpr int NUM_OUTPUTS = 2;
Expand Down Expand Up @@ -3676,7 +3675,7 @@ test_kernel_switch_sequences()
else if (memref.marker.marker_value == scheduler_t::SWITCH_THREAD)
sched_as_string[i] += 't';
else
sched_as_string[i] += '?';
assert(false && "unknown context switch type");
break;
default: sched_as_string[i] += '?'; break;
}
Expand Down Expand Up @@ -3742,7 +3741,50 @@ test_kernel_switch_sequences()
// The 3-instr quantum should not count the 2 switch instrs.
check_ref(refs[0], idx, TID_BASE + 4, TRACE_TYPE_INSTR) &&
check_ref(refs[0], idx, TID_BASE + 4, TRACE_TYPE_INSTR) &&
check_ref(refs[0], idx, TID_BASE + 4, TRACE_TYPE_INSTR) && true;
check_ref(refs[0], idx, TID_BASE + 4, TRACE_TYPE_INSTR);

{
// Test a bad input sequence.
std::vector<trace_entry_t> bad_switch_sequence = {
/* clang-format off */
make_header(TRACE_ENTRY_VERSION),
make_thread(TID_IN_SWITCHES),
make_pid(TID_IN_SWITCHES),
make_marker(TRACE_MARKER_TYPE_CONTEXT_SWITCH_START, scheduler_t::SWITCH_PROCESS),
make_instr(PROCESS_SWITCH_PC_START),
make_marker(TRACE_MARKER_TYPE_CONTEXT_SWITCH_END, scheduler_t::SWITCH_PROCESS),
make_footer(),
make_header(TRACE_ENTRY_VERSION),
make_thread(TID_IN_SWITCHES),
make_pid(TID_IN_SWITCHES),
// Error: duplicate type.
make_marker(TRACE_MARKER_TYPE_CONTEXT_SWITCH_START, scheduler_t::SWITCH_PROCESS),
make_instr(PROCESS_SWITCH_PC_START),
make_marker(TRACE_MARKER_TYPE_CONTEXT_SWITCH_END, scheduler_t::SWITCH_PROCESS),
make_footer(),
/* clang-format on */
};
auto bad_switch_reader =
std::unique_ptr<mock_reader_t>(new mock_reader_t(bad_switch_sequence));
auto switch_reader_end = std::unique_ptr<mock_reader_t>(new mock_reader_t());
std::vector<scheduler_t::input_workload_t> sched_inputs;
std::vector<scheduler_t::input_reader_t> readers;
std::vector<trace_entry_t> inputs;
inputs.push_back(make_header(TRACE_ENTRY_VERSION));
readers.emplace_back(std::unique_ptr<mock_reader_t>(new mock_reader_t(inputs)),
std::unique_ptr<mock_reader_t>(new mock_reader_t()),
TID_BASE);
sched_inputs.emplace_back(std::move(readers));
scheduler_t::scheduler_options_t sched_ops(scheduler_t::MAP_TO_ANY_OUTPUT,
scheduler_t::DEPENDENCY_TIMESTAMPS,
scheduler_t::SCHEDULER_DEFAULTS);
sched_ops.kernel_switch_reader = std::move(bad_switch_reader);
sched_ops.kernel_switch_reader_end = std::move(switch_reader_end);
scheduler_t scheduler;
if (scheduler.init(sched_inputs, NUM_OUTPUTS, std::move(sched_ops)) !=
scheduler_t::STATUS_ERROR_INVALID_PARAMETER)
assert(false);
}
}

int
Expand Down

0 comments on commit 7d422e9

Please sign in to comment.