Skip to content

Conversation

@donbarbos
Copy link
Contributor

@donbarbosdonbarbos commented Feb 18, 2025

timeit benchmark with my script:

inspect_bench.py:

importreimporttimeitfromtypingimportList, Dict, Any, Uniondefold_format_annotation(annotation): ifgetattr(annotation, "__module__", None) =="typing": defrepl(match): text=match.group() returntext.removeprefix("typing.") returnre.sub(r"[\w\.]+", repl, repr(annotation)) returnrepr(annotation) defnew_format_annotation(annotation): ifgetattr(annotation, "__module__", None) =="typing": return (repr(annotation) .replace(".typing.", "@TYPING@") .replace("typing.", "") .replace("@TYPING@", ".typing.")) returnrepr(annotation) old_time=timeit.timeit(lambda: old_format_annotation(Union[List[str], Dict[str, Any]]), number=100_000) new_time=timeit.timeit(lambda: new_format_annotation(Union[List[str], Dict[str, Any]]), number=100_000) print(f"Old version (re.sub): {old_time:.6f}s") print(f"New version (.replace()): {new_time:.6f}s") print(f"Difference: {old_time/new_time:.2f}x")

Result: 2.0s -> 1.18s = x1.74 as fast

$ ./python -B inspect_bench.py Old version (re.sub): 2.043900s New version (.replace()): 1.175867s Difference: 1.74x

✔️ This was also the only use of re in module so i got rid of the import

@donbarbosdonbarbos changed the title Improve speed of inspect.formatannotation by replacing regh-130167: Improve speed of inspect.formatannotation by replacing reFeb 18, 2025
Copy link
Member

@AA-TurnerAA-Turner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm less sure this is worth it, the replacement isn't clearly better maintenence-wise.

What do the benchmarks look like if you extract repl to a module-level _formatannotation_repl and use .sub() on a pre-compiled pattern?

A

@donbarbos
Copy link
ContributorAuthor

donbarbos commented May 1, 2025

sorry, I forgot how python was compiled then :( but I'm pretty sure I compiled it with gcc last time

so I reran the test with the following config args '--enable-optimizations' '--with-lto' 'CC=/usr/bin/clang-18' and got a new result for my bench:

Old version (re.sub): 0.982520s New version (.replace()): 0.701049s Difference: 1.40x # sometimes it's 1.35x

and if we rewrite old version of benchmark as you suggested:

_pattern=re.compile(r"[\w\.]+") def_repl(match): text=match.group() returntext.removeprefix("typing.") defold_format_annotation(annotation): ifgetattr(annotation, "__module__", None) =="typing": returnre.sub(_pattern, _repl, repr(annotation)) returnrepr(annotation)

we will get a bigger difference:

Old version (re.sub): 1.027774s New version (.replace()): 0.698902s Difference: 1.47x # the result can drop to 1.40

P.S.: another reason i like the PR version is because we get rid of the re import that is used once here

@Engelbarts
Copy link

Out of curiosity, why use the little replace dance in the refactored code instead of just return repr(annotation).removeprefix("typing.")?

@charmander
Copy link

charmander commented Nov 27, 2025

Out of curiosity, why use the little replace dance in the refactored code instead of just return repr(annotation).removeprefix("typing.")?

- Union[typing.List[str], typing.Dict[str, typing.Any]]+ Union[List[str], Dict[str, Any]]

The replace dance is pretty messy, though, and doesn’t preserve behaviour for footyping.Bar – not that applying a regex to repr output is perfectly reliable to begin with (def foo() -> Literal["typing."]: ...).

For what it’s worth:

pat=re.compile(r"\b(?<!\.)typing\.") pat.sub("", repr(annotation))

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@donbarbos@Engelbarts@charmander@hugovk@AA-Turner