Skip to content

Defer docutils imports and precompile s3 include/exclude patterns - #10545

Closed
Adityaj0 wants to merge 1 commit into
aws:v2from
Adityaj0:perf/lazy-doc-imports-and-filter-precompile
Closed

Defer docutils imports and precompile s3 include/exclude patterns#10545
Adityaj0 wants to merge 1 commit into
aws:v2from
Adityaj0:perf/lazy-doc-imports-and-filter-precompile

Conversation

@Adityaj0

Copy link
Copy Markdown

Two independent changes, both removing constant work that was being redone on every invocation or every file. No behaviour change in either.

1. Defer docutils imports until help is actually rendered

awscli/help.py imported docutils.core and the html4css1/manpage writers at module scope, and awscli/topictags.py imported docutils.core. Because awscli/customizations/commands.py subclasses HelpCommand, that whole 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.

Every use is inside a method, so the imports move into them.

Measured with 25 subprocess runs of aws --version per arm, alternating between arms three times to control for machine drift:

before   min 246-255ms
after    min 232-240ms

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, and TopicTagDB.scan still parses topic source 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 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:

                          ncalls  tottime
_match_pattern            600000    0.388
fnmatch.fnmatch           600000    0.274
fnmatchcase               600000    0.202
re.Pattern.match          600000    0.192   <- the only necessary work
posixpath.normcase       1200000    0.166
str.replace               600000    0.057

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%)

Worth being clear about the scope of that number: this is client-side filter evaluation only. On a real aws s3 sync the 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_patterns keep 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.
  • Behaviour equivalence for the filter change was checked with a differential test running the old and new matching over 400 randomized pattern/path combinations (mixed local/s3 source types, spaces, case variation, glob metacharacters in filenames): 0 mismatches.
  • Two tests added covering filter reuse across source types and stability across repeated calls. Unlike a bug fix, these pass against both the old and new implementation by design — they are refactor guards, not proof of the change.
  • ruff output on the touched files is unchanged from baseline.

I did not run the full tests/functional suite 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.

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>
@Adityaj0
Adityaj0 requested a review from a team as a code owner August 11, 2026 10:08
@aemous

aemous commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

@Adityaj0 For ease of rollback and making the review process easier, can you separate the two independent changes into two separate pull requests?

@Adityaj0

Copy link
Copy Markdown
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.

@Adityaj0 Adityaj0 closed this Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants