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

Expose Lithops wait_dur_sec to speed up tests #456

Merged
merged 1 commit into from
May 1, 2024
Merged

Conversation

tomwhite
Copy link
Member

Set to a smaller value for tests so they complete faster.

This is related to lithops-cloud/lithops#1292, but doesn't need any changes to Lithops. It's possible that there are more improvements to be made, but this already helps speed up the tests.

…ompleted tasks

Set to a smaller value for tests so they complete faster.
@tomwhite
Copy link
Member Author

The tests running on ubuntu-latest Python 3.9 went from 13m52s to 8m49s. Others showed significant improvements too.

(I also noticed that ubuntu-latest Python 3.12 is a lot slower than 3.9 - almost 2x - would be good to look into it.)

@tomwhite tomwhite merged commit 856dae0 into main May 1, 2024
9 checks passed
@tomwhite tomwhite deleted the wait-dur-sec branch May 1, 2024 07:10
@JosepSampe
Copy link
Contributor

JosepSampe commented Jun 7, 2024

Hi @tomwhite,

While running the cubed tests recently, I noticed a few details that could help improve their performance:

Firstly, regarding your observation about tests running 2x slower in Python 3.12, I found that this slowdown is caused by the codecov plugin. In my fork, I modified the workflow to run the coverage only on Ubuntu 3.9. This change resulted in a 2x reduction in test time for Python 3.12.

  - name: Run tests
    run: |
      if [ "$RUNNER_OS" == "Linux" ] && [ "${{ matrix.python-version }}" == "3.9" ]; then
        pytest -v --cov=cubed --cov-report=term-missing --cov-fail-under=90
      else
        pytest -v
      fi

Secondly, in Lithops, there is a configuration parameter that allows you to disable the module_manager module. This module analyzes the code (in this case, the cubed code), checks the dependencies, and packages and transfers them to the cloud if they are not present in the remote runtime. For localhost tests, this module_manager is unnecessary since all dependencies are already in the local environment, so there is no need to transfer anything. You can disable it in your utils.py file by setting:

LITHOPS_LOCAL_CONFIG = {
    "lithops": {
        "backend": "localhost",
        "storage": "localhost",
        "monitoring_interval": 0.1,
        "include_modules": None
    }
}

The overhead of the module_manager is probably negligible for a single map, but in your tests, where you run tens of maps, it starts to accumulate. For example, by disabling it, you can save approximately 30 seconds in the Ubuntu and macOS tests, and around 2 minutes in the Windows tests.

In summary, with these two changes, the tests can run in approximately 8 minutes instead of the current 17 minutes.

@tomwhite
Copy link
Member Author

Thanks for helping speed up the Cubed tests @JosepSampe!

I found this issue about the slow down in Python 3.12: nedbat/coveragepy#1665. It looks like you can now set COVERAGE_CORE=sysmon to make it run at a similar speed on 3.12. Would you be interested in doing a PR to fix that in Cubed?

Regarding module_manager, would that be a useful change to Lithops generally? I.e. never analyse the code when running on localhost.

@JosepSampe JosepSampe mentioned this pull request Jun 10, 2024
@JosepSampe
Copy link
Contributor

Regarding module_manager, would that be a useful change to Lithops generally? I.e. never analyse the code when running on localhost.

I've been thinking about this, but right now I'm not sure if all the use cases will align with yours. For example, in the Lithops tests, I need this activated to ensure the module_manager is working properly. Additionally, if you set a runtime different from the interpreter you use to invoke the Lithops script, such as a container image, this needs to be activated as well. But in any case, I will check if it is feasible and ensure it won't break any existing code from other users.

@tomwhite
Copy link
Member Author

Regarding module_manager, would that be a useful change to Lithops generally? I.e. never analyse the code when running on localhost.

I've been thinking about this, but right now I'm not sure if all the use cases will align with yours. For example, in the Lithops tests, I need this activated to ensure the module_manager is working properly. Additionally, if you set a runtime different from the interpreter you use to invoke the Lithops script, such as a container image, this needs to be activated as well. But in any case, I will check if it is feasible and ensure it won't break any existing code from other users.

+1 sounds good.

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

Successfully merging this pull request may close these issues.

2 participants