i#7318 rebalance: Ignore bind-to-all

If an input is bound to every output, we now ignore those bindings,
which eliminates complexities in initial output allocation where
rebalancing is required to even things out (and that gets complicated
by initially-blocked inputs) as well as removes overhead in the
runqueue code during dynamic scheduling. Binding to every output is
actually not uncommon as users have such bindings as a default value.

As part of detecting bind-to-all, invalid bindings are now detected.

Adds unit tests for the invalid bindings.

Updates the unbalanced rebalance test to bind to all but one as
binding to all is no longer unbalanced.

Issue: #7318

Author: derekbruening

clients/drcachesim/scheduler/scheduler_impl.cpp

@@ -837,7 +837,21 @@ scheduler_impl_tmpl_t<RecordType, ReaderType>::init(
                 int index = reader_info.tid2input[tid];
                 input_info_t &input = inputs_[index];
                 input.has_modifier = true;
-                input.binding = modifiers.output_binding;
+                // Check for valid bindings.
+                for (output_ordinal_t bind : modifiers.output_binding) {
+                    if (bind < 0 || bind > output_count) {
+                        error_string_ =
+                            "output_binding " + std::to_string(bind) + " out of bounds";
+                        return sched_type_t::STATUS_ERROR_INVALID_PARAMETER;
+                    }
+                }
+                // It is common enough for every output to be passed (as part of general
+                // code with a full set as a default value) that it is worth
+                // detecting and ignoring in order to avoid hitting binding-handling
+                // code and save time in initial placement and runqueue code.
+                if (modifiers.output_binding.size() < static_cast<size_t>(output_count)) {
+                    input.binding = modifiers.output_binding;
+                }
                 input.priority = modifiers.priority;
                 for (size_t i = 0; i < modifiers.regions_of_interest.size(); ++i) {
                     const auto &range = modifiers.regions_of_interest[i];

clients/drcachesim/tests/scheduler_unit_tests.cpp

@@ -2187,13 +2187,69 @@ test_synthetic_with_bindings_weighted()
     assert(sched_as_string[4] == ".AA.AA.AA.AA.A.______________________________");
 }
 
+static void
+test_synthetic_with_bindings_invalid()
+{
+    std::cerr << "\n----------------\nTesting synthetic with invalid bindings\n";
+    static constexpr memref_tid_t TID_A = 42;
+    std::vector<trace_entry_t> refs_A = {
+        /* clang-format off */
+        make_thread(TID_A),
+        make_pid(1),
+        make_version(TRACE_ENTRY_VERSION),
+        make_timestamp(1),
+        make_instr(10),
+        make_exit(TID_A),
+        /* clang-format on */
+    };
+    {
+        // Test negative bindings.
+        static constexpr int NUM_OUTPUTS = 2;
+        std::vector<scheduler_t::input_reader_t> readers;
+        readers.emplace_back(std::unique_ptr<mock_reader_t>(new mock_reader_t(refs_A)),
+                             std::unique_ptr<mock_reader_t>(new mock_reader_t()), TID_A);
+        std::vector<scheduler_t::input_workload_t> sched_inputs;
+        sched_inputs.emplace_back(std::move(readers));
+        std::set<scheduler_t::output_ordinal_t> cores;
+        cores.insert({ 1, -1 });
+        sched_inputs.back().thread_modifiers.emplace_back(cores);
+        scheduler_t::scheduler_options_t sched_ops(scheduler_t::MAP_TO_ANY_OUTPUT,
+                                                   scheduler_t::DEPENDENCY_TIMESTAMPS,
+                                                   scheduler_t::SCHEDULER_DEFAULTS,
+                                                   /*verbosity=*/3);
+        scheduler_t scheduler;
+        assert(scheduler.init(sched_inputs, NUM_OUTPUTS, std::move(sched_ops)) ==
+               scheduler_t::STATUS_ERROR_INVALID_PARAMETER);
+    }
+    {
+        // Test too-large bindings.
+        static constexpr int NUM_OUTPUTS = 2;
+        std::vector<scheduler_t::input_reader_t> readers;
+        readers.emplace_back(std::unique_ptr<mock_reader_t>(new mock_reader_t(refs_A)),
+                             std::unique_ptr<mock_reader_t>(new mock_reader_t()), TID_A);
+        std::vector<scheduler_t::input_workload_t> sched_inputs;
+        sched_inputs.emplace_back(std::move(readers));
+        std::set<scheduler_t::output_ordinal_t> cores;
+        cores.insert({ 1, 3 });
+        sched_inputs.back().thread_modifiers.emplace_back(cores);
+        scheduler_t::scheduler_options_t sched_ops(scheduler_t::MAP_TO_ANY_OUTPUT,
+                                                   scheduler_t::DEPENDENCY_TIMESTAMPS,
+                                                   scheduler_t::SCHEDULER_DEFAULTS,
+                                                   /*verbosity=*/3);
+        scheduler_t scheduler;
+        assert(scheduler.init(sched_inputs, NUM_OUTPUTS, std::move(sched_ops)) ==
+               scheduler_t::STATUS_ERROR_INVALID_PARAMETER);
+    }
+}
+
 static void
 test_synthetic_with_bindings()
 {
     test_synthetic_with_bindings_time(/*time_deps=*/true);
     test_synthetic_with_bindings_time(/*time_deps=*/false);
     test_synthetic_with_bindings_more_out();
     test_synthetic_with_bindings_weighted();
+    test_synthetic_with_bindings_invalid();
 }
 
 static void
@@ -5676,13 +5732,14 @@ test_unscheduled_initially_rebalance()
     // We need the initial runqueue assignment to be unbalanced.
     // We achieve that by using input bindings.
     // This relies on knowing the scheduler takes the 1st binding if there
-    // are any: so we can set to all cores and these will all pile up on output #0
+    // are any if the bindings don't include all cores: so we can set to all-but-one
+    // core and these will all pile up on output #0
     // prior to the init-time rebalance.  That makes output
     // #0 big enough for a rebalance attempt, which causes scheduler init to fail
     // without the i#7318 fix as it can only move one of those blocked inputs and
     // so it hits an IDLE status on a later move attempt.
     std::set<scheduler_t::output_ordinal_t> cores;
-    cores.insert({ 0, 1, 2 });
+    cores.insert({ 0, 1 });
     std::vector<scheduler_t::input_workload_t> sched_inputs;
     sched_inputs.emplace_back(std::move(readers));
     sched_inputs.back().thread_modifiers.emplace_back(cores);
@@ -7026,6 +7083,11 @@ test_main(int argc, const char *argv[])
     // Avoid races with lazy drdecode init (b/279350357).
     dr_standalone_init();
 
+    if (argc < 20) {
+        test_synthetic_with_bindings_invalid();
+        test_unscheduled_initially_rebalance();
+        return 0;
+    }
     test_serial();
     test_parallel();
     test_param_checks();