-
Notifications
You must be signed in to change notification settings - Fork 126
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
Cythonize call loop #104
Cythonize call loop #104
Conversation
Apparently |
please dont commit the c sources to the git repo, also not exactly what i had in mind for cythonizing :( |
6a64be8
to
c745962
Compare
Done.
We can always make it better 👍. |
c745962
to
080ee0e
Compare
the biggest problem i see is we broke sane pypy support - c extensions are just not the way to go) |
|
||
[testenv:benchmark] | ||
commands=py.test {posargs:testing/benchmark.py} | ||
commands=py.test testing/benchmark.py {posargs} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like this invocation got accidentally plucked incorrectly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually did this on purpose so I could pass additional args without having to pass testing/benchmark
every time. Should I put it back?
So what did you mean by cythonize then in our previous discussions? Also, with regard to We can keep the original pure python implementation as well and then only import the |
i recall that cython had a mode where the code was pure python * a separate file with annotation |
I'm not sure the speed up is worth it; I have the feeling that the bottleneck in applications using |
@nicoddemus the speed of the fixture mechanism and the hook calling mechanism is actually critical in some bits the switch from kwargs to positional args already did a lot to safe time, and i suspect there is more to gain i recall one of the reasons why pytest doesn't integrate xunit with fixtures is, that the fixtures where too expensive |
Probably because the initial implementation always added a But I digress, if indeed this is a bottleneck let's see if we can squeeze some more performance from it. 👍 |
Yes there is pure python mode though speed gains are much more negligible. |
080ee0e
to
52044ca
Compare
52044ca
to
792d68e
Compare
Cythonize `pluggy.callers._multicall` which gives around a 2x speedup according to the benchmark suite. Transform the `pluggy.callers` module in a new package and move utils and the legacy call loop into separate modules.
When cython is installed always rebuild the C sources, when not installed use whatever C sources were included in the sdist.
792d68e
to
4f92948
Compare
Is there any project where calling hooks is shown to be a speed issue? |
@hpk42 I totally agree. Forgive my premature cythonization ;) |
Consider this a working initial draft to begin our foray into
cython
-land.The benchmark suite shows a x2 speedup with this cythonized version of
_multicall
👍