Only rebuild the Python dependency graph when files changed (#2599) - #2670
Open
mikkelke wants to merge 1 commit into
Open
Only rebuild the Python dependency graph when files changed (#2599)#2670mikkelke wants to merge 1 commit into
mikkelke wants to merge 1 commit into
Conversation
…n#2599) PythonDeps.refresh_dep_graph reads and ast.parses every Python file under the apps directory. Dependencies.update called it unconditionally, and check_app_updates calls update on every utility loop iteration, so every pass re-parsed the whole app tree even when nothing had been touched. On a real installation with 79 app files (2.76 MB) this costs 2.19 s per iteration, 98% of it in ast.parse, on the event loop. Once the pass exceeds max_utility_skew the utility loop also skips its own sleep, so it becomes a hot loop: measured 86% of the event loop blocked in ~2.2 s chunks every ~2.4 s, and the container pinned at 102% CPU. App callbacks arrive up to 2.4 s late as a result. FileCheck.update has already computed the new/modified/deleted sets by this point, so it knows whether anything needs re-parsing. Gate the refresh on that via a needs_refresh hook, overridden only in PythonDeps - AppDeps.refresh_dep_graph walks the already-parsed app config rather than the filesystem, and is left refreshing unconditionally. Files that previously failed to parse are still retried: a fixed file has a new mtime and therefore lands in FileCheck.modified.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #2599.
The problem
PythonDeps.refresh_dep_graphreads andast.parses every Python file under the apps directory.Dependencies.updatecalls it unconditionally, andcheck_app_updatescallsupdateon every utility-loop iteration — so every pass re-parses the entire app tree even when nothing was touched.Measured on a live install (79 app files, 2.76 MB, AppDaemon 4.5.13, Python 3.12), replaying the exact code path in-container:
98% of the cost is
ast.parse. Reading the files is 13 ms — this is CPU, not I/O.It compounds: once the pass exceeds
max_utility_skew,utility_loopskips its own sleep, so the loop goes hot. Measured from inside the container (482 sequential requests over 15 s):86% of the event loop blocked in ~2.2 s chunks every ~2.4 s, container pinned at 102% CPU. Because app-side sync API calls go through
run_coroutine_threadsafeonto that same loop, app callbacks arrive up to ~2.4 s late — visible in app logs as stalls mid-callback.The fix
FileCheck.updatehas already computed the new/modified/deleted sets by the timerefresh_dep_graphis called, so it already knows whether anything needs re-parsing. This gates the refresh onthere_were_changesvia aneeds_refreshhook, overridden only inPythonDeps.AppDepsdeliberately keeps refreshing unconditionally: itsrefresh_dep_graphwalks the already-parsed app config rather than the filesystem, so it is cheap, and it is also called explicitly elsewhere when app configs change.Files that previously failed to parse are still retried — a fixed file has a new mtime and therefore lands in
FileCheck.modified. There is a test for exactly that.Tests
Adds
tests/unit/test_dependency_manager.py(7 tests): graph built on init; unchanged files do not rebuild; new/modified/deleted each do rebuild; a fixed bad file is retried;AppDepsstill refreshes unconditionally. Three of them fail ondevand pass with the change. Full unit suite: 209 passed.Note
This is a targeted fix, not the
watchdog/inotify rework mentioned in #1897 — it just stops the redundant work. Raisingutility_delayis a workaround but costs reload latency; this keeps detection at the normal interval.