AirspeedVelocity.jl
AirspeedVelocity.jl copied to clipboard
feature request: measure CPU occupancy and memory usage
On a UNIX machine the time command can be used to see how many CPU seconds were used, what was the wall time, and how much peak memory was used. Some standardized access to this would be nice.
It seems that memory is readily available from the benchmark results. Would it be easy to add a memory usage column?
Good idea. Sure. Actually maybe not a column, but another table under it? We could have that table by default collapsed (with <details> ... </details>).
Sorry I mean in the GitHub action. I guess the command line we could have another column available.
I was also thinking of the github action. Another column could also work, or another row per column. Anything, really :)
In the benchpkgtable command you could try to add a --cols that reports this sort of stuff maybe?
The current CLI usage is as follows:
Usage
benchpkgtable <args> [options] [flags]
Args
<package_name> Name of the package.
Options
-r, --rev <arg> Revisions to test (delimit by comma).
-i, --input-dir <arg> Where the JSON results were saved (default: ".").
Flags
--ratio Whether to include the ratio (default: false). Only applies
when comparing two revisions.
-h, --help Print this help message.
--version Print version.
Instead of --ratio we could have a flag like --cols=ratio,memory?
Or actually since they are completely separate tables maybe we just need a --memory flag, and have each cell report both the times and memory, then the --ratio would give both ratios of time and memory.
Ok, I looked at the code, and it appears that the simplest change would be around _flatten_results!. Instead of computing summary statistics of the results["times"], we could instead send the full results to compute_summary_statistics and then add a key "memory", and possibly "allocs" to the OrderedDict. This doesn't change the keys on the existing dictionary, so - I think - it is non-breaking.
Alternatively, we could change the existing key to be prefix * "/times" and create a separate conditional inside _flatten_results! to define prefix * "/memory" and prefix * "/allocs".
More radically, we could make changes at a higher level, but I don't see the benefits at the moment.
Let me know what you think.
Both sound like good approaches to me; happy with either. We can iterate if need be.