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

Add support for custom gauge adapters #209

Merged
merged 6 commits into from Mar 20, 2023
Merged

Add support for custom gauge adapters #209

merged 6 commits into from Mar 20, 2023

Conversation

smarr
Copy link
Owner

@smarr smarr commented Mar 18, 2023

At the moment, we have only a limited, predefined set of gauge adapters in ReBench.

With this PR, we add support to provide custom gauge adapters by specifying a path to a Python file, from which to load the adapter.

This way, we do not need to support all possible benchmark harnesses in ReBench, and can instead provide support when it's needed as a kind of plugin.

Previously, we could only use built-in parsers like this:

benchmark_suites:
  ExampleSuite:
    gauge_adapter: ReBenchLog

With this PR, a custom parser can be used like this:

benchmark_suites:
  ExampleSuite:
    gauge_adapter:
      MyClass: ./my_parser.py

Remaining Todos

  • expand the documentation to sketch how to provide a minimal custom implementation

@smarr smarr added the Feature label Mar 18, 2023
@coveralls
Copy link

coveralls commented Mar 18, 2023

Coverage Status

Coverage: 81.257% (+0.5%) from 80.721% when pulling 7ca4e62 on custom-gauge-adapters into 3914b3e on master.

@smarr smarr marked this pull request as ready for review March 19, 2023 22:06
@smarr
Copy link
Owner Author

smarr commented Mar 19, 2023

@OctaveLarose could you give this a spin locally to see whether you can use this to get your gauge adapter to work as desired?

You probably want absolute imports in the python file for the gauge adapter, which is something I ran into when creating the ones for testing.

@OctaveLarose
Copy link
Contributor

@OctaveLarose could you give this a spin locally to see whether you can use this to get your gauge adapter to work as desired?

You probably want absolute imports in the python file for the gauge adapter, which is something I ran into when creating the ones for testing.

Indeed, I had to switch to absolute imports. I can confirm it's functional! I would just suggest adding an explicit error message when the class can't be found (when _get_adapter_case_insensitively_from_module returns None) since I had originally forgotten to replace some MyClass entries with the name of my custom gauge adapter, and was confused why it was failing.

Also, if this gets merged, please make sure to update the rebench version on yuria so I can use it for my experiments

@smarr
Copy link
Owner Author

smarr commented Mar 20, 2023

The latest commit will make sure it fails in the same way as for a misnamed standard gauge adapter.

@smarr
Copy link
Owner Author

smarr commented Mar 20, 2023

And the last push adds a revised bit of documentation, with a minimal example on how to implement a custom adapter.

- also change the schema to permit for the new configuration

Signed-off-by: Stefan Marr <git@stefan-marr.de>
- the used schema language isn’t expressive enough to validate this

Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
@smarr
Copy link
Owner Author

smarr commented Mar 20, 2023

Some minor cleanups. If CI passes, I'll merge this and update branches.

Signed-off-by: Stefan Marr <git@stefan-marr.de>
@smarr smarr merged commit 453ffb0 into master Mar 20, 2023
14 checks passed
@smarr smarr deleted the custom-gauge-adapters branch March 20, 2023 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants