Skip to content

refactor(markers): use match statements for marker dispatch - #1370

Open
henryiii wants to merge 1 commit into
pypa:mainfrom
henryiii:henryiii/refactor/match
Open

refactor(markers): use match statements for marker dispatch#1370
henryiii wants to merge 1 commit into
pypa:mainfrom
henryiii:henryiii/refactor/match

Conversation

@henryiii

@henryiii henryiii commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

These are cases where pattern matching is natural (AST-like work is what it was
designed for).

Assisted-by: ClaudeCode:claude-fable-5

Convert the isinstance/elif chains in _evaluate_markers,
_format_marker, and _normalize_extras to match statements, in the
same spirit as the tags.py conversion.

The marker-item case now uses class patterns, so a malformed tuple
raises the existing TypeError instead of failing at unpack time.

Assisted-by: ClaudeCode:claude-fable-5
@notatallshaw

notatallshaw commented Aug 8, 2026

Copy link
Copy Markdown
Member

Slower:

| Change   | Before [3c6e2aab]    | After [50601022]    |   Ratio | Benchmark (Parameter)                                                                                  |
|----------|----------------------|---------------------|---------|--------------------------------------------------------------------------------------------------------|
|          | 3.99±0.03ms          | 4.14±0.01ms         |    1.04 | markers.TimeMarkerSuite.time_constructor [runnervmvrwv9/virtualenv-py3.10-PYTHONHASHSEED0]             |
|          | 2.90±0.02ms          | 3.05±0.05ms         |    1.05 | markers.TimeMarkerSuite.time_constructor [runnervmvrwv9/virtualenv-py3.11-PYTHONHASHSEED0]             |
|          | 3.40±0.03ms          | 3.51±0.03ms         |    1.03 | markers.TimeMarkerSuite.time_constructor [runnervmvrwv9/virtualenv-py3.12-PYTHONHASHSEED0]             |
|          | 3.35±0.02ms          | 3.48±0.03ms         |    1.04 | markers.TimeMarkerSuite.time_constructor [runnervmvrwv9/virtualenv-py3.13-PYTHONHASHSEED0]             |
|          | 3.27±0.03ms          | 3.37±0.03ms         |    1.03 | markers.TimeMarkerSuite.time_constructor [runnervmvrwv9/virtualenv-py3.14-PYTHONHASHSEED0]             |
|          | 786±8μs              | 846±4μs             |    1.08 | markers.TimeMarkerSuite.time_evaluate [runnervmvrwv9/virtualenv-py3.10-PYTHONHASHSEED0]                |
| +        | 535±5μs              | 610±5μs             |    1.14 | markers.TimeMarkerSuite.time_evaluate [runnervmvrwv9/virtualenv-py3.11-PYTHONHASHSEED0]                |
| +        | 630±5μs              | 728±9μs             |    1.16 | markers.TimeMarkerSuite.time_evaluate [runnervmvrwv9/virtualenv-py3.12-PYTHONHASHSEED0]                |
| +        | 592±5μs              | 685±10μs            |    1.16 | markers.TimeMarkerSuite.time_evaluate [runnervmvrwv9/virtualenv-py3.13-PYTHONHASHSEED0]                |
| +        | 546±5μs              | 643±7μs             |    1.18 | markers.TimeMarkerSuite.time_evaluate [runnervmvrwv9/virtualenv-py3.14-PYTHONHASHSEED0]                |

I did some of my own benchmarking and locally found significantly worse slowdowns in specific cases:

MATCH_CLASS replaces specialized isinstance calls. _format_marker goes from 98 to 123 ops, _evaluate_markers from 141 to 210. Over a sample of real markers on 3.12: str(Marker) +53%, Marker.evaluate +11%; on 3.14 +17% and +12%. Isolated, _format_marker on the [[...]] unwrap path is 1.7x slower on 3.12 and 2.9x on six levels of nesting.

@henryiii

henryiii commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Ahh, didn't think about this being on a hot path. I know several match statements are slower than the matching if statements, often because they do more checks: [a, b] is slower because it verifies that this is a Sequence, while the if version usually just burns an arbitrary iterator if it's given one. This looks pretty equivalent and I don't see any of the known slow cases, so I'm slightly sad it's slower, but not very surprised. I do wonder if it's extra checks, and which ones it might be.

Looks like it was useful anyway in finding some ways to make this faster, so not wasted. :)

@notatallshaw

Copy link
Copy Markdown
Member

match is not well optimized in general, it should be better in 3.15, various optimizations have gone in. Markers do become a hot path when doing universal resolution.

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.

2 participants