feat: optimize reject_recursive_repeats

[BobTheBuidler]

What was wrong?

Nothing was wrong, but reject_recursive_repeats was ripe for optimization since it is used frequently throughout the codebase

Related to Issue #N/A
Closes #N/A

How was it fixed?

I optimized the decorator by refactoring out unnecessary variables and repetitive attr lookups

Todo:

• Clean up commit history
• Add entry to the release notes

Cute Animal Picture

[BobTheBuidler]

-2 var assignments, -2 var reads, -2 tuple creations per call

[BobTheBuidler]

-1 attr lookup, -1 setitem per call

[BobTheBuidler]

-1 attr lookup per call

[BobTheBuidler]

1 less attr lookup per access

[BobTheBuidler]

-1 set var, -1 var lookup per call

[BobTheBuidler]

Now that you've merged #3665 and #3666 I rebased my branch on master and reran the tests, looks like everything passes now

[kclowes]

Thanks for the PR! IMHO, the readability is about the same, and I'm not seeing any gains in test run times. If you can provide some benchmarks that prove that this helps, we'd be happy to take another look and merge it in.

[BobTheBuidler]

@kclowes I found a few minutes while waiting for some scripts to startup, and put together a benchmark for this:

import functools
import threading
from time import time
from typing import Any, Callable, Set, Tuple

def reject_recursive_repeats_old(to_wrap: Callable[..., Any]) -> Callable[..., Any]:
    """
    Prevent simple cycles by returning None when called recursively with same instance
    """
    # types ignored b/c dynamically set attribute
    to_wrap.__already_called = {}  # type: ignore

    @functools.wraps(to_wrap)
    def wrapped(*args: Any) -> Any:
        arg_instances = tuple(map(id, args))
        thread_id = threading.get_ident()
        thread_local_args = (thread_id,) + arg_instances
        if thread_local_args in to_wrap.__already_called:  # type: ignore
            raise Exception(f"Recursively called {to_wrap} with {args!r}")
        to_wrap.__already_called[thread_local_args] = True  # type: ignore
        try:
            wrapped_val = to_wrap(*args)
        finally:
            del to_wrap.__already_called[thread_local_args]  # type: ignore
        return wrapped_val

    return wrapped


def reject_recursive_repeats_new(to_wrap: Callable[..., Any]) -> Callable[..., Any]:
    """
    Prevent simple cycles by returning None when called recursively with same instance
    """
    # types ignored b/c dynamically set attribute
    already_called: Set[Tuple[int, ...]] = set()
    to_wrap.__already_called = already_called  # type: ignore

    add_call = already_called.add
    remove_call = already_called.remove

    @functools.wraps(to_wrap)
    def wrapped(*args: Any) -> Any:
        thread_local_args = (threading.get_ident(), *map(id, args))
        if thread_local_args in already_called:
            raise Exception(f"Recursively called {to_wrap} with {args!r}")
        add_call(thread_local_args)
        try:
            return to_wrap(*args)
        finally:
            remove_call(thread_local_args)

    return wrapped


@reject_recursive_repeats_old
def test_old():
    pass

@reject_recursive_repeats_new
def test_new():
    pass

start = time()
for _ in range(10_000):
    test_old()
print(f"old: {time() - start}")

start = time()
for _ in range(10_000):
    test_new()
print(f"new: {time() - start}")

old: 0.006606578826904297
new: 0.005003213882446289

The new implementation takes 25% less time!

[kclowes]

Sick, thanks for putting in the work for the benchmarking script!