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

Migrate restore_state helper to use registry loading pattern #93773

Merged
merged 13 commits into from May 31, 2023

Conversation

bdraco
Copy link
Member

@bdraco bdraco commented May 30, 2023

Its been a while since I looked at startup performance. The new yellow got me looking at it again since when I migrated my production config to it, I noticed a few integrations failed at startup (and had to retry) because the cpu load got too high during startup.

Proposed change

As more entities have started using restore_state over time, it has become a startup bottleneck as each entity being added is creating tasks (was using a gather) to load restore state data that is already loaded since it is a singleton

We now use the same pattern as the registry helpers

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:

As more entities have started using restore_state over time, it
has become a startup bottleneck as each entity being added is
creating a task to load restore state data that is already loaded
since it is a singleton

We now use the same pattern as the registry helpers
@bdraco
Copy link
Member Author

bdraco commented May 30, 2023

This one is interesting since it looks like some race bugs might be relying on the gather that was previously here.

Was a refactoring error 🤦

@bdraco
Copy link
Member Author

bdraco commented May 30, 2023

add_to_platform_finish looks a lot better now along with the automation bound method change.

Will likely need to adjust some mocks

@bdraco
Copy link
Member Author

bdraco commented May 30, 2023

It seems like setting update_before_add=True when its not needed is more common than I expected. Will need to audit for that

@bdraco bdraco closed this May 30, 2023
@bdraco bdraco reopened this May 30, 2023
@bdraco bdraco marked this pull request as ready for review May 30, 2023 15:17
@bdraco bdraco requested review from dmulcahey, Adminiuga, puddly and a team as code owners May 30, 2023 15:17
@bdraco
Copy link
Member Author

bdraco commented May 30, 2023

Missing coverage is existing. Code is only moved that if affected and not changed

@bdraco
Copy link
Member Author

bdraco commented May 31, 2023

Thanks. Thats the bulk of the startup improvements. Going to work with Mike on the numerous hassio get requests at startup next cycle

@bdraco bdraco merged commit fba826a into dev May 31, 2023
48 checks passed
@bdraco bdraco deleted the restore_state_bottleneck branch May 31, 2023 01:48
@github-actions github-actions bot locked and limited conversation to collaborators Jun 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants