Skip to content

Compile s3 --include/--exclude patterns once per transfer - #10552

Open
Adityaj0 wants to merge 1 commit into
aws:v2from
Adityaj0:perf/s3-filter-precompile
Open

Compile s3 --include/--exclude patterns once per transfer#10552
Adityaj0 wants to merge 1 commit into
aws:v2from
Adityaj0:perf/s3-filter-precompile

Conversation

@Adityaj0

Copy link
Copy Markdown

Split out of #10545 at @aemous's request — this is half 2 of 2. The other half is #10551 (deferring the docutils imports); the two are independent and touch disjoint files.

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: 996 passed.
  • Behaviour equivalence 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.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

``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.

    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.

``self.patterns`` / ``self.dst_patterns`` keep their existing shape and
contents, since tests and external callers read them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant