Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lower level api #52

Open
imposeren opened this issue May 16, 2016 · 8 comments
Open

lower level api #52

imposeren opened this issue May 16, 2016 · 8 comments

Comments

@imposeren
Copy link

I'm currently using your module to benchmark some external api performance. Currently this is implemented using severala coroutines running concurrent parallel taks where one coroutine calls some "processor" and this one processor should be benchmarked. using weave here is possible but problematic bacause of complex "setup" and concurrency issues. I have a workaround to solve my problem:

def some_test(benchmark, event_loop):
    benchmark._mode = 'benchmark(...)'
    stats = benchmark._make_stats(1)
    primary_task = event_loop.create_task(primary_task_factory(data_source, duration_callback=stats.update))
@asyncio.coroutine
def process_docs(
        docs_iterator, max_frequency, max_amount, processor,
        doc_index_in_args=1,
        duration_callback=None,
        *args, **kwargs):
    # ...
    p_t0 = default_timer()
    processor(*args, **kwargs)
    processor_time = default_timer()-p_t0
    total_processor_time += processor_time
    if duration_callback:
        duration_callback(processor_time)

It would be great to have some "legal" and documented way to do similar things. I had to read pytest-benchmark source code to do this. I think that public api for similar tasks should be exposed:

def test_something(benchmark):
    duration_handler = benchmark.manual()
    for x in something():
        foo()
        baz()

        duration_handler.start()
        some_processing_to_benchmark()
        duration_handler.end()

@ionelmc
Copy link
Owner

ionelmc commented May 16, 2016

How come you don't wrap the processor function instead? Does the processor have side-effects?

@imposeren
Copy link
Author

imposeren commented May 16, 2016

I'm sorry my example was incorrect it's more like:

@asyncio.coroutine
def process_docs(
        docs_iterator, max_frequency, max_amount, processor,
        doc_index_in_args=1,
        duration_callback=None,
        *args, **kwargs):
    for doc in docs_iterator:
        args[doc_index_in_args] = doc

        p_t0 = default_timer()
        processor(*args, **kwargs)
        processor_time = default_timer()-p_t0
        total_processor_time += processor_time
        if duration_callback:
            duration_callback(processor_time)
        for x in asyncio.sleep(wait_t):
            yield x

def test_something(benchmark, event_loop):
        benchmark._mode = 'benchmark(...)'
        stats = benchmark._make_stats(1)
        def done_callback(future):
            for task in unlimited_tasks:
                if not task.done():
                    task.cancel()
            event_loop.stop()

        limited_primary_task = event_loop.create_task(primary_task_factory(es_data, duration_callback=stats.update))
        # primary_task_factory will call `process_docs` internally
        limited_task.add_done_callback(done_callback)

        unlimited_tasks = []

        for i in range(SECONDARY_TASKS_CONCURRENCY):
            unlimited_tasks += [
                event_loop.create_task(secondary_task_factory(es_data))
                for secondary_task_factory in secondary_tasks_factories
            ]

        try:
            event_loop.run_forever()
        finally:
            event_loop.close()
            for task in [limited_task] + unlimited_tasks:
                if (not task.cancelled()) and task.exception():
                    raise task.exception()

Processor is invoked multiple times (see coroutine in my initial message): this will cause exception in benchmark when self._mode is checked.

Wrapping function with for loop is possible but that way i will get timing for whole run instead of single run and this for-loop is a generator with yield from asyncio.sleep() which also causes problems

Wrapping only a "processor" with benchmark seems to be hard because I also need to have "concurrent" secondary tasks to be run too but benchmark(processor) will run it outside of event loop

@ionelmc
Copy link
Owner

ionelmc commented May 16, 2016

I still think something is wrong there - you shouldn't have runs with different data aggregated in the same benchmark. What the point with this? Get avg time for the "docs building thing"? In that case min and deviation values are not useful (you would have no way to know how stable your benchmark was).

How fast is this process function? Is it less than 100ms?

My understanding is that you want to isolate the asyncio overhead (or whatever) and only get metrics for the "hot" part in your coroutines (the process func). However, that hot part is called multiple times during a test.

I think a better way would be to just isolate the process function so you only call it once in a test (and let pytest-benchmark repeat it as necessary).

@imposeren
Copy link
Author

I\m using pytest-benchmark for a kind of unrelated purpose: I'm testing API performance that is a part of our infrastructure. This is a ElasticSearch API and i'm benchmarking search using current index configuration and queries structure. ES performance have high dependency on data diversity and also depends on concurrent queries. So I need to

  1. benchmark 500 runs of search
  2. during this benchmark other queries should also be performed to simulate situation closer to reality.

That's why I can't simply let pytest-benchmark to rerun call 500 times: I can't run parallel concurrent tasks with controlled frequency using current api. I have a workaround:

        benchmark._mode = 'benchmark(...)'
        stats = benchmark._make_stats(1)

        # ...
        prev_t = default_timer()
        benchmarked_function_call()
        processor_time = default_timer()-prev_t
        stats.update(main_call_duration)

        # check concurrent queries frequency
        # run concurrent tasks zero or several times to reach target frequency

But this is not documented anywhere and I think that there may also be other possible usecases of pytest-benchmark where such low level api will be useful

@ionelmc
Copy link
Owner

ionelmc commented Jun 3, 2016

I need to read your comment a bit more but have you looked at the manual mode? http://pytest-benchmark.readthedocs.io/en/latest/pedantic.html

@imposeren
Copy link
Author

Maybe my situation can be solved by using pedantic(..., setup=foo, ... ) It still have some limitations that require workarounds but still it's better because I don't need to call or to know any internals of pytest-benchmark. Thanks

@adiroiban
Copy link

Hi guys and thanks for your work on this project.

Just adding some feedback that might be related to this issue.

I/we are looking to benchmark the SSH handshake process.
In order to test this we need to start and stop a SSH server and SSH client.

We have some tests that need extra setup/teardown before each run.

The pedantic mode provides a setup, but I can't see the teardown callback.

The current code that we use is here:

https://github.com/twisted/twisted/pull/12220/files#diff-617e7b5d7d22473c5fc42a59977e49960b29c5ec89407732ba2d2ea3857af0a7R986


so maybe the pedantic mode will work with the addition of a teardown functionality.

At the same time pedantic mode comes without automatic callibration.

I don't need to pass different arguments to the system under test , just setup and teardown code that is excluded from the benchmark counters

I hope it makes sense

@ionelmc
Copy link
Owner

ionelmc commented Oct 30, 2024

Ref #264 - that should fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants