From e09edd170de966f97afb6b37dfff1cc0c4afc9ce Mon Sep 17 00:00:00 2001 From: Aditya Jain Date: Tue, 11 Aug 2026 03:03:46 -0700 Subject: [PATCH] Avoid repeating constant work on hot paths Two independent changes, both removing work that was being redone on every invocation or every file. 1. Defer docutils imports until help is rendered. ``awscli.help`` imported ``docutils.core`` and the html4css1/manpage writers at module scope, and ``awscli.topictags`` imported ``docutils.core``. Because ``awscli.customizations.commands`` subclasses ``HelpCommand``, that chain was pulled in during customization registration on every CLI invocation -- including ``pygments`` and ``PIL`` via the docutils rst directives -- even for commands that never render help. All of the uses are inside methods, so the imports move into them. Measured with 25 subprocess runs of ``aws --version`` per arm, alternating between arms three times to control for drift: before min 246-255ms after min 232-240ms so roughly 14ms (~6%) off every invocation. ``aws help``, ``aws s3 help``, ``aws ec2 describe-instances help`` and ``aws help topics`` were checked by hand, and ``TopicTagDB.scan`` still parses topic files. 2. Compile s3 --include/--exclude patterns once per transfer. ``Filter._match_pattern`` ran ``pattern.replace('/', os.sep)`` for every file for every pattern, and ``fnmatch.fnmatch`` normcased both the path and the pattern on each call before its cache lookup. All of that is constant across the transfer. Patterns are now separator-normalized and translated to a compiled regex once per source type, and the path is normcased once per file rather than once per pattern. Profiling 50k files against 6 patterns, only 0.19s of the original 1.68s was actual regex matching; the rest was the repeated setup. End to end: 100k files, 2 patterns 3.30 -> 2.12 us/file (-36%) 100k files, 6 patterns 9.94 -> 5.99 us/file (-40%) Behaviour is unchanged: a differential test comparing the old and new matching over 400 randomized pattern/path combinations reports no mismatches, and the two added tests pass against both implementations. Co-Authored-By: Claude Opus 5 --- .../next-release/enhancement-s3-49636.json | 5 ++ .../enhancement-startup-22805.json | 5 ++ awscli/clidriver.py | 13 +++-- awscli/customizations/s3/filters.py | 54 +++++++++++++++---- awscli/help.py | 16 ++++-- awscli/topictags.py | 7 +-- tests/unit/customizations/s3/test_filters.py | 27 ++++++++++ 7 files changed, 104 insertions(+), 23 deletions(-) create mode 100644 .changes/next-release/enhancement-s3-49636.json create mode 100644 .changes/next-release/enhancement-startup-22805.json 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()