[v2] Add support for chunking performance tests, and architectural changes #9485
Conversation
| - `mode` (string) **(optional)**: The write mode to use for writing the | ||
| file contents. | ||
| - Default: `w` |
There was a problem hiding this comment.
Is anything actually using this? It looks hardcoded below in _create_file_with_size and begin_iteration.
There was a problem hiding this comment.
Ah, it's intended to be used in the JSONStubbedBenchmarkSuite. In particular, it was useful for one-off performance testing of CBOR protocol for an operation with binary input. Will update in next revision
| if ( | ||
| sum( | ||
| x is not None | ||
| for x in [parsed_args.num_chunks, parsed_args.chunk_id] | ||
| ) | ||
| == 1 | ||
| ): |
There was a problem hiding this comment.
Is this pattern common? For readability summing bools seems odd.
| @@ -0,0 +1,24 @@ | |||
| class BaseBenchmarkSuite: | |||
There was a problem hiding this comment.
Do we need a base class if there's only one implementation of it?
If keeping, could you add class-level comments when when someone would use this?
There was a problem hiding this comment.
I like having this base class so that it forces us to implement our only test suite via a contract that we know is extendable to other use cases. I previously extended this base class to implement CBOR benchmarks for a couple other services, and those tests fit nicely against the functions on this base class.
Even though we won't ship those CBOR tests, they served to show locally that the function contract on this class is extendible/reusable.
I will add those class-level comments in the next iteration
Context:
The main purpose of this PR is adding 'chunking' support to performance tests to enable sharding of the tests across parallel workers in a distributed workflow. The secondary purpose of this PR is to refactor the architecture of the performance testing code to make it easier to integrate more complex performance tests. The architectural decisions were led by implementing more complex RPCv2 CBOR performance test cases and observing points of friction.
Description of changes:
BaseBenchmarkSuiteclass which defines the abstract functions that must be implemented to define new performance tests. The default performance test cases were abstracted out into aJSONStubbedBenchmarkSuiteimplementation.JSONStubbedBenchmarkSuitedimensionsformat to require the usage ofnameandvaluekeys to reflect the CloudWatch Metrics model.modekey onfile_literalsdefinition.BenchmarkResultsSerializerclass. This makes it easier to override this class for one-off performance testing when a different output format is expected.Summarizerto return metric results in a newMetricdata model, which includes additional data like description and unit.Description of tests:
./scripts/performance/run-benchmarks --num-iterations 20and observed expected results.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.