JoeyHendricks / python-benchmark-harness

A micro/macro benchmark framework for the Python programming language that helps with optimizing your software.
MIT License
155 stars 13 forks source link

strange behaviour #5

Closed larrieu-olivier closed 3 years ago

larrieu-olivier commented 3 years ago

Hi i ve just discovered QuickPotato and for the first use i ve encountered strange behaviour.

this the very simple test i ve used image

first point On the method pt.measure_method_performance there is not named_arguments param, i looked in the source code and it figure than named parameters are not considered.

image

second point data.average_response_time raise a ZeroDivisionError because data._response_times is an empty list.

best regards

JoeyHendricks commented 3 years ago

Thank you for opening up the first-ever issue on this repository I appreciate that a lot so thanks a lot! I reviewed your first point and indeed there is something wonky happening there with the collection of the performance statistics.

Luckily I found the problem this is what is happening:

It all has to do with the async - payload delivery that automatically goes on if your Python version is above 3.8.2. After your code is profiled QuickPotato will use async to reduce the time of your tests (reduce the overhead) by using a "fire and forget" process and hoping that once your test is finished all the database inserts are done and the computation can take place.

On longer-running tests, this is usually the case but since your code is blazingly fast which basically reduces your test run time to almost zero. QuickPotato sometimes is still busy with transferring the data to the local SQLite database when you are trying to read the same data it is currently processing.

So what I am going to do about this:

I am going to disable async delivery by default and be a bit smarter when I will allow somebody to enable it. A bit of context why this "fire and forget" system even exists:

"async is handy when you are running time-consuming performance tests on more complicated logic and you want to reduce the test duration or you want to run QuickPotato on a method in production using the intrusive decorator and you want to reduce the overhead a bit for the unlucky end-user who's method call is being profiled ."

BTW The boundaries object can be loaded with a dictionary this makes it easier to push your performance requirements into your test by just "yeeting" your dictionary into the object. (just make sure the keys match)

Below your code but now working because I disabled the async:

from QuickPotato.profiling.intrusive import performance_test as pt  # <-- I don't like this path so I will change it
from QuickPotato.configuration.management import options

def hello(name, age=0):
    return f"hello {name} with age {age}"

def test_performance__info_dataset():
    options.enable_the_selection_of_untested_or_failed_test_ids = True
    options.enable_asynchronous_payload_delivery = False  # <-- the bug that is actually a feature :)

    pt.test_case_name = "test_performance: info_data_sets"
    pt.max_and_min_boundary_for_average = {"max": 10, "min": 0.001}
    pt.measure_method_performance(
        method=hello,
        arguments=['oliver'],
    )
    # I like this! will add it to my boundary object
    assert pt.benchmark_measurements.average_response_time() < 0.005

test_performance__info_dataset()

Also, settings are sticky in QuickPotato changing them once will change them forever until you change them back by using the options object or changing them in the YAML file manually.

On your second point, yes you are absolutely right I am at this point not considering named parameters. This can be a bit annoying if you have a large number of parameters but I am going to add this to "pt.measure_method_performance()" method call in the next release to version 0.4 I am also going to introduce more threading capabilities and test start-up and shutdown policies so more complicated performance tests can be defined.

Can you let me know if this workaround is functioning on your side?

larrieu-olivier commented 3 years ago

Thank you @JoeyHendricks

i appreciate your explanations, i ll giv a try later and will let you know the results.

regards

JoeyHendricks commented 3 years ago

I have updated the project to reflect the changes let me know if you have any further questions or feedback.