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

Prevent regressions in import time #6387

Open
danielhollas opened this issue May 13, 2024 · 0 comments
Open

Prevent regressions in import time #6387

danielhollas opened this issue May 13, 2024 · 0 comments

Comments

@danielhollas
Copy link
Collaborator

I guess the CI test we added a long time ago to catch these regressions is not doing its job. Guess it can be difficult to tune where we catch real significant regressions in performance but don't get hit by false positives all the time. Appreciate your effort to optimize this manually 👍

Originally posted by @sphuber in #6382 (review)

I spent some (too much?) time on optimizing the import time of the various aiida-core submodules --- the question is how do we make sure we don't regress?

We are already testing that unwanted modules are not loaded during verdi completion and verdi invocation. That's probably the most impactful for UX so it's great we have tests for that.

Perhaps we could expand the tests a little bit. For example a recent regression in aiida.orm import was caused by an import of aiida.storage from aiida.orm. Perhaps this is something we should test again as well, would be beneficial both in terms of import speed and in general to keep the various modules decoupled from each other.

(for example, right now aiida.cmdline is imported from aiida.orm which is not great from a design standpoint.

Besides automated tests, perhaps we could have a checklist of performance checks that should be done before each release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant