Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i#6726 replay cpuid: Sort as-traced outputs by cpuid #6729

Merged
merged 7 commits into from
Mar 27, 2024

Conversation

derekbruening
Copy link
Contributor

Currently, in as-traced mode the output streams are assigned to the cores in the as-traced schedule file in file order. But that order is essentially random, which scrambles key arrangements like which core is on which socket. Here we sort by the recorded cpuid to recreate the same cpuid order as before.

Adds a unit test. Also tested on larger cases with > 100 cores.

Issue: #6726

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
Currently, in as-traced mode the output streams are assigned to the
cores in the as-traced schedule file in file order.  But that order is
essentially random, which scrambles key arrangements like which core
is on which socket.  Here we sort by the recorded cpuid to recreate
the same cpuid order as before.

Adds a unit test.  Also tested on larger cases with > 100 cores.

Issue: #6726
clients/drcachesim/tests/scheduler_unit_tests.cpp Outdated Show resolved Hide resolved
clients/drcachesim/scheduler/scheduler.cpp Outdated Show resolved Hide resolved
clients/drcachesim/scheduler/scheduler.cpp Outdated Show resolved Hide resolved
clients/drcachesim/scheduler/scheduler.cpp Outdated Show resolved Hide resolved
clients/drcachesim/scheduler/scheduler.cpp Show resolved Hide resolved
clients/drcachesim/scheduler/scheduler.cpp Outdated Show resolved Hide resolved
clients/drcachesim/scheduler/scheduler.cpp Outdated Show resolved Hide resolved
clients/drcachesim/scheduler/scheduler.cpp Outdated Show resolved Hide resolved
clients/drcachesim/scheduler/scheduler.cpp Outdated Show resolved Hide resolved
clients/drcachesim/scheduler/scheduler.cpp Outdated Show resolved Hide resolved
clients/drcachesim/scheduler/scheduler.cpp Outdated Show resolved Hide resolved
Base automatically changed from i6721-synthetic-header-bug to master March 27, 2024 04:56
@derekbruening derekbruening merged commit d20abf4 into master Mar 27, 2024
16 checks passed
@derekbruening derekbruening deleted the i6726-sort-cpuids branch March 27, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants