Add a Job Status enum + scheduler refactoring #92
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Address the `S101` failure with ruff check. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Break apart a couple of the more complex functions to resolve ruff lint warnings related to method complexity. Also take the opportunity to slightly improve some of the documentation on these functions and to refactor some of the code to be nicer to read. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
There may still be more work needed on the typing front but this resolves the singular remaining type error given by pyright within the scheduler. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Currently the job status is passed around DVSim as a string, where the
expectation is that it is always a single upper case character in ('Q',
'D', 'P', 'F', 'K') - some limited parts of the code also handle an 'E'
error case.
Turn these into a small Enum type so that we have much stricter typing
on these values and improved ergonomics when referring to different job
statuses in code.
This status Enum lives in its own module instead of with the other job
data to avoid circular imports (as the job status' registered callbacks
reference the launcher definition).
Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
AlexJones0
force-pushed
the
scheduler_cleanup
branch
from
c8dd48b to
4f91b01
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR performs some small cleanup of the scheduler in preparation for potential larger-scale changes. Specifically, it aims to fix any outstanding
pyrightandruff checkfailures that exist within the scheduler itself, to provide a good base for further refactoring or modification.One other clear deficiency in the design that I wanted to address was the use of a string (always a single upper-case character) like "D", "P" or "K" to refer to the Job status, rather than the more standard Enumerated type. Using an Enum improves code readability & ergonomics, adds stricter typing of status values and better modularises the code - this is however a more wide-spread change that touches many different parts of DVSim, albeit a simple one.