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

⚡️ Speed up function collect_func_params by 10% #1128

Conversation

misrasaurabh1
Copy link
Contributor

Saurabh's comment - Does make it less readable, but it looks like a really important function to speed up, since it will speed up every inference call, by reducing the overhead of usage collection. I would also recommend you to double check the optimization here.

📄 392% (3.92x) speedup for collect_func_params in inference/usage_tracking/utils.py

⏱️ Runtime : 525 microseconds 107 microseconds (best of 234 runs)

📝 Explanation and details

Here's the optimized version of your code.

Changes and Optimizations.

  1. Early Merging: We use dictionary operations more efficiently by merging args and kwargs earlier.

Correctness verification report:

Test Status
⚙️ Existing Unit Tests 🔘 None Found
🌀 Generated Regression Tests 33 Passed
⏪ Replay Tests 🔘 None Found
🔎 Concolic Coverage Tests 🔘 None Found
📊 Tests Coverage 89.5%
🌀 Generated Regression Tests Details
import inspect
from typing import Any, Callable, Dict, Iterable

# imports
import pytest  # used for our unit tests
from inference.usage_tracking.utils import collect_func_params


# mock logger to avoid actual logging during tests
class MockLogger:
    def error(self, msg, *args):
        pass

logger = MockLogger()
from inference.usage_tracking.utils import collect_func_params


# unit tests
def test_basic_positional_args():
    def func(a): pass
    codeflash_output = collect_func_params(func, (1,), {})

    def func(a, b, c): pass
    codeflash_output = collect_func_params(func, (1, 2, 3), {})

def test_basic_keyword_args():
    def func(a): pass
    codeflash_output = collect_func_params(func, (), {'a': 1})

    def func(a, b, c): pass
    codeflash_output = collect_func_params(func, (), {'a': 1, 'b': 2, 'c': 3})

def test_mixed_args():
    def func(a, b, c): pass
    codeflash_output = collect_func_params(func, (1,), {'b': 2, 'c': 3})
    codeflash_output = collect_func_params(func, (1, 2), {'c': 3})

def test_default_values():
    def func(a, b=2, c=3): pass
    codeflash_output = collect_func_params(func, (1,), {})
    codeflash_output = collect_func_params(func, (1,), {'b': 4})

def test_args_kwargs():
    def func(a, *args): pass
    codeflash_output = collect_func_params(func, (1, 2, 3), {})

    def func(a, **kwargs): pass
    codeflash_output = collect_func_params(func, (1,), {'b': 2, 'c': 3})

def test_no_parameters():
    def func(): pass
    codeflash_output = collect_func_params(func, (), {})



def test_complex_signatures():
    def func(a, /, b, *, c, **kwargs): pass
    codeflash_output = collect_func_params(func, (1, 2), {'c': 3, 'd': 4})

def test_large_number_of_parameters():
    def func(*args): pass
    large_args = tuple(range(1000))
    codeflash_output = collect_func_params(func, large_args, {})

def test_annotations():
    def func(a: int, b: str): pass
    codeflash_output = collect_func_params(func, (1,), {'b': 'test'})

def test_variable_length_args():
    def func(*args): pass
    codeflash_output = collect_func_params(func, (1, 2, 3), {})

def test_keyword_only_args():
    def func(*, a, b): pass
    codeflash_output = collect_func_params(func, (), {'a': 1, 'b': 2})
# codeflash_output is used to check that the output of the original code is the same as that of the optimized code.

import inspect
from typing import Any, Callable, Dict, Iterable

# imports
import pytest  # used for our unit tests
from inference.core.logger import logger
from inference.usage_tracking.utils import collect_func_params


# unit tests
def test_basic_positional_args():
    # Function with no default parameters
    def func(a, b, c): pass
    codeflash_output = collect_func_params(func, (1, 2, 3), {})

    # Function with some default parameters
    def func(a, b=2, c=3): pass
    codeflash_output = collect_func_params(func, (1,), {})

def test_basic_keyword_args():
    # Function with no default parameters
    def func(a, b, c): pass
    codeflash_output = collect_func_params(func, (), {'a': 1, 'b': 2, 'c': 3})

    # Function with some default parameters
    def func(a, b=2, c=3): pass
    codeflash_output = collect_func_params(func, (), {'a': 1})

