diff --git a/.changes/next-release/enhancement-s3-49636.json b/.changes/next-release/enhancement-s3-49636.json new file mode 100644 index 000000000000..57308db368ab --- /dev/null +++ b/.changes/next-release/enhancement-s3-49636.json @@ -0,0 +1,5 @@ +{ + "type": "enhancement", + "category": "s3", + "description": "Compile ``--include``/``--exclude`` filter patterns once per transfer instead of re-normalizing and re-matching them for every file, reducing client-side filter evaluation time by roughly 40%." +} diff --git a/.changes/next-release/enhancement-startup-22805.json b/.changes/next-release/enhancement-startup-22805.json new file mode 100644 index 000000000000..bc5affc87e5a --- /dev/null +++ b/.changes/next-release/enhancement-startup-22805.json @@ -0,0 +1,5 @@ +{ + "type": "enhancement", + "category": "startup", + "description": "Defer importing ``docutils`` until help output is actually rendered. It was previously imported on every CLI invocation, along with ``pygments`` and ``PIL``, even for commands that never render help." +} diff --git a/awscli/clidriver.py b/awscli/clidriver.py index bc860a4a8bf2..91a37dc36097 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -65,11 +65,6 @@ ) from awscli.formatter import get_formatter from awscli.handlers_registry import MAIN_COMMAND_TABLE_OPS -from awscli.help import ( - OperationHelpCommand, - ProviderHelpCommand, - ServiceHelpCommand, -) from awscli.lazy_emitter import LazyInitEmitter from awscli.logger import ( disable_crt_logging, @@ -534,6 +529,10 @@ def _create_cli_argument(self, option_name, option_params): ) def create_help_command(self): + # Imported here because the help machinery pulls in docutils, + # which is only needed when help is actually requested. + from awscli.help import ProviderHelpCommand + cli_data = self._get_cli_data() return ProviderHelpCommand( self.session, @@ -769,6 +768,8 @@ def _add_lineage(self, command_table): command_obj.lineage = self.lineage + [command_obj] def create_help_command(self): + from awscli.help import ServiceHelpCommand + command_table = self._get_command_table() return ServiceHelpCommand( session=self.session, @@ -964,6 +965,8 @@ def __call__(self, args, parsed_globals): ) def create_help_command(self): + from awscli.help import OperationHelpCommand + return OperationHelpCommand( self._session, operation_model=self._operation_model, diff --git a/awscli/customizations/s3/filters.py b/awscli/customizations/s3/filters.py index 2c03f2a1eb5c..109eed951175 100644 --- a/awscli/customizations/s3/filters.py +++ b/awscli/customizations/s3/filters.py @@ -13,6 +13,7 @@ import fnmatch import logging import os +import re from awscli.customizations.s3.utils import split_s3_bucket_key @@ -94,6 +95,7 @@ def __init__(self, patterns, rootdir, dst_rootdir): self._original_patterns = patterns self.patterns = self._full_path_patterns(patterns, rootdir) self.dst_patterns = self._full_path_patterns(patterns, dst_rootdir) + self._compiled_cache = {} def _full_path_patterns(self, original_patterns, rootdir): # We need to transform the patterns into patterns that have @@ -119,14 +121,24 @@ def call(self, file_infos): before it. """ for file_info in file_infos: + patterns, dst_patterns = self._compiled_patterns( + file_info.src_type + ) file_path = file_info.src + # ``fnmatch.fnmatch`` normcases both of its arguments on every + # call. The pattern side is already normcased when it is + # compiled, so only the path has to be normcased here, and only + # once for all of the patterns. + norm_file_path = os.path.normcase(file_path) file_status = (file_info, True) - for pattern, dst_pattern in zip(self.patterns, self.dst_patterns): - current_file_status = self._match_pattern(pattern, file_info) + for pattern, dst_pattern in zip(patterns, dst_patterns): + current_file_status = self._match_pattern( + pattern, file_info, norm_file_path + ) if current_file_status is not None: file_status = current_file_status dst_current_file_status = self._match_pattern( - dst_pattern, file_info + dst_pattern, file_info, norm_file_path ) if dst_current_file_status is not None: file_status = dst_current_file_status @@ -138,15 +150,37 @@ def call(self, file_infos): if file_status[1]: yield file_info - def _match_pattern(self, pattern, file_info): + def _compiled_patterns(self, src_type): + # The patterns are fixed for the duration of a transfer, so the + # separator normalization and the fnmatch -> regex translation are + # done once per source type rather than once per file. + compiled = self._compiled_cache.get(src_type) + if compiled is None: + compiled = ( + self._compile_patterns(self.patterns, src_type), + self._compile_patterns(self.dst_patterns, src_type), + ) + self._compiled_cache[src_type] = compiled + return compiled + + def _compile_patterns(self, patterns, src_type): + compiled = [] + for pattern_type, pattern in patterns: + if src_type == 'local': + path_pattern = pattern.replace('/', os.sep) + else: + path_pattern = pattern.replace(os.sep, '/') + regex = re.compile( + fnmatch.translate(os.path.normcase(path_pattern)) + ) + compiled.append((pattern_type, path_pattern, regex)) + return compiled + + def _match_pattern(self, pattern, file_info, norm_file_path): file_status = None file_path = file_info.src - pattern_type = pattern[0] - if file_info.src_type == 'local': - path_pattern = pattern[1].replace('/', os.sep) - else: - path_pattern = pattern[1].replace(os.sep, '/') - is_match = fnmatch.fnmatch(file_path, path_pattern) + pattern_type, path_pattern, regex = pattern + is_match = regex.match(norm_file_path) is not None if is_match and pattern_type == 'include': file_status = (file_info, True) LOG.debug("%s matched include filter: %s", file_path, path_pattern) diff --git a/awscli/help.py b/awscli/help.py index fa6a3099a41b..7c5b2975df5d 100644 --- a/awscli/help.py +++ b/awscli/help.py @@ -20,11 +20,6 @@ from subprocess import PIPE, Popen from botocore.exceptions import ProfileNotFound -from docutils.core import publish_string -from docutils.writers import ( - html4css1, - manpage, -) from awscli import ( _DEFAULT_BASE_REMOTE_URL, @@ -239,6 +234,9 @@ class PosixHelpRenderer(PosixPagingHelpRenderer): """ def _convert_doc_content(self, contents): + from docutils.core import publish_string + from docutils.writers import manpage + settings_overrides = self._DEFAULT_DOCUTILS_SETTINGS_OVERRIDES.copy() settings_overrides["report_level"] = 3 man_contents = publish_string( @@ -265,6 +263,9 @@ class PosixBrowserHelpRenderer(BrowserHelpRenderer): """ def _convert_doc_content(self, contents): + from docutils.core import publish_string + from docutils.writers import manpage + settings_overrides = self._DEFAULT_DOCUTILS_SETTINGS_OVERRIDES.copy() settings_overrides["report_level"] = 3 man_contents = publish_string( @@ -310,6 +311,8 @@ class WindowsHelpRenderer(WindowsPagingHelpRenderer): """Render help content on a Windows platform.""" def _convert_doc_content(self, contents): + from docutils.core import publish_string + text_output = publish_string( contents, writer=TextWriter(), @@ -322,6 +325,9 @@ class WindowsBrowserHelpRenderer(BrowserHelpRenderer): """Render help content in the browser on a Windows platform.""" def _convert_doc_content(self, contents): + from docutils.core import publish_string + from docutils.writers import html4css1 + text_output = publish_string( contents, writer=html4css1.Writer(), diff --git a/awscli/topictags.py b/awscli/topictags.py index dfd9f5a7f505..6b001ce9f21f 100644 --- a/awscli/topictags.py +++ b/awscli/topictags.py @@ -22,8 +22,6 @@ import json import os -import docutils.core - class TopicTagDB: """This class acts like a database for the tags of all available topics. @@ -182,7 +180,10 @@ def _find_topic_name(self, topic_src_file): def _add_tag_and_values_from_content(self, topic_name, content): # Retrieves tags and values and adds from content of topic file - # to the dictionary. + # to the dictionary. Imported here because docutils is only + # needed when the topic index is (re)generated or queried. + import docutils.core + doctree = docutils.core.publish_doctree(content).asdom() fields = doctree.getElementsByTagName('field') for field in fields: diff --git a/tests/unit/customizations/s3/test_filters.py b/tests/unit/customizations/s3/test_filters.py index 77024589e1b5..cc5296cf5370 100644 --- a/tests/unit/customizations/s3/test_filters.py +++ b/tests/unit/customizations/s3/test_filters.py @@ -244,6 +244,33 @@ def test_create_filter_s3_to_s3(self): for filtered_file in filtered: self.assertFalse('.txt' in filtered_file.src) + def test_reuses_filter_across_src_types(self): + # Patterns are compiled per source type and cached, so a filter + # reused across source types has to keep giving each type its own + # separator normalization rather than the first one it saw. + exclude_filter = self.create_filter([['exclude', '*.txt']]) + for _ in range(2): + local = list(exclude_filter.call(self.local_files)) + self.assertEqual( + [os.path.basename(f.src) for f in local], + ['test.jpg', 'test.jpg'], + ) + s3 = list(exclude_filter.call(self.s3_files)) + self.assertEqual( + [f.src for f in s3], ['bucket/test.jpg', 'bucket/key/test.jpg'] + ) + + def test_repeated_calls_are_stable(self): + # The compiled-pattern cache must not accumulate or mutate state + # between calls. + include_filter = self.create_filter( + [['exclude', '*'], ['include', '*.jpg']] + ) + first = [f.src for f in include_filter.call(self.local_files)] + second = [f.src for f in include_filter.call(self.local_files)] + self.assertEqual(first, second) + self.assertEqual(len(first), 2) + if __name__ == "__main__": unittest.main()