Defer docutils imports and precompile s3 include/exclude patterns - #10545
Closed
Adityaj0 wants to merge 1 commit into
Closed
Defer docutils imports and precompile s3 include/exclude patterns#10545Adityaj0 wants to merge 1 commit into
Adityaj0 wants to merge 1 commit into
Conversation
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 <noreply@anthropic.com>
Contributor
|
@Adityaj0 For ease of rollback and making the review process easier, can you separate the two independent changes into two separate pull requests? |
This was referenced Aug 13, 2026
Author
|
Thanks @aemous — split as requested:
Each has its own changelog entry, and the two touch disjoint files, so they can be reviewed, merged and rolled back independently. I verified the split is lossless: every changed line of the commit here appears exactly once across the two branches, and both diffs are against the same base as this PR. Each branch's suites were re-run on its own — 11967 passed / 3 skipped for #10551, 996 passed for #10552. Closing this one in favour of those two. |
This file contains hidden or 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
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.
Two independent changes, both removing constant work that was being redone on every invocation or every file. No behaviour change in either.
1. Defer
docutilsimports until help is actually renderedawscli/help.pyimporteddocutils.coreand thehtml4css1/manpagewriters at module scope, andawscli/topictags.pyimporteddocutils.core. Becauseawscli/customizations/commands.pysubclassesHelpCommand, that whole chain was pulled in during customization registration on every CLI invocation — includingpygmentsandPILvia the docutils rst directives — even for commands that never render help.Every use is inside a method, so the imports move into them.
Measured with 25 subprocess runs of
aws --versionper arm, alternating between arms three times to control for machine drift:About 14ms (~6%) off every invocation. I'm quoting min-of-25 rather than the mean because run-to-run noise on this machine is ±20ms, which is larger than the effect — sequential (non-interleaved) runs initially suggested a bigger win than the interleaved A/B supports.
Checked by hand that help still renders:
aws help,aws s3 help,aws ec2 describe-instances help,aws help topics, andTopicTagDB.scanstill parses topic source files.2. Compile s3
--include/--excludepatterns once per transferFilter._match_patternranpattern.replace('/', os.sep)for every file for every pattern, andfnmatch.fnmatchnormcased both the path and the pattern on every call before its internal cache lookup. All of that is constant across a transfer.Patterns are now separator-normalized and translated to a compiled regex once per source type, and the path is normcased once per file instead of once per pattern.
Profiling 50k files against 6 patterns showed only 0.19s of the original 1.68s was actual regex matching — the rest was repeated setup:
End to end:
Worth being clear about the scope of that number: this is client-side filter evaluation only. On a real
aws s3 syncthe network dominates, so this matters most for large listings where most files are filtered out — it is not a 40% reduction in overall sync time.self.patterns/self.dst_patternskeep their existing shape and contents, since tests and external callers read them.Testing
tests/unit/customizations/s3,tests/functional/s3,tests/functional/docs,test_help.py,test_topictags.py,test_clidriver.py: 12963 passed, 3 skipped.local/s3source types, spaces, case variation, glob metacharacters in filenames): 0 mismatches.ruffoutput on the touched files is unchanged from baseline.I did not run the full
tests/functionalsuite locally.Happy to split these into two PRs if you'd prefer them reviewed separately.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.