def test_basic_mixed_args():
    # Function with no default parameters
    def func(a, b, c): pass
    codeflash_output = collect_func_params(func, (1,), {'b': 2, 'c': 3})

    # Function with some default parameters
    def func(a, b=2, c=3): pass
    codeflash_output = collect_func_params(func, (1,), {'b': 2})

def test_edge_no_args():
    # Function with no parameters
    def func(): pass
    codeflash_output = collect_func_params(func, (), {})

    # Function with all parameters having default values
    def func(a=1, b=2): pass
    codeflash_output = collect_func_params(func, (), {})





def test_special_variadic_params():
    # Function with *args
    def func(*args): pass
    codeflash_output = collect_func_params(func, (1, 2, 3), {})

    # Function with **kwargs
    def func(**kwargs): pass
    codeflash_output = collect_func_params(func, (), {'a': 1, 'b': 2})

def test_special_combination_variadic_and_regular_params():
    # Function with regular parameters followed by *args
    def func(a, *args): pass
    codeflash_output = collect_func_params(func, (1, 2, 3), {})

    # Function with regular parameters followed by **kwargs
    def func(a, **kwargs): pass
    codeflash_output = collect_func_params(func, (1,), {'b': 2, 'c': 3})

def test_default_values_handling():
    # Function with all parameters having default values
    def func(a=1, b=2, c=3): pass
    codeflash_output = collect_func_params(func, (), {})

    # Function with some parameters having default values
    def func(a, b=2, c=3): pass
    codeflash_output = collect_func_params(func, (1,), {})

    # Function with no default values
    def func(a, b, c): pass
    codeflash_output = collect_func_params(func, (1, 2, 3), {})



def test_large_scale_high_number_of_arguments():
    # Function with a large number of parameters
    def func(*args): pass
    codeflash_output = collect_func_params(func, tuple(range(100)), {})

    # Function with a large number of arguments provided in args and kwargs
    def func(**kwargs): pass
    codeflash_output = collect_func_params(func, (), {f'a{i}': i for i in range(100)})
# codeflash_output is used to check that the output of the original code is the same as that of the optimized code.

Codeflash

Here's the optimized version of your code.

### Changes and Optimizations.

2. **Early Merging**: We use dictionary operations more efficiently by merging `args` and `kwargs` earlier.
@PawelPeczek-Roboflow
Copy link
Collaborator

@grzegorz-roboflow I would not touch that, since this code is not on the critical execution path and changes may break usage - but leaving up to your judgement

@misrasaurabh1
Copy link
Contributor Author

I might be wrong but I suspect that this code is being run with every inference UsageCollection through a call at

@grzegorz-roboflow
Copy link
Contributor

I am going to review this since the change is quite substantial refactor, had other priorities and this is on my TODO list

@grzegorz-roboflow
Copy link
Contributor

Hi @misrasaurabh1 , I added a few tests in #1139, applied your branch locally, below tests are failing:

  • test_collect_func_params_with_args_and_catchall_kwargs
  • test_collect_func_params_with_only_kwargs
  • test_collect_func_params_with_only_kwargs_and_errorneous_args_passed

Lets not change behavior of this util

@misrasaurabh1 misrasaurabh1 changed the title ⚡️ Speed up function collect_func_params by 392% ⚡️ Speed up function collect_func_params by 10% Apr 3, 2025
@misrasaurabh1
Copy link
Contributor Author

Thank you for your new test cases. I used those + the test cases attached here + created 25 more. The latest version is 10% faster than the original and is more easily verifiable.

@grzegorz-roboflow
Copy link
Contributor

Thanks @misrasaurabh1 !!

I guess the initial idea was to remove call to inspect? If so, I propose this change, please have a look

@grzegorz-roboflow
Copy link
Contributor

Merged changes from main to this branch

@PawelPeczek-Roboflow PawelPeczek-Roboflow merged commit e47888a into roboflow:main Apr 4, 2025
23 of 27 checks passed
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

Successfully merging this pull request may close these issues.

3 participants