AirspeedVelocity.jl icon indicating copy to clipboard operation
AirspeedVelocity.jl copied to clipboard

feature request: measure CPU occupancy and memory usage

Open Krastanov opened this issue 2 years ago • 8 comments

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.

Krastanov avatar Apr 07 '23 14:04 Krastanov

It seems that memory is readily available from the benchmark results. Would it be easy to add a memory usage column?

abelsiqueira avatar Feb 19 '24 15:02 abelsiqueira

Good idea. Sure. Actually maybe not a column, but another table under it? We could have that table by default collapsed (with <details> ... </details>).

MilesCranmer avatar Feb 19 '24 16:02 MilesCranmer

Sorry I mean in the GitHub action. I guess the command line we could have another column available.

MilesCranmer avatar Feb 19 '24 16:02 MilesCranmer

I was also thinking of the github action. Another column could also work, or another row per column. Anything, really :)

abelsiqueira avatar Feb 19 '24 18:02 abelsiqueira

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?

MilesCranmer avatar Feb 19 '24 19:02 MilesCranmer

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.

MilesCranmer avatar Feb 19 '24 19:02 MilesCranmer

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.

abelsiqueira avatar Feb 21 '24 09:02 abelsiqueira

Both sound like good approaches to me; happy with either. We can iterate if need be.

MilesCranmer avatar Feb 21 '24 10:02 MilesCranmer