psf / pyperf

Toolkit to run Python benchmarks
http://pyperf.readthedocs.io/
MIT License
799 stars 78 forks source link

pyperf is incompatible with click #65

Open pganssle opened 4 years ago

pganssle commented 4 years ago

When a script calls pyperf.Runner.bench_func from a script that uses click to handle arguments, it raises an exception in pyperf's argument parsing logic. Here is a minimal reproducing script:

import click
import pyperf

@click.command()
def cli():
    def f():
        pass

    runner = pyperf.Runner()

    runner.bench_func("Example", f)

if __name__ == "__main__":
    cli()

Here's a requirements.txt, pinned to the versions I have tried this with:

click==7.1.1
pyperf==2.0.0

The error message is:

$ python my_benchmark.py 
Usage: my_benchmark.py [OPTIONS]
Try 'my_benchmark.py --help' for help.

Error: no such option: --worker

I believe that the issue is that pyperf.Runner is directly using argparse to add its own command line arguments, which is not really the behavior I would expect from a library.

It might be a lot to ask, but I think a better option would be to refactor this into a config object that can also be constructed automatically from the parser, something like this:

import attr

@attr.s(auto_attrib=True)
class RunnerConfig:
    verbose: bool = False
    quiet: bool = False
    pipe: int = None
    ...

    @classmethod
    def from_argparse(cls, argparser=None):
        if argparser is None:
            argparser = argparse.ArgumentParser()
        parser.description = "Benchmark"
        ....
        args = argparser.parse_args()
        return cls(verbose=args.verbose, quiet=args.quiet, pipe=args.pipe, ...)

To avoid backwards incompatibility issues, you can add a flag to Runner like use_argparse=True, which users of click could set to False to avoid this problem.

vstinner commented 4 years ago

Can't you avoid importing the code which uses click in your benchmark? Or maybe mock sys.argv when it's imported. Why does your code parse command line arguments even if it's not the main module?

pganssle commented 4 years ago

Why does your code parse command line arguments even if it's not the main module?

@vstinner I don't understand, my code is the __main__ module, it's pyperf that is parsing arguments without being __main__. A simplified version of my code looks more like this:

import click
import pyperf

import module_to_benchmark

def function_factory(x, y):
    def some_function(x=x, y=y):
        module_to_benchmark.function_to_benchmark(x, y)

    return some_function

@click.cli()
@click.option("-x")
@click.option("-y")
def main(x, y):
    func = function_factory(x, y)
    runner = pyperf.Runner()
    runner.bench_func(f"Benchmarking function with ({x}, {y})", func)

Because I'm passing arbitrary Python objects, it would require a very different approach to refactor this using subprocess calls.

I think it would be preferable if pyperf only used argparse when invoked as a module or script. The click incompatibility is just one issue - I find this behavior very surprising anyway, since I do not expect methods on a class to cause my script to start accepting command line arguments unless I have explicitly enabled that.

vstinner commented 4 years ago

pyperf spawns multiple processes and it uses the command line to pass arguments to worker processes: https://pyperf.readthedocs.io/en/latest/run_benchmark.html#pyperf-architecture

You should avoid using click and pyperf in the same file.

pganssle commented 4 years ago

You should avoid using click and pyperf in the same file.

Presumably also you should avoid using argparse and pyperf in the same file, because you'll have the same issue if pyperf has any options that conflict with the options from the orchestration script.

I think I don't really understand the point of pyperf.Runner.bench_func if it's not to do stuff like this.

I suppose for my purposes it will be good enough to go back to using timeit.

vstinner commented 4 years ago

Can't you move your benchmark code into a separated script, and import your functions in the benchmark script? Something like that: https://pyperf.readthedocs.io/en/latest/examples.html#bench-func-method

Maybe pyperf should have an option to not spawn worker processes, but run all benchmarks in the same process.

I suppose for my purposes it will be good enough to go back to using timeit.

Most timeit users run the benchmark an arbitrary number of times (usually 3 to 5 times) and then pick their favorite result, like the minimum.

pyperf does it for you, but tries to be smarter: store all values, ignore the first "warmup" value, and display the mean with the standard deviation, rather than just the minimum: https://pyperf.readthedocs.io/en/latest/cli.html#timeit-versus-pyperf-timeit