-
Notifications
You must be signed in to change notification settings - Fork 133
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
improve module reloader performance #1197
Conversation
Cache module dependencies. Large modules, including some part of the standard lib (that will never be edited), take >1sec to be analyzed. This makes reloads feel (almost) instantaneous.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
) -> dict[str, types.ModuleType]: | ||
"""Returns the set of modules used by the graph that have been modified""" | ||
stale_modules: dict[str, types.ModuleType] = {} | ||
modified_modules = reloader.check(modules=sys.modules, reload=False) | ||
# TODO(akshayka): could also exclude modules part of the standard library; | ||
# haven't found a reliable way to do this, however. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we list them out? at least the top 20?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The performance hit isn't too bad, given the new commit I pushed. Listing them out will lead to issues when people declare modules with the same name as built-in modules (which isn't that uncommon). If this becomes a bottleneck, we can look for a better solution (I'm pretty sure one will exist).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, one small comment on the todo
... and remove manual caching. This makes sure that expensive packages that aren't imported top-level in the notebook still get cache hits.
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.4.3-dev6 |
Use a single
ModuleFinder
instance to make use of its built-in cache. Large modules, including some part of the standard lib (that will never be edited), take >1sec to be analyzed. This makes reloads feel (almost) instantaneous.