Instrumentation refactoring cleanups & UX improvements#193
Merged
AlexJones0 merged 4 commits intoMay 14, 2026
Conversation
machshev
approved these changes
May 14, 2026
Collaborator
machshev
left a comment
There was a problem hiding this comment.
Nice, thanks @AlexJones0!
Reduce duplication of naming across the instrumentation logic to try and keep a single source of truth, which will be useful when trying to load back JSON instrumentation reports. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Users may want the ability to choose whether metadata should be included in instrumentation reports or not - it may not be relevant to their use case, and bloat the size of the generated reports for large runs. Also, the previous implicit behaviour where we automatically include it if you use any other type of instrumentation is not particularly nice or intuitive. Now, users must implicitly include metadata instrumentation with the "meta" type. As the unique `job_id`, the full name will necessarily always be included in the output instrumentation report if any type of instrumentation is used. But all other metadata must be explicitly included. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
It can be arduous to define all the instrumentation types when a relatively common pattern will be to just collect all the data. Introduce a special 'all' keyword that internally maps to just using all instrumentation types that are available. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Since this "resources" instrumentation mechanism was added, resources have been separately introduced to DVSim as the concept of resource proxies used by jobs to define more granular runtime parallelism limits. Now the naming and command-line options are overlapping somewhat. "Resources" was also a bit of a generic name anyway, so lets rename this to "compute" - short for "compute resources" - to more accurately reflect what we're capturing (and to make it more distinct). Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
b6374d6 to
288138c
Compare
Contributor
Author
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 is the fourth of a series of PRs to introduce instrumentation reporting to DVSim, which in this case also involves refactoring the existing instrumentation implementation to allow better modeling & static typing around this use case.
This PR performs some final small cleanups after the instrumentation refactoring work in the preceding PRs. Alongside removing some duplication of the instrumentation names, it primarily focuses on improving the user experience:
resourcesinstrumentation tocompute, to make it clear that we are measuring "compute resources" and not the new "resources" concept that was introduced to the scheduler in Augment the scheduler with resources to allow more fine-grained parallelism limitation #184 for more granular parallelism limits.allinstrumentation type which automatically includes all the available instrumentations. This lets you type--instrument allinstead of--instrument meta timing compute, which can get overly verbose.See the commit messages for more info.