Switch to use the new scheduler & remove old unused Scheduler & Launcher logic#172
Conversation
AlexJones0
changed the title
AlexJones0
force-pushed
the
switch_scheduler
branch
from
c0e6b1f to
bd6d6f4
Compare
machshev
left a comment
machshev
left a comment
There was a problem hiding this comment.
Thanks @AlexJones0! Really good work, this is a huge improvement overall.
In terms of migration I'd suggest merging this sooner rather than later. Opentitan and Mocha both use pinned versions of the package, so I don't think there is much risk in releasing. By default the OT nightly regressions will run the latest package and we can use that as a final check (it's more difficult to do this with a feature flag, although still possible). If there are issues then we can pin the nightly to the last known good and focus on patching any issues.
Personally I'd opt for the moving fast approach here as trying to maintain two schedulers in parallel is likely to introduce more risk than just switching. The OT and mocha pinned version of DVSim can be updated after a successful nightly regression run, potentially delayed a couple of days if people want to test it out locally themselves.
AlexJones0
force-pushed
the
switch_scheduler
branch
from
bd6d6f4 to
ef8e112
Compare
AlexJones0
force-pushed
the
switch_scheduler
branch
from
ef8e112 to
758adc5
Compare
This commit makes the full transition to use the new async scheduler in place of the old scheduler, instead of hiding it behind an experimental environment variable. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
These have now been replaced by equivalent / improved asynchronous interfaces. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Now that the async scheduler and status printer have been made the defaults, and the old versions have been removed, rename the `async_core` and `async_status_printer` modules to just be `core` and `status_printer` as before, respectively. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This `Timer` class was only used by the scheduler and is not particularly useful, especially in async contexts. Deprecate and remove this util module. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Now that everything goes through the RuntimeBackend registry instead, there is no need for the Launcher factory to exist any more, as it has been superseded. Likewise, the LocalLauncher and FakeLauncher have been replaced by the LocalRuntimeBackend and FakeRuntimeBackend respectively - as such, these legacy launchers can also be removed. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
AlexJones0
force-pushed
the
switch_scheduler
branch
from
758adc5 to
bd37201
Compare
|
The last few force pushes have all been rebasing on |
2 participants
Note: this PR is currently a draft as it depends on #136 which has not yet been merged; the first 3 commits are from that PR and can be safely ignored. Only the last 5 commits are relevant. It is otherwise ready to review.
This PR is the twentieth of a series of PRs to rewrite DVSim's core scheduling functionality (Scheduler, status display, launchers / runtime backends) to use an async design, with key goals of long term maintainability and extensibility.
This PR makes the major change to switch to the new scheduler that was previously behind the
--experimental-enable-async-schedulerflag, and removes the old scheduler and launcher logic that has been ported. From this point onwards, the new scheduler will be the default when using DVSim.Note: ideally we would want to test the new scheduler a bit more thoroughly with general usage before this PR is merged, to give some confidence. Also, it is an open question whether it is fine to switch like this, or whether we should still support the old scheduler for some time through an optional flag. This would likely depend on perceived stability.