8000 Only reload modified automations #80282
Conversation
|
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( |
| await asyncio.gather(*tasks) | ||
|
|
||
| # Create automations which have new config or have been added | ||
| # Create automations which have changed config or have been added |
There was a problem hiding this comment.
If an automation has a changed config, could we just call set_config on it and re-attach the trigger? That way it doesn't have to go to unavailable in between.
There was a problem hiding this comment.
That might be feasible (unsure) for automations with a unique Id, which enables conclusively determining that the automation was changed. For others, it is harder to conclusively match them up if changes occurred, especially if an automation had the alias changed. One could perhaps try to match up based only on alias, but care would be needed around edge cases like an automation with I’d, and one without swapping aliases.
If the alias changed, and the user has not overridden the entity name, this would presumably need to also update the entity name to match the behavior of reloading from scratch, right?
|
The new worst case time complexity for this is O(N^2). I believe the best theoretically possible time complexity here is O(N), however achieving that is somewhat complicated, so it is probably not worth doing unless the current approach proves too slow for larger numbers of automations in practice. I’ll lay out a quick sketch of how to achieve the better time complexity, just in case it proves useful.
Loop through the the existing automations. For each automation, we perform a canonical serialization of the raw_config, and then generate a sha1 (or better) hash of the result. We use this to populate a data structure of a dict mapping the sha1 hash to a list of indices into the existing automations list for entries with that hash. Adding a new dict entry is amortized O(1), or if the key is present, pushing the index to the back of the list is also amortized O(1) time.
Now create two sets. One will hold indices of found matches for the previously loaded automations, and the other will hold indices for found matches of newly read automations. Then loop through the newly read automations, performing the same canonical serialization and hashing. Look up the hash in the previously constructed dictionary list. If it is present, and contains a nonempty list, add the index to the set, pop the last entry from the looked up list, and add it to the other set. Now the filtered versions of both lists can be performed via list comprehensions where indices were not in the sets are kept (just like the current code does) Having to construct canonical serializations of these values and then hash them is what makes this a complicated approach that I would not recommend unless performance of the current approach proves to be a problem in practice. Automations without aliases could be handled by including the generated name in the serialization. |
| for config_idx, config in enumerate(automation_configs): | ||
| if config_idx in config_matches: | ||
| # Only allow an automation config to match at most once | ||
| 8000 | continue |
There was a problem hiding this comment.
I guess an alternative, slightly faster approach could be to store list(enumerate(automation_configs)) and pop out any matches.
There was a problem hiding this comment.
How is it faster? You mean we don't need to check if config_idx is has a match?
|
8000
@KevinCathcart I'm not sure your solution would be beneficial because of the typically very very poor performance of serializing. |
|
I don't think that is needed, in the end, this is not an operation that is running that often that it becomes a concern. As a matter of fact, this PR is probably improving performance overall already (as not everything gets reloaded). |
|
I'll merge this now, optimizing away the dict comparisons can be done in a follow-up 👍 |
|
The issue with an O(N^2) algorithm, is that if N gets big enough things get really slow. There will certainly exist some N for which this is slower than reloading everything. The question is how big is that N. If it is requires thousands of automations then we probably don’t care. If only 200 automations were enough to be slower then we might care. That is why I said pursuing the better complexity may not be worthwhile. It could easily be slower for all N that we actually care about. And yes, using a hashable dict could be an alternative to serialization if implemented corrected, and we used similarly hashable values all the way down. But again, only relevant if perf becomes an issue at counts we actually care about. |
Proposed change
Only reload modified automation when automation configuration is reloaded
Similar to #80254, but works also when manually editing the YAML configuration
WTH: https://community.home-assistant.io/t/wth-do-all-automatons-get-reloaded-when-you-change-any-of-them/467270
Type of change
Additional information
Checklist
black --fast 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.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: