i#6721: Read ahead in as-traced drmemtrace mode (#6722)

Removes the exception for avoiding reading ahead to get the version and
filetype for as-traced drmemtrace mode, as we need those values for the
record_filter.

Adds explicit error checking and error return up front to the
record_filter for missing version info.

Adjusts the scheduler unit test's record ordinal to account for the new
read-ahead.

Adds a test with a checked-in trace of hello,world (truncated to make it
smaller) that ran on 4 different cores. The test only reproduces the
problem if the read-ahead in open_reader() to find the tid is removed,
which I did temporarily; I kept the test as it is also a good additional
general test of other corner cases. To create a test that always
reproduces in our suite we would need to combine a synthetic reader with
use of the scheduler in as-traced mode, which none of our unit tests are
set up for.

The fix was further tested on large inputs in an internal setup which
always passes a synthetic reader, where the problem reproduces every
time without the fix.

Fixes #6721

clients/drcachesim/scheduler/scheduler.cpp

@@ -809,10 +809,6 @@ scheduler_tmpl_t<RecordType, ReaderType>::set_initial_schedule(
     // The filetype, if present, is before the first timestamp.  If we only need the
     // filetype we avoid going as far as the timestamp.
     bool gather_filetype = options_.read_inputs_in_init;
-    // Avoid reading ahead for replay as it makes the input ords not match in tests.
-    if (options_.mapping == MAP_TO_RECORDED_OUTPUT &&
-        options_.replay_as_traced_istream != nullptr)
-        gather_filetype = false;
     if (gather_filetype || gather_timestamps) {
         sched_type_t::scheduler_status_t res =
             get_initial_input_content(gather_timestamps);
@@ -1043,7 +1039,11 @@ scheduler_tmpl_t<RecordType, ReaderType>::read_recorded_schedule()
     }
     for (int i = 0; i < static_cast<output_ordinal_t>(outputs_.size()); ++i) {
         if (outputs_[i].record.empty()) {
+            // XXX i#6630: We should auto-set the output count and avoid
+            // having extra outputs; these complicate idle computations, etc.
+            VPRINT(this, 1, "output %d empty: returning eof up front\n", i);
             set_cur_input(i, INVALID_INPUT_ORDINAL);
+            outputs_[i].at_eof = true;
         } else if (outputs_[i].record[0].type == schedule_record_t::IDLE) {
             set_cur_input(i, INVALID_INPUT_ORDINAL);
             outputs_[i].waiting = true;
@@ -1205,8 +1205,13 @@ scheduler_tmpl_t<RecordType, ReaderType>::read_traced_schedule()
                        outputs_[output_idx].record[0].key.input);
                 set_cur_input(output_idx, outputs_[output_idx].record[0].key.input);
             }
-        } else
+        } else {
+            // XXX i#6630: We should auto-set the output count and avoid
+            // having extra ouputs; these complicate idle computations, etc.
+            VPRINT(this, 1, "Output %d empty: returning eof up front\n", output_idx);
+            outputs_[output_idx].at_eof = true;
             set_cur_input(output_idx, INVALID_INPUT_ORDINAL);
+        }
     }
     return STATUS_SUCCESS;
 }

clients/drcachesim/tests/record_filter_start_idle.templatex

@@ -0,0 +1,7 @@
+Output .* entries from .* entries.
+Schedule stats tool results:
+.*
+Core #0 schedule: .*
+Core #1 schedule: .*
+Core #2 schedule: .*
+Core #3 schedule: .*

clients/drcachesim/tests/scheduler_unit_tests.cpp

@@ -3423,7 +3423,10 @@ test_replay_as_traced_from_file(const char *testdir)
     static const char *const SCHED_STRING =
         "Core #0: 1257598 \nCore #1: 1257603 \nCore #2: 1257601 \n"
         "Core #3: 1257599 => 1257604 @ <366987,87875,13331862029895453> "
-        "(<366986,87875,13331862029895453> => <1,0,0>) \n"
+        // The ordinal is really 1 ("<1,0,0>") but with the scheduler's readahead
+        // it becomes 2; easier to just check for that as trying to avoid readahead
+        // causes other problems with start-idle cores (i#6721).
+        "(<366986,87875,13331862029895453> => <2,0,0>) \n"
         "Core #4: 1257600 \nCore #5: 1257596 \nCore #6: 1257602 \n";
     static constexpr int NUM_OUTPUTS = 7; // Matches the actual trace's core footprint.
     scheduler_t scheduler;

clients/drcachesim/tools/filter/record_filter.cpp

@@ -208,6 +208,10 @@ record_filter_t::initialize_shard_output(per_shard_t *per_shard,
         // Now synchronize determining the extension.
         auto lock = std::unique_lock<std::mutex>(input_info_mutex_);
         if (!output_ext_.empty()) {
+            VPRINT(this, 2,
+                   "Shard #%d using pre-set ext=%s, ver=%" PRIu64 ", type=%" PRIu64 "\n",
+                   shard_stream->get_shard_index(), output_ext_.c_str(), version_,
+                   filetype_);
             per_shard->output_path += output_ext_;
             lock.unlock();
         } else if (!input_name.empty()) {
@@ -218,12 +222,25 @@ record_filter_t::initialize_shard_output(per_shard_t *per_shard,
             // Set the other key input data.
             version_ = shard_stream->get_version();
             filetype_ = add_to_filetype(shard_stream->get_filetype());
+            if (version_ == 0) {
+                // We give up support for version 0 to have an up-front error check
+                // rather than having some output files with bad headers (i#6721).
+                return "Version not available at shard init time";
+            }
+            VPRINT(this, 2,
+                   "Shard #%d setting ext=%s, ver=%" PRIu64 ", type=%" PRIu64 "\n",
+                   shard_stream->get_shard_index(), output_ext_.c_str(), version_,
+                   filetype_);
             per_shard->output_path += output_ext_;
             lock.unlock();
             input_info_cond_var_.notify_all();
         } else {
             // We have to wait for another shard with an input to set output_ext_.
             input_info_cond_var_.wait(lock, [this] { return !output_ext_.empty(); });
+            VPRINT(this, 2,
+                   "Shard #%d waited for ext=%s, ver=%" PRIu64 ", type=%" PRIu64 "\n",
+                   shard_stream->get_shard_index(), output_ext_.c_str(), version_,
+                   filetype_);
             per_shard->output_path += output_ext_;
             lock.unlock();
         }
@@ -649,6 +666,10 @@ record_filter_t::process_delayed_encodings(per_shard_t *per_shard, trace_entry_t
 bool
 record_filter_t::parallel_shard_memref(void *shard_data, const trace_entry_t &input_entry)
 {
+    if (!success_) {
+        // Report an error that happened during shard init.
+        return false;
+    }
     per_shard_t *per_shard = reinterpret_cast<per_shard_t *>(shard_data);
     ++per_shard->input_entry_count;
     trace_entry_t entry = input_entry;

suite/tests/CMakeLists.txt

@@ -4696,6 +4696,24 @@ if (BUILD_CLIENTS)
       set(tool.record_filter_as_traced_basedir
         "${PROJECT_SOURCE_DIR}/clients/drcachesim/tests")
       set(tool.record_filter_as_traced_rawtemp ON) # no preprocessor
+
+      # Test the record_filter in as-traced mode with start-idle cores.
+      # This checked-in partial-hello,world-trace ran on 4 different cores.
+      set(trace_dir
+        "${PROJECT_SOURCE_DIR}/clients/drcachesim/tests/drmemtrace.as_traced.x64.tracedir")
+      set(sched_file "${trace_dir}/cpu_schedule.bin.zip")
+      set(outdir ${CMAKE_CURRENT_BINARY_DIR}/filter_start_idle)
+      file(MAKE_DIRECTORY ${outdir})
+      torunonly_api(tool.record_filter_start_idle "${drcachesim_path}"
+        "record_filter_start_idle"
+        "" "-simulator_type;schedule_stats;-indir;${outdir}" OFF OFF)
+      set(tool.record_filter_start_idle_runcmp "${CMAKE_CURRENT_SOURCE_DIR}/runmulti.cmake")
+      set(tool.record_filter_start_idle_precmd
+        "${drcachesim_path}@-simulator_type@record_filter@-cpu_schedule_file@${sched_file}@-core_sharded@-cores@4@-indir@${trace_dir}@-outdir@${outdir}")
+      set(tool.record_filter_start_idle_basedir
+        "${PROJECT_SOURCE_DIR}/clients/drcachesim/tests")
+      set(tool.record_filter_start_idle_rawtemp ON) # no preprocessor
+
     endif ()
 
     if (AARCH64)