Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change METH_VARARGS to METH_O; much faster parsing 🎉😅 #130

Merged
merged 1 commit into from
Nov 22, 2022

Conversation

movermeyer
Copy link
Collaborator

@movermeyer movermeyer commented Nov 19, 2022

What are you trying to accomplish?

When I saw that cPython's 3.11 datetime.fromisoformat was outperforming ciso8601.parse_datetime (without time zone information), I went looking to see what they were doing differently.

Their methods were defined as METH_O (i.e., method with single positional parameter), while ours were using METH_VARARGS (i.e., method with multiple positional parameters). It turns out there is a large performance benefit when you:

a) No longer have to call PyArg_ParseTuple
b) Allow Python to optimize based on knowing there is just a single parameter

What approach did you choose and why?

I changed each of the parser methods from METH_VARGS to METH_O.
This changed their C signatures, and so I had to plumb through the changes.

What should reviewers focus on?

...

The impact of these changes

Embarrassingly faster parsing times:

Module Python 3.11 Python 3.10 Python 3.9 Python 3.8 Python 3.7 Python 2.7 Relative Slowdown
ciso8601 (this PR) 84 nsec 109 nsec 103 nsec 102 nsec 107 nsec 122 nsec N/A
datetime.fromisoformat 109 nsec N/A N/A N/A N/A N/A 1.3x
ciso8601 v2.2.0 130 nsec 154 nsec 147 nsec 158 nsec 158 nsec 168 nsec 1.5x

@movermeyer movermeyer changed the title Change METH_VARARGS to METH_O Change METH_VARARGS to METH_O; much faster parsing 🎉😅 Nov 19, 2022
@movermeyer movermeyer marked this pull request as ready for review November 19, 2022 03:56
@movermeyer movermeyer added the performance Things that improve our performance label Nov 21, 2022
@AlecRosenbaum AlecRosenbaum removed the request for review from thomasst November 22, 2022 18:13
@movermeyer movermeyer merged commit ab4f91b into master Nov 22, 2022
@movermeyer movermeyer deleted the movemeyer/better_method_flags branch December 21, 2022 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Things that improve our performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants