-
Notifications
You must be signed in to change notification settings - Fork 18
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 setup code performance #565
Conversation
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.
@superstar54 thanks! Please see my comment above, I propose to hold this PR at the moment to see if there are better solutions and see if aida-core==2.5.0
will fix the performance issue.
_setup_code(code_name) | ||
python_code += _generate_string_to_setup_code(code_name) | ||
try: | ||
run(["python", "-c", python_code], capture_output=True, check=True) |
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.
I just realized this change might be a bit flaky because it is not very common way we setup codes in aiida.
It looks still like a workaround. The ideal solution will be calling the API to setup the codes instead of creating a script and run it with subprocess.run
.
On the other hand, we already know that the performance issue comes from every time calling verdi
it takes time to import the modules which improved in v2.5.0
and will be released soon.
Let's hold the solution of this PR or maybe you try using calling API directly?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #565 +/- ##
=======================================
Coverage 80.73% 80.73%
=======================================
Files 49 49
Lines 3415 3415
=======================================
Hits 2757 2757
Misses 658 658
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Will make another PR from my forked repo. |
Reported by @unkcpz in #564 that, it takes 1-2 sec to set up a code, and we have 14 codes, so around 20 sec in total. Which is not good, and it takes too long to load the container.
We can remove some of the codes, and only keep the ones that will used in the present plugin, as suggested in #564 . There are still 6 codes, which need around 10 secs.
I looked into the issue and found that most of the time is used to set up the verdi command, profile and database. Thus it's would be good to combine all code set up as one.
Since the code setup is run in a different thread, thus, using the
InstalledCode
directly will fail when storing the code. This PR uses thepython -c
command to run in thesubprocess
.The total time to set up all 14 codes now reduced to 2-3 secs.