From 3c3eb49d13bf08159eaddcafeb1621f73a95c685 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Mon, 24 Feb 2025 19:47:52 -0500 Subject: [PATCH 1/5] i#7157 dyn inject: Fix extra switch injections Fixes a bug in the dynamic injection logic in the drmemtrace scheduler that caused the context switch sequence to be injected more times than actual context switches. It was erroneously injected also when set_cur_input was invoked without any change to the input. update_switch_stats() is a better control point for switch sequence injection, which is renamed to on_context_switch here. Now, in the switch insertion offline test, we have 11 context switch injections (== input-to-input + idle-to-input switches), instead of the prior 363 Issue: #7157 --- clients/drcachesim/common/memtrace_stream.h | 6 +- .../drcachesim/scheduler/scheduler_impl.cpp | 97 +++++++++++-------- clients/drcachesim/scheduler/scheduler_impl.h | 4 +- .../drcachesim/tests/core_serial.templatex | 1 + .../tests/schedule_stats_nopreempt.templatex | 5 + .../tests/switch_insertion.templatex | 32 ++++-- clients/drcachesim/tools/schedule_stats.cpp | 7 +- clients/drcachesim/tools/schedule_stats.h | 4 +- suite/tests/CMakeLists.txt | 2 +- 9 files changed, 107 insertions(+), 51 deletions(-) diff --git a/clients/drcachesim/common/memtrace_stream.h b/clients/drcachesim/common/memtrace_stream.h index b479ff5785..44e48891e3 100644 --- a/clients/drcachesim/common/memtrace_stream.h +++ b/clients/drcachesim/common/memtrace_stream.h @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2022-2024 Google, Inc. All rights reserved. + * Copyright (c) 2022-2025 Google, Inc. All rights reserved. * **********************************************************/ /* @@ -112,6 +112,10 @@ class memtrace_stream_t { * inputs from being scheduled onto an output. */ SCHED_STAT_HIT_OUTPUT_LIMIT, + /** + * Counts the instances when the context switch sequence was injected. + */ + SCHED_STAT_SWITCH_SEQUENCE_INJECTIONS, /** Count of statistic types. */ SCHED_STAT_TYPE_COUNT, }; diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index 3ff55edf36..091c763a6e 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2023-2024 Google, Inc. All rights reserved. + * Copyright (c) 2023-2025 Google, Inc. All rights reserved. * **********************************************************/ /* @@ -2288,39 +2288,6 @@ scheduler_impl_tmpl_t::set_cur_input( stream->page_size_ = inputs_[input].reader->get_page_size(); } - if (inputs_[input].pid != INVALID_PID) { - insert_switch_tid_pid(inputs_[input]); - } - - if (!switch_sequence_.empty() && - outputs_[output].stream->get_instruction_ordinal() > 0) { - switch_type_t switch_type = sched_type_t::SWITCH_INVALID; - if (prev_workload != inputs_[input].workload) - switch_type = sched_type_t::SWITCH_PROCESS; - else - switch_type = sched_type_t::SWITCH_THREAD; - // Inject kernel context switch code. Since the injected records belong to this - // input (the kernel is acting on behalf of this input) we insert them into the - // input's queue, but ahead of any prior queued items. This is why we walk in - // reverse, for the push_front calls to the deque. We update the tid of the - // records here to match. They are considered as is_record_synthetic() and do - // not affect input stream ordinals. - // XXX: These will appear before the top headers of a new thread which is slightly - // odd to have regular records with the new tid before the top headers. - if (!switch_sequence_[switch_type].empty()) { - for (int i = static_cast(switch_sequence_[switch_type].size()) - 1; - i >= 0; --i) { - RecordType record = switch_sequence_[switch_type][i]; - record_type_set_tid(record, inputs_[input].tid); - inputs_[input].queue.push_front(record); - } - VPRINT(this, 3, - "Inserted %zu switch records for type %d from %d.%d to %d.%d\n", - switch_sequence_[switch_type].size(), switch_type, prev_workload, - outputs_[output].prev_input, inputs_[input].workload, input); - } - } - inputs_[input].prev_time_in_quantum = outputs_[output].cur_time->load(std::memory_order_acquire); @@ -2413,27 +2380,79 @@ scheduler_impl_tmpl_t::pick_next_input(output_ordinal_t } // We can't easily place these stats inside set_cur_input() as we call that to // temporarily give up our input. - update_switch_stats(output, prev_index, index); + on_context_switch(output, prev_index, index); set_cur_input(output, index); return res; } template void -scheduler_impl_tmpl_t::update_switch_stats( +scheduler_impl_tmpl_t::on_context_switch( output_ordinal_t output, input_ordinal_t prev_input, input_ordinal_t new_input) { + bool do_inject_switch_seq = false; if (prev_input == new_input) ++outputs_[output].stats[memtrace_stream_t::SCHED_STAT_SWITCH_NOP]; else if (prev_input != sched_type_t::INVALID_INPUT_ORDINAL && - new_input != sched_type_t::INVALID_INPUT_ORDINAL) + new_input != sched_type_t::INVALID_INPUT_ORDINAL) { ++outputs_[output].stats[memtrace_stream_t::SCHED_STAT_SWITCH_INPUT_TO_INPUT]; - else if (new_input == sched_type_t::INVALID_INPUT_ORDINAL) + do_inject_switch_seq = true; + } else if (new_input == sched_type_t::INVALID_INPUT_ORDINAL) ++outputs_[output].stats[memtrace_stream_t::SCHED_STAT_SWITCH_INPUT_TO_IDLE]; else { ++outputs_[output].stats[memtrace_stream_t::SCHED_STAT_SWITCH_IDLE_TO_INPUT]; // Reset the flag so we'll try to steal if we go idle again. outputs_[output].tried_to_steal_on_idle = false; + do_inject_switch_seq = true; + } + + // We want to insert the context switch sequence on input-to-input and + // idle-to-input cases. This is a better control point to do that than + // set_cur_input. Here we get the stolen input events too, and we don't have + // to filter out the init-time set_cur_input cases. + // RFC: Was there any other benefit to doing this in set_cur_input before? + // If this logic is kept a part of set_cur_input, when USE_INPUT_ORDINALS is + // set, it is harder to differentiate between idle-to-input events and start + // of output; this is because get_instruction_ordinal() does not return the + // intended global value in that case (it returns the input-local value + // which is also not adjusted for the scheduler read-ahead, so is non-zero). + if (do_inject_switch_seq) { + if (inputs_[new_input].pid != INVALID_PID) { + insert_switch_tid_pid(inputs_[new_input]); + } + if (!switch_sequence_.empty()) { + switch_type_t switch_type = sched_type_t::SWITCH_INVALID; + if (prev_input == sched_type_t::INVALID_INPUT_ORDINAL || + inputs_[prev_input].workload != inputs_[new_input].workload) + switch_type = sched_type_t::SWITCH_PROCESS; + else + switch_type = sched_type_t::SWITCH_THREAD; + // Inject kernel context switch code. Since the injected records belong to + // this input (the kernel is acting on behalf of this input) we insert them + // into the input's queue, but ahead of any prior queued items. This is why + // we walk in reverse, for the push_front calls to the deque. We update the + // tid of the records here to match. They are considered as + // is_record_synthetic() and do not affect input stream ordinals. + // XXX: These will appear before the top headers of a new thread which is + // slightly odd to have regular records with the new tid before the top + // headers. + if (!switch_sequence_[switch_type].empty()) { + ++outputs_[output] + .stats[memtrace_stream_t::SCHED_STAT_SWITCH_SEQUENCE_INJECTIONS]; + for (int i = static_cast(switch_sequence_[switch_type].size()) - 1; + i >= 0; --i) { + RecordType record = switch_sequence_[switch_type][i]; + record_type_set_tid(record, inputs_[new_input].tid); + inputs_[new_input].queue.emplace_front(record); + } + VPRINT(this, 3, "Inserted %zu switch for type %d from %d.%d to %d.%d\n", + switch_sequence_[switch_type].size(), switch_type, + prev_input != sched_type_t::INVALID_INPUT_ORDINAL + ? inputs_[prev_input].workload + : -1, + prev_input, inputs_[new_input].workload, new_input); + } + } } } @@ -2653,7 +2672,7 @@ scheduler_impl_tmpl_t::next_record(output_ordinal_t outp } else if (res == sched_type_t::STATUS_STOLE) { // We need to loop to get the new record. input = &inputs_[outputs_[output].cur_input]; - update_switch_stats(output, prev_input, input->index); + on_context_switch(output, prev_input, input->index); lock.unlock(); lock = std::unique_lock(*input->lock); lock.lock(); diff --git a/clients/drcachesim/scheduler/scheduler_impl.h b/clients/drcachesim/scheduler/scheduler_impl.h index 29a429be85..9fa0d0fffb 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.h +++ b/clients/drcachesim/scheduler/scheduler_impl.h @@ -406,8 +406,8 @@ template class scheduler_impl_tmpl_t scale_blocked_time(uint64_t initial_time) const; void - update_switch_stats(output_ordinal_t output, input_ordinal_t prev_input, - input_ordinal_t new_input); + on_context_switch(output_ordinal_t output, input_ordinal_t prev_input, + input_ordinal_t new_input); /// /////////////////////////////////////////////////////////////////////////// diff --git a/clients/drcachesim/tests/core_serial.templatex b/clients/drcachesim/tests/core_serial.templatex index 1fdc9b8531..4ad9fe5dfe 100644 --- a/clients/drcachesim/tests/core_serial.templatex +++ b/clients/drcachesim/tests/core_serial.templatex @@ -8,6 +8,7 @@ Total counts: 106490 instructions per context switch 6 voluntary context switches 0 direct context switches + 0 context switch sequence injections 100.00% voluntary switches 0.00% direct switches 4 switches input-to-input diff --git a/clients/drcachesim/tests/schedule_stats_nopreempt.templatex b/clients/drcachesim/tests/schedule_stats_nopreempt.templatex index b04b4d291d..e511c57a31 100644 --- a/clients/drcachesim/tests/schedule_stats_nopreempt.templatex +++ b/clients/drcachesim/tests/schedule_stats_nopreempt.templatex @@ -8,6 +8,7 @@ Total counts: 106490 instructions per context switch 6 voluntary context switches 0 direct context switches + 0 context switch sequence injections 100\.00% voluntary switches 0\.00% direct switches 5 switches input-to-input @@ -45,6 +46,7 @@ Core #0 counts: *[0-9]* instructions per context switch . voluntary context switches 0 direct context switches + 0 context switch sequence injections 100\.00% voluntary switches 0\.00% direct switches .* @@ -56,6 +58,7 @@ Core #1 counts: *[0-9]* instructions per context switch . voluntary context switches 0 direct context switches + 0 context switch sequence injections 100\.00% voluntary switches 0\.00% direct switches .* @@ -67,6 +70,7 @@ Core #2 counts: *[0-9]* instructions per context switch . voluntary context switches 0 direct context switches + 0 context switch sequence injections 100\.00% voluntary switches 0\.00% direct switches .* @@ -78,6 +82,7 @@ Core #3 counts: *[0-9]* instructions per context switch . voluntary context switches 0 direct context switches + 0 context switch sequence injections 100\.00% voluntary switches 0\.00% direct switches .* diff --git a/clients/drcachesim/tests/switch_insertion.templatex b/clients/drcachesim/tests/switch_insertion.templatex index 91f6dc0466..096cf5355d 100644 --- a/clients/drcachesim/tests/switch_insertion.templatex +++ b/clients/drcachesim/tests/switch_insertion.templatex @@ -1,8 +1,28 @@ -Basic counts tool results: +Schedule stats tool results: Total counts: - [1-9][0-9][0-9][0-9][0-9][0-9] total \(fetched\) instructions - 5971 total unique \(fetched\) instructions - [1-9][0-9][0-9][0-9][0-9][0-9] total userspace instructions - [1-9][0-9][0-9] total kernel instructions - [1-9][0-9][0-9][0-9][0-9][0-9] total non-fetched instructions + 4 cores +.* + 638960 instructions + 11 total context switches +.* + 6 voluntary context switches + 0 direct context switches + 11 context switch sequence injections +.* + 9 switches input-to-input + 5 switches input-to-idle + 2 switches idle-to-input +.* +Core #0 counts: +.* + 117031 instructions + 8 total context switches +.* + 3 voluntary context switches + 0 direct context switches + 8 context switch sequence injections +.* + 6 switches input-to-input + 3 switches input-to-idle + 2 switches idle-to-input .* diff --git a/clients/drcachesim/tools/schedule_stats.cpp b/clients/drcachesim/tools/schedule_stats.cpp index 5d4b26b6b8..abc1ae6e04 100644 --- a/clients/drcachesim/tools/schedule_stats.cpp +++ b/clients/drcachesim/tools/schedule_stats.cpp @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2017-2024 Google, Inc. All rights reserved. + * Copyright (c) 2017-2025 Google, Inc. All rights reserved. * **********************************************************/ /* @@ -166,6 +166,9 @@ schedule_stats_t::get_scheduler_stats(memtrace_stream_t *stream, counters_t &cou memtrace_stream_t::SCHED_STAT_RUNQUEUE_REBALANCES)); counters.at_output_limit = static_cast( stream->get_schedule_statistic(memtrace_stream_t::SCHED_STAT_HIT_OUTPUT_LIMIT)); + counters.switch_sequence_injections = + static_cast(stream->get_schedule_statistic( + memtrace_stream_t::SCHED_STAT_SWITCH_SEQUENCE_INJECTIONS)); // XXX: Currently, schedule_stats is measuring swap-ins to a real input. If we // want to match what "perf" targeting this app would record, which is swap-outs, @@ -420,6 +423,8 @@ schedule_stats_t::print_counters(const counters_t &counters) << counters.voluntary_switches << " voluntary context switches\n"; std::cerr << std::setw(12) << counters.direct_switches << " direct context switches\n"; + std::cerr << std::setw(12) << counters.switch_sequence_injections + << " context switch sequence injections\n"; print_percentage(static_cast(counters.voluntary_switches), static_cast(counters.total_switches), "% voluntary switches\n"); diff --git a/clients/drcachesim/tools/schedule_stats.h b/clients/drcachesim/tools/schedule_stats.h index cc082a8423..95133455d4 100644 --- a/clients/drcachesim/tools/schedule_stats.h +++ b/clients/drcachesim/tools/schedule_stats.h @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2023-2024 Google, Inc. All rights reserved. + * Copyright (c) 2023-2025 Google, Inc. All rights reserved. * **********************************************************/ /* @@ -216,6 +216,7 @@ class schedule_stats_t : public analysis_tool_t { syscalls += rhs.syscalls; maybe_blocking_syscalls += rhs.maybe_blocking_syscalls; direct_switch_requests += rhs.direct_switch_requests; + switch_sequence_injections += rhs.switch_sequence_injections; observed_migrations += rhs.observed_migrations; waits += rhs.waits; idles += rhs.idles; @@ -252,6 +253,7 @@ class schedule_stats_t : public analysis_tool_t { int64_t syscalls = 0; int64_t maybe_blocking_syscalls = 0; int64_t direct_switch_requests = 0; + int64_t switch_sequence_injections = 0; // Our observed migrations will be <= the scheduler's reported migrations // for a dynamic schedule as we don't know the initial runqueue allocation // and so can't see the migration of an input that didn't execute in the diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index aa1c39b6d2..9ec5d3d6f4 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4067,7 +4067,7 @@ if (BUILD_CLIENTS) set(switch_file "${PROJECT_SOURCE_DIR}/clients/drcachesim/tests/mock_switch_sequences.x64.zip") torunonly_simtool(switch_insertion ${ci_shared_app} - "-indir ${thread_trace_dir} -tool basic_counts -core_sharded -sched_quantum 1000 -sched_switch_file ${switch_file}" + "-indir ${thread_trace_dir} -tool schedule_stats -core_sharded -sched_quantum 1000 -sched_switch_file ${switch_file}" "") set(tool.switch_insertion_rawtemp ON) # no preprocessor From 9fdc3b50c621ee770cc879215f70bacc22d46bc6 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Tue, 25 Feb 2025 10:40:34 -0500 Subject: [PATCH 2/5] Reviewer suggested edits --- clients/drcachesim/common/memtrace_stream.h | 4 ++-- clients/drcachesim/scheduler/scheduler_impl.cpp | 4 ++-- clients/drcachesim/tools/schedule_stats.cpp | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clients/drcachesim/common/memtrace_stream.h b/clients/drcachesim/common/memtrace_stream.h index 44e48891e3..b02b4dfddf 100644 --- a/clients/drcachesim/common/memtrace_stream.h +++ b/clients/drcachesim/common/memtrace_stream.h @@ -113,9 +113,9 @@ class memtrace_stream_t { */ SCHED_STAT_HIT_OUTPUT_LIMIT, /** - * Counts the instances when the context switch sequence was injected. + * Counts the instances when the kernel context switch sequence was injected. */ - SCHED_STAT_SWITCH_SEQUENCE_INJECTIONS, + SCHED_STAT_KERNEL_SWITCH_SEQUENCE_INJECTIONS, /** Count of statistic types. */ SCHED_STAT_TYPE_COUNT, }; diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index 091c763a6e..65b4a27efe 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -2437,8 +2437,8 @@ scheduler_impl_tmpl_t::on_context_switch( // slightly odd to have regular records with the new tid before the top // headers. if (!switch_sequence_[switch_type].empty()) { - ++outputs_[output] - .stats[memtrace_stream_t::SCHED_STAT_SWITCH_SEQUENCE_INJECTIONS]; + ++outputs_[output].stats + [memtrace_stream_t::SCHED_STAT_KERNEL_SWITCH_SEQUENCE_INJECTIONS]; for (int i = static_cast(switch_sequence_[switch_type].size()) - 1; i >= 0; --i) { RecordType record = switch_sequence_[switch_type][i]; diff --git a/clients/drcachesim/tools/schedule_stats.cpp b/clients/drcachesim/tools/schedule_stats.cpp index abc1ae6e04..fd7b4849bb 100644 --- a/clients/drcachesim/tools/schedule_stats.cpp +++ b/clients/drcachesim/tools/schedule_stats.cpp @@ -168,7 +168,7 @@ schedule_stats_t::get_scheduler_stats(memtrace_stream_t *stream, counters_t &cou stream->get_schedule_statistic(memtrace_stream_t::SCHED_STAT_HIT_OUTPUT_LIMIT)); counters.switch_sequence_injections = static_cast(stream->get_schedule_statistic( - memtrace_stream_t::SCHED_STAT_SWITCH_SEQUENCE_INJECTIONS)); + memtrace_stream_t::SCHED_STAT_KERNEL_SWITCH_SEQUENCE_INJECTIONS)); // XXX: Currently, schedule_stats is measuring swap-ins to a real input. If we // want to match what "perf" targeting this app would record, which is swap-outs, From 54c4e93b2a67a45bfe6fe83b70c89646f12f1a6a Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Tue, 25 Feb 2025 14:24:40 -0500 Subject: [PATCH 3/5] Reviewer suggested edits --- .../drcachesim/scheduler/scheduler_impl.cpp | 89 +++++++++---------- 1 file changed, 44 insertions(+), 45 deletions(-) diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index 65b4a27efe..6cdf5cf8ce 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -2397,9 +2397,13 @@ scheduler_impl_tmpl_t::on_context_switch( new_input != sched_type_t::INVALID_INPUT_ORDINAL) { ++outputs_[output].stats[memtrace_stream_t::SCHED_STAT_SWITCH_INPUT_TO_INPUT]; do_inject_switch_seq = true; - } else if (new_input == sched_type_t::INVALID_INPUT_ORDINAL) + } else if (new_input == sched_type_t::INVALID_INPUT_ORDINAL) { + // XXX: For now, we do not inject a kernel context switch sequence on + // input-to-idle transitions (note that we do so on idle-to-input though). + // However, we may want to inject some other suitable sequence, but we're not + // sure yet. ++outputs_[output].stats[memtrace_stream_t::SCHED_STAT_SWITCH_INPUT_TO_IDLE]; - else { + } else { ++outputs_[output].stats[memtrace_stream_t::SCHED_STAT_SWITCH_IDLE_TO_INPUT]; // Reset the flag so we'll try to steal if we go idle again. outputs_[output].tried_to_steal_on_idle = false; @@ -2410,50 +2414,45 @@ scheduler_impl_tmpl_t::on_context_switch( // idle-to-input cases. This is a better control point to do that than // set_cur_input. Here we get the stolen input events too, and we don't have // to filter out the init-time set_cur_input cases. - // RFC: Was there any other benefit to doing this in set_cur_input before? - // If this logic is kept a part of set_cur_input, when USE_INPUT_ORDINALS is - // set, it is harder to differentiate between idle-to-input events and start - // of output; this is because get_instruction_ordinal() does not return the - // intended global value in that case (it returns the input-local value - // which is also not adjusted for the scheduler read-ahead, so is non-zero). - if (do_inject_switch_seq) { - if (inputs_[new_input].pid != INVALID_PID) { - insert_switch_tid_pid(inputs_[new_input]); - } - if (!switch_sequence_.empty()) { - switch_type_t switch_type = sched_type_t::SWITCH_INVALID; - if (prev_input == sched_type_t::INVALID_INPUT_ORDINAL || - inputs_[prev_input].workload != inputs_[new_input].workload) - switch_type = sched_type_t::SWITCH_PROCESS; - else - switch_type = sched_type_t::SWITCH_THREAD; - // Inject kernel context switch code. Since the injected records belong to - // this input (the kernel is acting on behalf of this input) we insert them - // into the input's queue, but ahead of any prior queued items. This is why - // we walk in reverse, for the push_front calls to the deque. We update the - // tid of the records here to match. They are considered as - // is_record_synthetic() and do not affect input stream ordinals. - // XXX: These will appear before the top headers of a new thread which is - // slightly odd to have regular records with the new tid before the top - // headers. - if (!switch_sequence_[switch_type].empty()) { - ++outputs_[output].stats - [memtrace_stream_t::SCHED_STAT_KERNEL_SWITCH_SEQUENCE_INJECTIONS]; - for (int i = static_cast(switch_sequence_[switch_type].size()) - 1; - i >= 0; --i) { - RecordType record = switch_sequence_[switch_type][i]; - record_type_set_tid(record, inputs_[new_input].tid); - inputs_[new_input].queue.emplace_front(record); - } - VPRINT(this, 3, "Inserted %zu switch for type %d from %d.%d to %d.%d\n", - switch_sequence_[switch_type].size(), switch_type, - prev_input != sched_type_t::INVALID_INPUT_ORDINAL - ? inputs_[prev_input].workload - : -1, - prev_input, inputs_[new_input].workload, new_input); - } - } + if (!do_inject_switch_seq) + return; + if (inputs_[new_input].pid != INVALID_PID) { + insert_switch_tid_pid(inputs_[new_input]); } + if (switch_sequence_.empty()) + return; + switch_type_t switch_type = sched_type_t::SWITCH_INVALID; + if ( // idle-to-input transitions are assumed to be process switches. + prev_input == sched_type_t::INVALID_INPUT_ORDINAL || + inputs_[prev_input].workload != inputs_[new_input].workload) + switch_type = sched_type_t::SWITCH_PROCESS; + else + switch_type = sched_type_t::SWITCH_THREAD; + if (switch_sequence_[switch_type].empty()) + return; + // Inject kernel context switch code. Since the injected records belong to + // this input (the kernel is acting on behalf of this input) we insert them + // into the input's queue, but ahead of any prior queued items. This is why + // we walk in reverse, for the push_front calls to the deque. We update the + // tid of the records here to match. They are considered as + // is_record_synthetic() and do not affect input stream ordinals. + // XXX: These will appear before the top headers of a new thread which is + // slightly odd to have regular records with the new tid before the top + // headers. + ++outputs_[output] + .stats[memtrace_stream_t::SCHED_STAT_KERNEL_SWITCH_SEQUENCE_INJECTIONS]; + for (int i = static_cast(switch_sequence_[switch_type].size()) - 1; i >= 0; + --i) { + RecordType record = switch_sequence_[switch_type][i]; + record_type_set_tid(record, inputs_[new_input].tid); + inputs_[new_input].queue.emplace_front(record); + } + VPRINT(this, 3, "Inserted %zu switch for type %d from %d.%d to %d.%d\n", + switch_sequence_[switch_type].size(), switch_type, + prev_input != sched_type_t::INVALID_INPUT_ORDINAL + ? inputs_[prev_input].workload + : -1, + prev_input, inputs_[new_input].workload, new_input); } template From 18d75b0ea79963c9eea36bbc97dfb32805d4fe1e Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Tue, 25 Feb 2025 14:25:11 -0500 Subject: [PATCH 4/5] Update comment; enum was already updated before --- clients/drcachesim/common/memtrace_stream.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/drcachesim/common/memtrace_stream.h b/clients/drcachesim/common/memtrace_stream.h index 31d4a6ea50..b02b4dfddf 100644 --- a/clients/drcachesim/common/memtrace_stream.h +++ b/clients/drcachesim/common/memtrace_stream.h @@ -113,7 +113,7 @@ class memtrace_stream_t { */ SCHED_STAT_HIT_OUTPUT_LIMIT, /** - * Counts the instances when the context switch sequence was injected. + * Counts the instances when the kernel context switch sequence was injected. */ SCHED_STAT_KERNEL_SWITCH_SEQUENCE_INJECTIONS, /** Count of statistic types. */ From d881097d2667bd891c0c7bfe8757d2c3bce59075 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Tue, 25 Feb 2025 16:02:18 -0500 Subject: [PATCH 5/5] Change to XXX comment --- clients/drcachesim/scheduler/scheduler_impl.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index 6cdf5cf8ce..eeda7fbe1b 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -2422,7 +2422,8 @@ scheduler_impl_tmpl_t::on_context_switch( if (switch_sequence_.empty()) return; switch_type_t switch_type = sched_type_t::SWITCH_INVALID; - if ( // idle-to-input transitions are assumed to be process switches. + if ( // XXX: idle-to-input transitions are assumed to be process switches + // for now. But we may want to improve this heuristic. prev_input == sched_type_t::INVALID_INPUT_ORDINAL || inputs_[prev_input].workload != inputs_[new_input].workload) switch_type = sched_type_t::SWITCH_PROCESS;