Skip to content

Conversation

@misrasaurabh1
Copy link
Contributor

Change Summary

📄 unwrap_wrapped_function() in pydantic/_internal/_decorators.py

📈 Performance improved by 93% (0.93x faster)

⏱️ Runtime went down from 69.2 microseconds to 35.8 microseconds

Explanation and details

To optimize the given Python program for faster runtime, we can make several enhancements. Primarily, we make use of Python's tuple for type checks and avoid unnecessary set operations by consolidating them into a single set operation. Here's the optimized version.

Key Optimizations.

  1. Tuple Consolidation for isinstance:

    • Instead of building a set and updating it conditionally, we use a tuple that is directly used in isinstance checks.
  2. Simplified isinstance Checks:

    • We handle type checks and reassignments more efficiently without repeated checks or updates to the set, streamlining the process.

These changes collectively reduce the complexity and improve the performance by minimizing unnecessary operations and making the control flow straightforward.

Correctness verification

The new optimized code was tested for correctness. The results are listed below.

🔘 (none found) − ⚙️ Existing Unit Tests

✅ 12 Passed − 🌀 Generated Regression Tests

(click to show generated tests)
# imports
from functools import cached_property, partial, partialmethod, wraps
from typing import Any

import pytest  # used for our unit tests
from pydantic._internal._decorators import unwrap_wrapped_function

# unit tests

def test_simple_function():
    # Test a simple undecorated function
    def simple_function():
        pass
    assert unwrap_wrapped_function(simple_function) is simple_function

def test_property():
    # Test a class with a property
    class MyClass:
        @property
        def my_property(self):
            return 42
    assert unwrap_wrapped_function(MyClass.my_property) is MyClass.my_property.fget

def test_cached_property():
    # Test a class with a cached property
    class MyClass:
        @cached_property
        def my_cached_property(self):
            return 42
    assert unwrap_wrapped_function(MyClass.my_cached_property) is MyClass.my_cached_property.func

def test_partial_function():
    # Test a function wrapped with functools.partial
    def original_function(a, b):
        return a + b
    partial_function = partial(original_function, 1)
    assert unwrap_wrapped_function(partial_function) is original_function

def test_partial_method():
    # Test a class with a method wrapped with functools.partialmethod
    class MyClass:
        def original_method(self, a, b):
            return a + b
        partial_method = partialmethod(original_method, 1)
    assert unwrap_wrapped_function(MyClass.partial_method) is MyClass.original_method

def test_class_method():
    # Test a class with a class method
    class MyClass:
        @classmethod
        def my_class_method(cls):
            return 42
    assert unwrap_wrapped_function(MyClass.my_class_method) is MyClass.my_class_method.__func__

def test_static_method():
    # Test a class with a static method
    class MyClass:
        @staticmethod
        def my_static_method():
            return 42
    assert unwrap_wrapped_function(MyClass.my_static_method) is MyClass.my_static_method.__func__

def test_nested_decorators():
    # Test a function with multiple decorators
    def decorator(func):
        @wraps(func)
        def wrapper(*args, **kwargs):
            return func(*args, **kwargs)
        return wrapper

    @decorator
    @partial
    def nested_function(a, b):
        return a + b

    assert unwrap_wrapped_function(nested_function) is nested_function.__wrapped__

def test_combined_decorators():
    # Test a class with methods that have combined decorators
    class MyClass:
        @classmethod
        @staticmethod
        def combined_method(cls):
            return 42
        partial_combined_method = partialmethod(combined_method, 1)
    assert unwrap_wrapped_function(MyClass.combined_method) is MyClass.combined_method.__func__
    assert unwrap_wrapped_function(MyClass.partial_combined_method) is MyClass.combined_method

def test_non_callable():
    # Test passing a non-callable object
    non_callable = 42
    assert unwrap_wrapped_function(non_callable) is non_callable

def test_unwrapped_function():
    # Test a function that is already unwrapped
    def already_unwrapped():
        pass
    assert unwrap_wrapped_function(already_unwrapped) is already_unwrapped

def test_large_scale():
    # Test performance with a deeply nested function wrapped with many partial decorators
    def deeply_nested_function(a, b):
        return a + b
    for _ in range(1000):
        deeply_nested_function = partial(deeply_nested_function, 1)
    assert unwrap_wrapped_function(deeply_nested_function) is deeply_nested_function.func

🔘 (none found) − ⏪ Replay Tests

This optimization was discovered automatically with codeflash.ai

Related issue number

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

codeflash-ai bot and others added 2 commits June 6, 2024 21:13
To optimize the given Python program for faster runtime, we can make several enhancements. Primarily, we make use of Python's tuple for type checks and avoid unnecessary set operations by consolidating them into a single set operation. Here's the optimized version.



### Key Optimizations.

1. **Tuple Consolidation for `isinstance`:** 
   - Instead of building a set and updating it conditionally, we use a tuple that is directly used in `isinstance` checks.

2. **Simplified `isinstance` Checks:**
   - We handle type checks and reassignments more efficiently without repeated checks or updates to the set, streamlining the process.

These changes collectively reduce the complexity and improve the performance by minimizing unnecessary operations and making the control flow straightforward.
@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Jun 21, 2024
@codspeed-hq
Copy link

codspeed-hq bot commented Jun 21, 2024

CodSpeed Performance Report

Merging #9727 will not alter performance

Comparing misrasaurabh1:codeflash/optimize-unwrap_wrapped_function-2024-06-06T21.13.38 (4111c32) with main (76c191d)

Summary

✅ 13 untouched benchmarks

Copy link
Contributor

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look great overall here, but I did find one issue with the unwrap_types logic.

Great work on the docstring corrections and general performance improvements.

Comment on lines 732 to 736
unwrap_types = (
(property, cached_property)
+ (partial, partialmethod if unwrap_partial else ())
+ (staticmethod, classmethod if unwrap_class_static_method else ())
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
unwrap_types = (
(property, cached_property)
+ (partial, partialmethod if unwrap_partial else ())
+ (staticmethod, classmethod if unwrap_class_static_method else ())
)
unwrap_types = (
(property, cached_property)
+ (partial, partialmethod) if unwrap_partial else ()
+ (staticmethod, classmethod) if unwrap_class_static_method else ()
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for this, I played a bit with the different ways of doing this and found the best one which had the correct behavior. Updated the PR with the change

Comment on lines 747 to 746
else:
# Make coverage happy as it can only get here in the last possible case
assert isinstance(func, cached_property)
elif isinstance(func, cached_property):
func = func.func # type: ignore
elif unwrap_class_static_method and isinstance(func, (classmethod, staticmethod)):
func = func.__func__
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One note here - I think we might make coverage unhappy with this change

@pydantic-hooky pydantic-hooky bot added the awaiting author revision awaiting changes from the PR author label Jun 22, 2024
@sydney-runkle sydney-runkle added relnotes-performance Used for performance improvements. and removed relnotes-fix Used for bugfixes. labels Jun 22, 2024
Copy link
Contributor

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Waiting on some annoying CI flakiness to merge, but will do soon!

@sydney-runkle sydney-runkle enabled auto-merge (squash) June 25, 2024 22:31
@sydney-runkle sydney-runkle merged commit bd4bc1a into pydantic:main Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting author revision awaiting changes from the PR author relnotes-performance Used for performance improvements.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants