Fix performance regressions and overhead for Python runner actions #3809
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
moved this script from st2actions into st2common. Since test was broken, we didn't correctly detect the timing and performance issues. It looks like we introduced a bunch of new performance issues which now need to be tracked down again.
some very expensive imports inline so they are not imported by functions inside that module which don't need that functionality.
compatibility issues with some 3rd party libraries which break some Python runner actions.
enykeev
approved these changes
Oct 24, 2017
|
Great to see this getting attention. In some environments a couple of seconds has a lot of impact. Thanks for the progress. |
|
@koenbud Yeah - I also fixed / improved the test so hopefully regressions like that won't happen in the future anymore. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the past I spent quite a lot of time working on optimizing the run time of simple and short Python runner actions (there was a lot of overhead added by start up time, mostly by some very slow imports).
I managed to get it down to 0.2 seconds or so, but we recently moved some modules around and added some function definitions to some modules which introduced big performance overheads (run time is now back at ~2 seconds).
The reason for that was a broken test - the test which should ensure that we don't introduce performance regressions didn't work correctly because it didn't actually check that the script it tries to run exists (it broke when we moved wrapper script from
st2actionstost2common).As a first step, I of course fixed this test so things like that won't happen again - 69a63f5.
I also managed to track down performance regressions we introduced and get run time down to around 0.5s - 9f7feba. This is still not where we were originally, but with changes here and changes in #3803 (merged in time for v2.6), run time is down back to 0.2-0.3 seconds again.
Before:
(virtualenv)vagrant@vagrant-ubuntu-trusty-64:/data/stanley$ python /data/stanley/st2common/st2common/runners/python_action_wrapper.py --is-subprocess usage: python_action_wrapper.py [-h] --pack PACK --file-path FILE_PATH [--config CONFIG] [--parameters PARAMETERS] [--user USER] [--parent-args PARENT_ARGS] python_action_wrapper.py: error: argument --pack is required (virtualenv)vagrant@vagrant-ubuntu-trusty-64:/data/stanley$ time python /data/stanley/st2common/st2common/runners/python_action_wrapper.py --is-subprocess usage: python_action_wrapper.py [-h] --pack PACK --file-path FILE_PATH [--config CONFIG] [--parameters PARAMETERS] [--user USER] [--parent-args PARENT_ARGS] python_action_wrapper.py: error: argument --pack is required real 0m1.779s user 0m0.371s sys 0m0.691s (virtualenv)vagrant@vagrant-ubuntu-trusty-64:/data/stanley$ python /data/stanley/st2common/st2common/runners/python_action_wrapper.py --is-subprocess usage: python_action_wrapper.py [-h] --pack PACK --file-path FILE_PATH [--config CONFIG] [--parameters PARAMETERS] [--user USER] [--parent-args PARENT_ARGS] python_action_wrapper.py: error: argument --pack is required (virtualenv)vagrant@vagrant-ubuntu-trusty-64:/data/stanley$ time python /data/stanley/st2common/st2common/runners/python_action_wrapper.py --is-subprocess usage: python_action_wrapper.py [-h] --pack PACK --file-path FILE_PATH [--config CONFIG] [--parameters PARAMETERS] [--user USER] [--parent-args PARENT_ARGS] python_action_wrapper.py: error: argument --pack is required real 0m1.779s user 0m0.371s sys 0m0.691sAfter:
(virtualenv)vagrant@vagrant-ubuntu-trusty-64:/data/stanley$ time python /data/stanley/st2common/st2common/runners/python_action_wrapper.py --is-subprocess usage: python_action_wrapper.py [-h] --pack PACK --file-path FILE_PATH [--parameters PARAMETERS] [--user USER] [--parent-args PARENT_ARGS] python_action_wrapper.py: error: argument --pack is required real 0m0.517s user 0m0.070s sys 0m0.225s/cc @bigmstone @m4dcoder - just a heads up - some imports like eventlet, etc. can add a lot of overhead and that's why the imports were organized as they were before to avoid this overhead in python runner wrapper script