Allow overriding benchmark parameters #262
Merged
Conversation
This file contains 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
This is just avoid mixing those with the next batch.
The `BenchmarkSuite` class provides the harness with an interface that allows it to (mostly) forget about the `BenchmarkRegistry` and `ModuleLoader` classes. Both could (and should) be made package private if it weren't for the `MarkdownWriter` class, which should really be part of the build system, not the harness. The `BenchmarkInfo` class has been replaced with `BenchmarkDescriptor` which is a bit lightweight in the sense that it does not try to pull data from `benchmark.properties` into instance variables and instead uses the underlying map of the properties whenever we try to get some information about a benchmark. The `BenchmarkSuite` class provides some services that do not belong to the `BenchmarkDescriptor` class even though, because they would make the descriptor into more than it should be. It also collected some of the responsibilities that were either duplicated (benchmark context creation) or difficult to place elsewhere and were moving between `ModuleLoader` and `BenchmarkInfo` (creating benchmark and extension instances). In the end, this allowed to simplify other classes such as the `ExecutionDriver`, `RenaissanceSuite` and `JmhRenaissanceBenchmark`. In addition, because `BenchmarkSuite` provides the `ExecutionDriver` with an instances of `InternalBenchmarkContext`, the `ExecutionDriver` can now print the configuration name that depends on whether there are any overridden benchmark parameters.
The `ModuleLoader` class is now only used within the core package. We cannot make the whole class package-private yet, because the `MarkdownWriter` wants to get the class name.
The `thisJvmVersion()` method is now a convenience method provided by the `BenchmarkSuite` class -- here it was just pulling a dependency on `ManagementFactory` into what was meant to be a simple value class. On the other hand, the `compareTo(Optional<Version>)` comes handy and there should be no reason to duplicate it in other classes.
The code now uses the `BenchmarkSuite` class so that it does not need to initialize and use the `BenchmarkRegistry` and `ModuleLoader` on its own. This allowed us to get rid of the context creation and benchmark compatibility checking, which is now provided by the `BenchmarkSuite` and shared with `RenaissanceSuite`.
The `ExecutionDriver` class only kept part of the execution context in instance variables and required `RenaissanceSuite` to pass a lot of it to `executeBenchmark()` method. Because none of the information changes, the `ExecutionDriver` class is now constructed with all that information and the `executeBenchmark()` only serves to trigger the benchmark execution. Similarly, the (internal) `executeOperation()` methods also uses most of the captured context and is only passed the index of the operation because that's the only thing that changes. The `ExecutionDriver` no longer doubles as a `BenchmarkContext`. The `SuiteBenchmarkContext` created by the `BencharkSuite` is now used in that role, but the internal type allows querying the actual name of the configuration which will reflect the fact that some of its parameters were overridden.
The `RenaissanceSuite` code does not have to deal with the `BenchmarkRegistry` and `ModuleLoader` classes anymore and instead uses more generic services provided by the `BenchmarkSuite` class which can be also shared with `JmhRenaissanceBenchmark`.
This ensures that configurations defined using the `@Configuration` annotation actually do exist in the `benchmarks.properties` file. This allows us to complain if someone tries to use a non-existent configuration.
|
Interesting, how such an innocent feature can amplify the lack of proper abstractions... Who's up for round 2? |
This allow to remove the `forEachPrimaryGroup()` methods from the `BenchmarkRegistry` class.
The `README.md` was updated to list the dummy benchmarks separately from the main benchmarks. The `CONTRIBUTION.md` was renamed to match where GitHub expects to find contribution guide.
|
BTW, travis-ci.org is now officially gone and the first reactions to travis-ci.com (from free users) seem to be... underwhelming... |
farquet
approved these changes
Jun 16, 2021
renaissance-harness/src/main/java/org/renaissance/harness/ExecutionDriver.java
Show resolved
Hide resolved
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.
Adds
-oor--overrideoption accepting a<name>=<value>pair. The override is applied to the selected configuration and benchmarks. An alternative would be to allow<name>to be a full name (or a regexp matching the name) of a property found in thebenchmarks.propertiesfile.At runtime, if the selected benchmark configuration has an overridden parameter, the suite prints the parameter together with the old and new value before running the benchmark and shows the displayed configuration name with a
modified-prefix.Interestingly, finishing the implementation after the initial proof-of-concept required a bit of refactoring which also enabled consolidation of some of the services provided by the suite core classes into the
BenchmarkSuiteclass, avoiding duplication of some of the functionality in theRenaissanceSuiteandJmhRenaissanceBenchmark. More details can be found in the commit messages.