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

Add support for caching entity properties #100601

Merged
merged 19 commits into from
Dec 22, 2023
Merged

Conversation

emontnemery
Copy link
Contributor

@emontnemery emontnemery commented Sep 19, 2023

Proposed change

Add support for caching entity properties

The implementation in this PR is essentially the same as in #95315 but:

  • Updates of Entity._attr_* are caught and cause the corresponding property's cache to be invalidated
  • All properties which by default return the corresponding Entity._attr_* attribute are cached

With the simple test below, the time needed for calculating state is decreased by around 1/3 on my development machine, from ~3.3 µs / iteration to ~1.8 µs / iteration:

from homeassistant.helpers.entity import Entity

ent = Entity()
return timeit.timeit(ent._async_calculate_state)

If this PR is accepted, base classes for entity domains should be modified in the same way in follow-up PRs

Needs:

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

@emontnemery emontnemery requested a review from a team as a code owner September 19, 2023 14:50
@emontnemery emontnemery marked this pull request as draft September 19, 2023 14:50
@MartinHjelmare
Copy link
Member

The PR description needs to be corrected. It's talking about the wrong attribute.

@emontnemery

This comment was marked as outdated.

@emontnemery emontnemery marked this pull request as ready for review December 20, 2023 14:52
@MartinHjelmare
Copy link
Member

This test shows that time needed for calculating state is decreased by around 1/3:

What is the amount of time as an example? It also matters if it's already slow or very fast.

@emontnemery
Copy link
Contributor Author

This test shows that time needed for calculating state is decreased by around 1/3:

What is the amount of time as an example? It also matters if it's already slow or very fast.

I updated the PR description with this information.

@bdraco
Copy link
Member

bdraco commented Dec 20, 2023

Will test this later today.

At first glance it looks like it will take care of some of the heavy hitters we see with a large number of polling integrations.

@bdraco
Copy link
Member

bdraco commented Dec 20, 2023

With previous testing the hot platforms where in order:

  • sensor
  • select
  • number
  • camera

These are the places where the cost is much higher where I expect this will really ✨

@bdraco
Copy link
Member

bdraco commented Dec 20, 2023

Looks like tests.common MockEntity needs some new mocks

@MartinHjelmare
Copy link
Member

decreased by around 1/3 on my development machine, from ~1.8 µs / iteration to ~3.3 µs / iteration

This sounds incorrect.

@emontnemery
Copy link
Contributor Author

decreased by around 1/3 on my development machine, from ~1.8 µs / iteration to ~3.3 µs / iteration

This sounds incorrect.

Yeah, the numbers were swapped, fixed now.

bdraco added a commit that referenced this pull request Dec 22, 2023
bdraco added a commit that referenced this pull request Dec 22, 2023
bdraco added a commit that referenced this pull request Dec 22, 2023
bdraco added a commit that referenced this pull request Dec 22, 2023
bdraco added a commit that referenced this pull request Dec 22, 2023
bdraco added a commit that referenced this pull request Dec 22, 2023
bdraco added a commit that referenced this pull request Dec 22, 2023
bdraco added a commit that referenced this pull request Dec 22, 2023
bdraco added a commit that referenced this pull request Dec 22, 2023
bdraco added a commit that referenced this pull request Dec 22, 2023
bdraco added a commit that referenced this pull request Dec 22, 2023
bdraco added a commit that referenced this pull request Dec 22, 2023
bdraco added a commit that referenced this pull request Dec 22, 2023
@MartinHjelmare MartinHjelmare added the noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) label Dec 22, 2023
Bre77 pushed a commit to Bre77/core that referenced this pull request Dec 23, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed core noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants