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

{Core} Delay import #13843

Merged
merged 2 commits into from
Jun 8, 2020
Merged

{Core} Delay import #13843

merged 2 commits into from
Jun 8, 2020

Conversation

jiasli
Copy link
Member

@jiasli jiasli commented Jun 5, 2020

Description

Previously during CLI initialization, importing these modules are very time-consuming, but actually often unnecessary:

  • msrest
  • azure.core
  • azure.mgmt.core
  • azure.common

This PR delays the import until these modules are actually used.

The effect is remarkable: For az verion -h, the import time dropped from 1.755s to 0.648s.

Before:
image

After:
image

Testing Guide

az verion -h --verbose

Generate the import time graph with

python -X importtime -m azure.cli version -h 2>perf.log
tuna perf.log

Comment on lines +851 to +855
except Exception as ex:
# Delay the import and mimic an exception handler
from msrest.exceptions import ValidationError
if isinstance(ex, ValidationError):
logger.debug('Validation error in %s.', str(validator))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mimic the behavior of

def handle_exception(ex): # pylint: disable=too-many-return-statements
# For error code, follow guidelines at https://docs.python.org/2/library/sys.html#sys.exit,
from jmespath.exceptions import JMESPathTypeError
from msrestazure.azure_exceptions import CloudError
from msrest.exceptions import HttpOperationError, ValidationError, ClientRequestError
from azure.cli.core.azlogging import CommandLoggerContext

@@ -305,6 +304,7 @@ def assemble_json(ids):
if full_id_list:
setattr(namespace, '_ids', full_id_list)

from azure.mgmt.core.tools import parse_resource_id, is_valid_resource_id
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migrate from msrestazure.tools to azure.mgmt.core.tools.

@haroldrandom
Copy link
Contributor

what's relationship with this PR Performance Boost ?

@jiasli
Copy link
Member Author

jiasli commented Jun 5, 2020

what's relationship with this PR Performance Boost ?

This is extracted from #13294 to make reviewing easier.

Copy link
Contributor

@haroldrandom haroldrandom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Learnt a lot!

@jiasli jiasli self-assigned this Jun 5, 2020
Copy link
Member

@jsntcy jsntcy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

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

Successfully merging this pull request may close these issues.

3 participants