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

Improve performance of State #2463

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

HHongSeungWoo
Copy link

Summary

#2389

I modified setattr to preserve the behavior of the existing State. Although assignment performance has slowed down compared to before, the rest of the performance significantly improves.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@HHongSeungWoo HHongSeungWoo changed the title Refactor State to utilize SimpleNamespace for improved perfomance Improve Performance of State Feb 8, 2024
@HHongSeungWoo HHongSeungWoo changed the title Improve Performance of State Improve performance of State Feb 8, 2024
@HHongSeungWoo
Copy link
Author

@Kludex I tested a few approaches to improve the performance of accessing State properties.
Are there any approaches listed below that look good?

import typing
from types import SimpleNamespace


class State:
    """
    An object that can be used to store arbitrary state.

    Used for `request.state` and `app.state`.
    """

    _state: dict[str, typing.Any]

    def __init__(self, state: dict[str, typing.Any] | None = None):
        if state is None:
            state = {}
        super().__setattr__("_state", state)

    def __setattr__(self, key: typing.Any, value: typing.Any) -> None:
        self._state[key] = value

    def __getattr__(self, key: typing.Any) -> typing.Any:
        try:
            return self._state[key]
        except KeyError:
            message = "'{}' object has no attribute '{}'"
            raise AttributeError(message.format(self.__class__.__name__, key))

    def __delattr__(self, key: typing.Any) -> None:
        del self._state[key]


class GetAttributeState:
    """
    An object that can be used to store arbitrary state.

    Used for `request.state` and `app.state`.
    """

    _state: dict[str, typing.Any]

    def __init__(self, state: dict[str, typing.Any] | None = None):
        if state is None:
            state = {}
        super().__setattr__("_state", state)

    def __setattr__(self, key: typing.Any, value: typing.Any) -> None:
        object.__getattribute__(self, "_state")[key] = value

    def __getattribute__(self, key: typing.Any) -> typing.Any:
        state = object.__getattribute__(self, "_state")

        if key in state:
            return state[key]

        return object.__getattribute__(self, key)

    def __delattr__(self, key: typing.Any) -> None:
        del self._state[key]


class NameSpaceState(SimpleNamespace):
    """
    An object that can be used to store arbitrary state.

    Used for `request.state` and `app.state`.
    """

    _state: dict[str, typing.Any]

    def __init__(self, state: dict[str, typing.Any] | None = None):
        if state is None:
            state = {}
        super().__setattr__("_state", state)
        super().__init__(**state)

    def __setattr__(self, key: typing.Any, value: typing.Any) -> None:
        super().__setattr__(key, value)
        self._state[key] = value


lifespan_state = {"foo": 1}

old = State(lifespan_state)
new = GetAttributeState(lifespan_state)
ns_state = NameSpaceState(lifespan_state)

print(timeit.timeit(lambda: getattr(old, "foo", 1)))  # 0.8453106439992553 sec
print(timeit.timeit(lambda: old.foo))  # 0.8073957750020782 sec
print(timeit.timeit(lambda: getattr(old, "bar", 1)))  # 2.2985301509979763 sec
print(timeit.timeit(lambda: setattr(old, "foo", 1)))  # 0.16180036000150722 sec
print("---------------")
print(timeit.timeit(lambda: getattr(new, "foo", 1)))  # 0.27732334599568276 sec
print(timeit.timeit(lambda: new.foo))  # 0.26606425999489147 sec
print(timeit.timeit(lambda: getattr(new, "bar", 1)))  # 1.4273305930037168 sec
print(timeit.timeit(lambda: setattr(new, "foo", 1)))  # 0.27730098600295605 sec
print("---------------")
print(timeit.timeit(lambda: getattr(ns_state, "foo", 1)))  # 0.0628476350029814 sec
print(timeit.timeit(lambda: ns_state.foo))  # 0.05678495100437431 sec
print(timeit.timeit(lambda: getattr(ns_state, "bar", 1)))  # 0.07138194199796999 sec
print(timeit.timeit(lambda: setattr(ns_state, "foo", 1)))  # 0.42666425200150115 sec

@Zaczero
Copy link
Contributor

Zaczero commented Apr 3, 2024

Context: I am just a random person - not a maintainer. I personally think the SimpleNamespace would work best but it can be simplified like that:

class NameSpaceState2(SimpleNamespace):
    """
    An object that can be used to store arbitrary state.

    Used for `request.state` and `app.state`.
    """

    def __init__(self, state: dict[str, typing.Any] | None = None):
        if state is not None:
            super().__init__(**state)

    @property
    def _state(self):
        return self.__dict__

There is no need to keep 2 separate dicts. It has better performance characteristic for setting attrs and does not bug out when accessing _state directly.

image

image

I confirm that this class is generally 10x faster compared to existing solution.

@Kludex
Copy link
Member

Kludex commented Dec 12, 2024

Context: I am just a random person - not a maintainer. I personally think the SimpleNamespace would work best but it can be simplified like that:

class NameSpaceState2(SimpleNamespace):
    """
    An object that can be used to store arbitrary state.

    Used for `request.state` and `app.state`.
    """

    def __init__(self, state: dict[str, typing.Any] | None = None):
        if state is not None:
            super().__init__(**state)

    @property
    def _state(self):
        return self.__dict__

There is no need to keep 2 separate dicts. It has better performance characteristic for setting attrs and does not bug out when accessing _state directly.

image

image

I confirm that this class is generally 10x faster compared to existing solution.

This fails on the test suite 🤔

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