Add support for importing integrations in the executor #111336
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alternative solution to #111331
home-assistant
bot
added
cla-signed
core
integration: matter
small-pr
|
Hey there @home-assistant/matter, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
bdraco
changed the title
|
|
|
2024-02-25 02:52:40.237 DEBUG (MainThread) [homeassistant.config_entries] Loaded flow for denonavr in 4.57s |
|
cut start up time in half on the green! bluetooth went from ~12s to 0.6s because its not longer blocked |
|
This is pretty awesome 😎 as I thought this wasn't able to be improved for years. It took the |
|
It also solves all the storage loading getting backed up as well |
|
|
|
Manually updated a yellow. There are a few more that block the loop for followups full log: |
|
Manually updated a blue There are a few more that block the loop for followups Full log: |
|
Going to test on a Raspberry Pi 3 (32-bit) next. That one takes a while for everything so it will be a good test |
|
rpi 3b 32bit There are a few more that block the loop for followups. I'm on the fence on these since rpi 3bs are less and less common though: full log |
|
@bdraco just for better understanding, were/are there IO's caused by this imports which block the event loop or were this just pure compute/CPU bound? I guess the latter makes this solution essentially Home Assistant startup "multi-threaded", hence better leverage multicore CPUs such as Greens quad-core CPU. |
|
Thinking a bit more about it, I guess Python on a lower layer always has to do some IO to import modules in the end. 🤔 In any case, even if there is no IO involved, making them run in an executor makes this leverage multi-threading. I was wondering if Python is handling multi-threaded imports well (as in, a) is it safe and b) does it support truly multi-threaded or is there a single global lock which serializes import). It seems it is safe and at least since Python 3.3 it does lock per-module, so this makes it truly parallel 🎉 (according to this StackOverflow post). |
For some modules with large files (both size or count), its I/O bound. For |
balloob
reviewed
Feb 26, 2024
balloob
reviewed
Feb 26, 2024
|
I need to update the docs/blog post since its only |
|
merge is to fix conflicts |
balloob
approved these changes
Feb 26, 2024
bdraco
added a commit
that referenced
this pull request
Feb 26, 2024
was not added to the original PR to contain scope identified at: #111336 (comment)
20 tasks
bdraco
added a commit
that referenced
this pull request
Feb 26, 2024
was not added to the original PR to contain scope identified at: #111336 (comment)
20 tasks
bdraco
added a commit
that referenced
this pull request
Feb 26, 2024
was not added to the original PR to contain scope identified at: #111336 (comment)
20 tasks
balloob
pushed a commit
that referenced
this pull request
Feb 26, 2024
…#111519) was not added to the original PR to contain scope identified at: #111336 (comment)
balloob
pushed a commit
that referenced
this pull request
Feb 26, 2024
…1520) was not added to the original PR to contain scope identified at: #111336 (comment)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed change
Load integrations in the executor if
import_executoris specified inmanifest.jsonto avoid the code import blocking the event loop.While investigating Bluetooth problems on a Home Assistant Green, I found that Bluetooth failed to start because the event loop was blocked for 7-12s from loading
matterwhich caused the system to be unstable and timeout many taskshome-assistant-libs/python-matter-server#578. The problem was not Bluetooth itself, as once the source of the event loop blocking was addressed and Bluetooth is no longer blocked, the startup time is < 1s.
I expect this change will fix hundreds of unexplained startup issues. It will also solve some instability at run time since the same problem happens when an integration is discovered and the config flow is loaded since it loads the modules there as well.
This is opt-in because some modules are cheap to import or call
asyncio.get_running_loop()or similar during import so they can't be imported in the executor.Type of change
Additional information
MatterClientblocks the event loop for ~7-12 seconds. home-assistant-libs/python-matter-server#578Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.To help with the load of incoming pull requests: