aiidalab / aiidalab-widgets-base

Reusable widgets for AiiDAlab applications
MIT License
6 stars 17 forks source link

Lazy import optimade_client #496

Closed danielhollas closed 1 year ago

danielhollas commented 1 year ago

optimade_client brings a lot of dependencies that slow down module import, and in turn slow down app loading even for applications that do not use Optimade.

I haven't actually tested this code since I do not use the Optimade widget, please test as part of review.

NOTE: This is part of a larger effort to improve import times, documented here

https://aiida.discourse.group/t/why-is-aiidalab-base-widget-import-so-slow/32/18

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.03 :warning:

Comparison is base (d0613e8) 79.63% compared to head (7ebdee8) 79.61%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #496 +/- ## ========================================== - Coverage 79.63% 79.61% -0.03% ========================================== Files 27 27 Lines 3752 3748 -4 ========================================== - Hits 2988 2984 -4 Misses 764 764 ``` | Flag | Coverage Δ | | |---|---|---| | python-3.10 | `79.61% <100.00%> (-0.03%)` | :arrow_down: | | python-3.8 | `79.65% <100.00%> (?)` | | | python-3.9 | `79.65% <100.00%> (-0.03%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/496?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | Coverage Δ | | |---|---|---| | [aiidalab\_widgets\_base/databases.py](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/496?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#diff-YWlpZGFsYWJfd2lkZ2V0c19iYXNlL2RhdGFiYXNlcy5weQ==) | `94.11% <100.00%> (-0.15%)` | :arrow_down: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

unkcpz commented 1 year ago

@danielhollas thanks for the great job to improve the import time. Do you also have a benchmark for the change in this PR, how much time it will improve?

I'll give OPTIMADE widget a test late today.

danielhollas commented 1 year ago

It will be much more noticeable after aiida-core changes are merged, but my quick testing showed hundreds of miliseconds improvements. Will do morencarefull benchmarking later. Thanks for taking a look!

danielhollas commented 1 year ago

Some timings, note that these are done on top of my faster-import branch of aiida-core.

Master branch of AWB

$ time python -c "import aiidalab_widgets_base"
real    0m1.717s
user    0m2.790s
sys 0m6.881s

lazy optimade import

08:38 $ time python -c "import aiidalab_widgets_base"
real    0m1.216s
user    0m2.304s
sys 0m6.698s

It looks like half a second difference!

It would probably be cool to see if optimade_client could be optimized so that QeApp can benefit as well. Here are some interesting timings.

$ time python -c "from optimade_client import default_parameters"

real    0m0.050s
user    0m0.038s
sys 0m0.013s

$ time python -c "from optimade_client import query_filter"

real    0m1.232s
user    0m1.820s
sys 0m3.390s

08:43 $ time python -c "from optimade_client import query_provider"
real    0m0.611s
user    0m0.551s
sys 0m0.060s
unkcpz commented 1 year ago

@danielhollas I test with a brand new environment, the OPTIMADE is working fine.

However, I everytime I run with time command it give divergent results:

(base) jovyan@7f3069412202:~/aiidalab-widgets-base$ time python -c "import aiidalab_widgets_base"

real    0m5.281s
user    0m3.491s
sys 0m1.429s
(base) jovyan@7f3069412202:~/aiidalab-widgets-base$ time python -c "import aiidalab_widgets_base"

real    0m12.569s
user    0m3.926s
sys 0m1.947s
(base) jovyan@7f3069412202:~/aiidalab-widgets-base$ time python -c "import aiidalab_widgets_base"

real    0m5.486s
user    0m4.048s
sys 0m1.248s
(base) jovyan@7f3069412202:~/aiidalab-widgets-base$ time python -c "import aiidalab_widgets_base"

real    0m2.897s
user    0m2.660s
sys 0m0.491s
(base) jovyan@7f3069412202:~/aiidalab-widgets-base$ time python -c "import aiidalab_widgets_base"

real    0m3.251s
user    0m2.628s
sys 0m0.838s
(base) jovyan@7f3069412202:~/aiidalab-widgets-base$ time python -c "import aiidalab_widgets_base"

real    0m4.001s
user    0m3.192s
sys 0m0.935s
(base) jovyan@7f3069412202:~/aiidalab-widgets-base$ time python -c "import aiidalab_widgets_base"

real    0m2.514s
user    0m2.257s
sys 0m0.458s
(base) jovyan@7f3069412202:~/aiidalab-widgets-base$ time python -c "import aiidalab_widgets_base"

real    0m2.475s
user    0m2.097s
sys 0m0.478s
(base) jovyan@7f3069412202:~/aiidalab-widgets-base$ time python -c "import aiidalab_widgets_base"

real    0m4.360s
user    0m3.560s
sys 0m0.941s
danielhollas commented 1 year ago

However, I everytime I run with time command it give divergent results:

I think this is because of the aiida.orm import is unstable. Can you try it with my faster-import aiida-core branch, as desribed on the forum?

https://github.com/danielhollas/aiida-core/commits/faster-import

unkcpz commented 1 year ago

Had a test with faster-import branch, it is faster! The time different with/without this PR is around 0.5s.

danielhollas commented 1 year ago

@unkcpz awesome, thanks! It's great that you could validate my timings.