-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Optimize imports #4248
Merged
Merged
Optimize imports #4248
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
Avoid loading early on the following modules: - questionary - pkg_resources (not used) - crf_entity_extractor
Fixes 'rasa.rasa.rasa.rasa.rasa.rasa' working.
wochinge
approved these changes
Aug 19, 2019
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.
Great detectives work! Do you also have a screenshot of the new import times which we can add to the issue / pr? And can we create an issue to do the import optimization for the CLI as well?
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.
Proposed changes:
Fixes #3010.
The idea of this PR is to make the
rasa
module load faster inside Python. This is somewhat related to the time it takes to runrasa
on the command line, but I consider that to be a separate, more complex, issue.If we improve the load time of the
rasa
module, then people using our module directly will notice a slight speedup on their applications' start-up time, assuming they are not loading all sub-modules explicitly. If they are, the start-up time will not be affected.To measure import time, first I considered just measuring the time it took to run
python -c 'import rasa'
. However, this wouldn't have been too precise since that would also measure the start-up time for the Python interpreter and built-in modules. A much better option was to use a new feature added in Python 3.7,importtime
. Withpython -X importtime rasa
, one can get a detailed report of how much time it took for therasa
module to be imported, and all of its sub-modules as well. I wrote a short script which runs the profiling command N times, selects the relevant parts of the output, and averages the results. It can be used with any number N and any Python module.To find which modules it was most convenient to look at, I used tuna. Tuna will take the output of
python -X importtime rasa
and then display all imported modules and their import times on a flamegraph:(import times for
master
branch)To use tuna, I ran the following commands:
The changes I ended up applying to the base
master
branch were:nlu/test.py
, don't loadCRFEntityExtractor
early.constants.py
, remove the unused variableFALLBACK_CONFIG_PATH
and then remove thepkg_resources
import, which represented about 28% of the total loading time for therasa
module.cli/utils.py
, avoid loading thequestionary
library (unless type checking).training_data/loading.py
, avoid loading therequests
library until it's used.After applying these four changes, the total loading time for the
rasa
module went from an average of 0.545 seconds to 0.260 seconds (N=15). This represents approximately a 52.3% decrease in loading time. The absolute values in seconds may vary between hardware configurations and operating systems.This PR also removes the
rasa
attribute from therasa
module, which was pointing at itself (Which allowed doingrasa.rasa.rasa.rasa.rasa
...).Status (please check what you already did):
black
(please check Readme for instructions)