[benchmark] Measure memory with rusage and a TON of gardening #18124

palimondo · 2018-07-21T00:06:07Z

This PR adds a feature to measure and report system environment indicators during benchmark execution:

Memory usage with maximum resident set size (MAX_RSS) in bytes

Proxy indicators of system load level:

Number of Involuntary Context Switches (ICS)
Number of Voluntary Context Switches (VCS)

MAX_RSS delta is reported in the last column of the log report, when enabled using the --memory flag.

The --verbose mode additionaly reports full values measured before and after the benchmark execution as well as their delta for MAX_RSS, ICS and VCS.

Benchmark_Driver already had the MAX_RSS reporting implemented on top of time -lp utility. This direct implementation using rusage in the Benchmark_O improves upon it by reporting the delta of the MAX_RSS before and after the benchmark execution, effectively excluding any overhead caused by the measurement infrastructure. Same is true for the system load indicators. This significantly reduces the variability in the reported values. A follow-up PR will switch the Benchmark_Driver to also use this implementation.

The PR also introduces thorough test coverage of the Benchmark_O's public interface (command line arguments and log format). The lit tests were converted into markdown format, to ease their self-documentation (literate testing), resulting in executable specification of the API. See Benchmark_O.test.md.

The DriverUtils.swift had a lot of accumulated technical debt, which required a ton of gardening to clean up. First a test coverage of a feature is established, followed by functionally neutral refactoring (labeled [Gardening] in the commit message). After the clean up, the new features were added on top of the improved code organization.

Major refactoring of the command line argument parsing has improved their error handling. The previously solid argument syntax parsing (parseArgs) is joined inside the ArgsParse.swift with type checking of arguments (previously done ad-hoc, per-argument in DriverUtils.swift) to form ArgumentParser class which handles the complete job of parsing command line arguments. It supports declarative configuration of the parsing, modeled after the interface of python's argparse module with the flexible addArgument method.

Program's API has remained largely unchanged, with the small exception of removed bogus Totals stats (c7fc745). Program's human facing interface was majorly improved to provide full documentation of all arguments with the --help command.

I've structured the PR as a series of small, incremental changes with detailed descriptions in the commit messages, it is therefore best reviewed sequentially by individual commits.

The testing of `Benchmark_O` and its public interface needs more documentation. The `lit` tests can be embedded in anything. Lets combine these.

Reformatting the `lit` test file to markdown format: * Removed the unnecessary `// `prefix. * Requires preambule in comment * Test documentation intro

* Added check for running by test number. * Documented “dry run” using `--list`. * Moved real run test to the end. * Added checks for logging benchmarks measurements (header and stats). * Added check that specifying the same test multiple times runs it just once.

Report only the total number of executed tests. Aggregating MIN, MAX and MEAN values for all executed benchmarks together (with microsecond precision!) has no statistical relevance.

Moved the formatting of`BenchmarkResults` into `runBenchmarks` function which already contained the logging of header and the special case of unsupported benchmark.

Test the `--verbose` mode output and the `--num-samples` option.

The `Test`struct was forwarding everything to `BenchmarkInfo` and its only contribution was carrying of the index. Tuple is fine for that.

The indices (test numbers) are strings on both ends of the IO — in user input as well as when printed to the console. We can spare few conversions and just store them directly as `String`.

Measure and report system environment indicators during benchmark execution: * Memory usage with maximum resident set size (MAX_RSS) in bytes Proxy indicators of system load level: * Number of Involuntary Context Switches (ICS) * Number of Voluntary Context Switches (VCS) MAX_RSS delta is always reported in the last column of the log report. The `--verbose` mode additionaly reports full values measured before and after the benchmark execution as well as their delta for MAX_RSS, ICS and VCS.

* Fix: flushing stdout before crashing to enable testing. * Added tests that verify reporting of errors when the parsing of command line arguments fails.

Refactored to use Swift’s idiomatic error handling. In case of invalid argument errors, the message is printed to `stderr` and we exit gracefully with error code 1. We no longer crash the app in most cases.

We no longer crash when the argument value parsing fails, but report an error.

Added handling for arguments that don’t have a value (flags), or whose values are optional (they can be empty). This covers all the argument types currently used in Benchmark_O with the single `optionalArg` function.

* Extracted tag parser * Reordered all parsing of optional arguments to be grouped together.

Processing command line arguments is an integral part of `TestConfig` initialization. Make it explicit.

Moving towards immutable TestConfig. The pattern used is inspired by the`PartialBenchmarkConfig` from [criterion.rs](https://github.com/japaric/criterion.rs/blob/master/src/benchmark.rs). The idea is to have a temporary mutable struct which collects the command line arguments and is filled by the “parser” (which will be extracted later). The partial configuration is used to initialize the immutable `TestConfig`. The optionality of the command line parameters is represented by the corresponding properties being `Optional`. (Accidentaly, all our arguments are optional… but that’s beside the point.) Null coalescing operator is used to fill in the defaults during initialization. For some reason, the compiler found `optionalArg` calls for `tags` and `skip-tags` ambiguous, when types changed to `Set<BenchmarkCategory>?`, which was resolved by providing fully qualified key paths for them (`\PartialTestConfig.tags`).

The `TestConfig` is now completely immutable. Arguments that are not necessary for running benchmarks were moved inside the config initialization, which also subsumed the `printRunInfo` function for displaying the configuration details in verbose mode.

Refactored `filterTests` to use two nested functions that better reveal the underlying logic.

Renamed `optionalArg` to `parseArg` because it can now also handle the positional arguments. Now all the command line arguments are handled through this function. Minor naming improvement of the `filterTests` parameter.

Moved the argument parsing logic into new class `ArgumentParser`. The `checked` function is also moved to the ArgParse.swift, next to the parser.

The `ArgumentParser` now has a configuration phase which specifies the supported arguments and their handling. The configured parser is then executed using the `parse` method which returns the parsed result.

Moved the printing of help message inside the `ArgumentParser`, which has all the necessary info. Added test that checks the `--help` option.

In case of invalid command line arguments, there is no reasonable recovery, the `ArgumentParser` can exit the program itself. It is therefore no longer necessary to propagate the `ArgumentError`s outside of the parser.

The `parseArgs` funtion is now a private method on `ArgumentParser`. Removed `Arguments` struct and moved the `Argument` as a nested struct into the parser. Adjusted error messages and the corresponding checks.

The `--help` option now prints standard usage description with documentaion for all arguments: ```` $ Benchmark_O --help usage: Benchmark_O [--argument=VALUE] [TEST [TEST ...]] positional arguments: TEST name or number of the benchmark to measure optional arguments: --help show this help message and exit --num-samples number of samples to take per benchmark; default: 1 --num-iters number of iterations averaged in the sample; default: auto-scaled to measure for 1 second --iter-scale number of seconds used for num-iters calculation default: 1 --verbose increase output verbosity --delim value delimiter used for log output; default: , --tags run tests matching all the specified categories --skip-tags don't run tests matching any of the specified categories; default: unstable,skip --sleep number of seconds to sleep after benchmarking --list don't run the tests, just log the list of test numbers, names and tags (respects specified filters) ````

Printing of the MAX_RSS is now hidden behind the optional `--memory` flag.

palimondo · 2018-07-22T05:08:35Z

@eeckstein Please review 🙏

eeckstein

Basically LGTM.
I have some (mostly stylistic) comments about some minor issues.

eeckstein · 2018-07-23T00:41:02Z