-
Notifications
You must be signed in to change notification settings - Fork 938
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
gevent._greenlet.pxd no longer provided in wheel (1.5 regression) #1568
Comments
The pxd files were never a documented interface. They're an internal implementation detail, subject to change at any time, even between minor releases, to the point where they might even come and go over time. That's especially true of the ones that have been added recently such as greenlet, queue, etc. It was an accident that they were ever in a distribution to start with, and a packaging update in 1.5a4 removed them. I'd really like to avoid having to support them. Can you just not use them, like you don't on PyPy? |
@jamadden thanks for feedback. While I understand the desire to minimize the surface of provided public API, there are reasons on pygolang side to prefer Cython interface compared to pure Python: the ultimate goal of pygolang is to provide its service in completely Besides
Regarding benchmarking at Python vs Cython-level - it is just that the benchmarks currently exist only in Python form. When used at Cython level (pygolang can be already used like this even with Said that isn't gevent/_greenlet.pxd, that we are talking about, providing Cython-level annotations for public gevent/greenlet.py? If i understand correctly the At least when using Once again, I understand the desire to avoid exposing things as API to users, but given the above rationales and that it already used to work out of the box for at least ~ 1 year, maybe we could do something that it is ok for both gevent and pygolang? Thanks beforehand, P.S. I'm considering to drop PyPy support every time I hit a bug in its runtime, but so far it got survived. (*) performance tests were done with caring about measurement stability. |
Let me try to be more clear. The definitions in the .pxd files are not public, documented or supported. They may change at any time. And when they do, any project that had consumed them must be recompiled or it is highly likely to be broken. This could be the easy thing where an extension module won't even load because of unresolved symbols, but it's at least as likely to simply crash. The way that Cython lays out a class's vtable is fragile from the ABI point of view, and methods invoked through it use hard-coded offsets; any change in that vtable invalidates all previously compiled code. Calling a method is likely to jump to code expecting different parameters or jump into garbage. If you wish to take that risk for the sake of performance, and wish to restrict users to exactly the version of gevent that your extension was compiled with, that's totally fine. But I don't want to encourage that because I don't have the time to deal with the inevitable bug reports; I choose to discourage that by not distributing the pxd files. Perhaps you can copy the definitions you wish to use ("vendor" them)? |
Thanks @jamadden, your message is clear. Even it is a pity to hear, I can understand you. |
Starting from gevent >= 1.5 '*.pxd' files for gevent API are no longer provided, at least in released gevent wheels. This broke pygolang: Error compiling Cython file: ------------------------------------------------------------ ... # Gevent runtime uses gevent's greenlets and semaphores. # When sema.acquire() blocks, gevent switches us from current to another greenlet. IF not PYPY: from gevent._greenlet cimport Greenlet ^ ------------------------------------------------------------ golang/runtime/_runtime_gevent.pyx:28:4: 'gevent/_greenlet.pxd' not found Since gevent upstream refuses to restore Cython level access[1], let's fix the build by using gevent bits via Python-level. Even when used via py import gevent-1.5 brings speed improvement compared to gevent-1.4 (used via cimport): (on i7@2.6GHz, gevent runtime) gevent-1.4 gevent-1.5 (cimport) (py import) name old time/op new time/op delta pyx_select_nogil 9.47µs ± 0% 8.74µs ± 0% -7.70% (p=0.000 n=10+9) pyx_go_nogil 14.3µs ± 1% 12.0µs ± 1% -16.52% (p=0.000 n=10+10) pyx_chan_nogil 7.10µs ± 1% 6.32µs ± 1% -10.89% (p=0.000 n=10+10) go 16.0µs ± 2% 13.4µs ± 1% -16.37% (p=0.000 n=10+10) chan 7.50µs ± 0% 6.79µs ± 0% -9.53% (p=0.000 n=10+10) select 10.8µs ± 1% 10.0µs ± 1% -6.78% (p=0.000 n=10+10) Using gevent-1.5 could have been even faster via cimport (it is still possible to compile and test against gevent installed in development mode via `pip install -e` because pxd files are there in gevent worktree and tarball): gevent-1.5 gevent-1.5 (py import) (cimport) name old time/op new time/op delta pyx_select_nogil 8.74µs ± 0% 7.90µs ± 1% -9.60% (p=0.000 n=9+10) pyx_go_nogil 12.0µs ± 1% 11.2µs ± 2% -6.35% (p=0.000 n=10+10) pyx_chan_nogil 6.32µs ± 1% 5.89µs ± 0% -6.80% (p=0.000 n=10+9) go 13.4µs ± 1% 12.4µs ± 1% -7.54% (p=0.000 n=10+9) chan 6.79µs ± 0% 6.42µs ± 0% -5.47% (p=0.000 n=10+10) select 10.0µs ± 1% 9.4µs ± 1% -6.39% (p=0.000 n=10+10) but we cannot use cimport to access gevent-1.5 universally, since pxd are not shipped in gevent wheel releases. In the future we might want to change plain version check into compile time check whether gevent/_greenlet.pxd is actually present or not and use faster access if yes. Requesting gevent to be installed in non-binary form might be also an option worth trying. However plain version check should be ok for now. [1] gevent/gevent#1568
Argh! I myself spent time struggling with the binary compatibility issue today. I made what I thought was a trivial change (for speed!) in one pxd file, rebuilt, and ran tests. They passed. Except one that hardcore crashed the interpreter in a I truly sympathize with the desire for maximum performance, but the safety just isn’t there. It took me longer than I want to admit to figure out what was happening, and that was in a fully controlled, single project environment with debug flags enabled. As these internal optimizations are obviously still in flux and likely to stay that way I can’t in good conscience foist these types of issues onto users or other maintainers. Thank you for understanding. |
@jamadden, thanks for feedback. I understand you. With the given state of lack of reliable Cython dependency tracking, and gevent pxd bits being still in flux, it probably indeed makes more sense not to expose pxd API to users in general, at least for now. The problem of unreliable dependency tracking on Cython side is old and, sadly, well-known. I myself am used to do Even if we are disabling pxd interface for gevent for now, I have to share that in my company gevent is regularly criticized for being slow. It is good to see gevent increasingly improving from release to release (thanks for your hard work on this!), but loosing any part of potential speedup is pity, since the competition is in order of magnitude, not percents: https://nexedi.com/NXD-Document.Blog.UVLoop.Python.Benchmark In my view, compared to other ways, gevent/greenlet/greenstack/... provide big advantage by preserving backward compatibility for existing codebases. This gives ability to switch existing programs into lightweight-threads mode without major rewrite and to further enhance them incrementally instead of rewriting everything in one go. Once again, I understand current situation and the rationale for decision to keep pxd in private. Kirill |
Is it? It hasn't failed me in years, so could you describe your setup and what doesn't work with it in a Cython ticket? |
@scoder, thanks for feedback. I think what @jamadden hit in #1568 (comment) is cython/cython#1428. For non-pure-python mode there is also a lot of confusion of whether to use See also cython/cython#3208. |
Ah, yes. |
Linux deco 4.19.0-8-amd64 #1 SMP Debian 4.19.98-1 (2020-01-26) x86_64 GNU/Linux
Description:
Hello up there. I've tried to
pip install pygolang
and got compilation failure with error saying that'gevent/_greenlet.pxd' not found
. I clearly remember this used to work out of the box on a fresh virtualenv. Investigating further it indeed confirmed that pygolang installation succeeds with gevent-1.4 and fails with recently released gevent-1.5 (btw thanks for the release). It boiled down to the following: gevent-1.4 ships_greenlet.pxd
in its wheel, while gevent-1.5 no longer ships it:gevent-1.4.0-cp37-cp37m-manylinux1_x86_64.whl
gevent-1.5.0-cp37-cp37m-manylinux2010_x86_64.whl
It still works fine if I
pip install -e
gevent-1.5 in development mode from a git checkout. It breaks only if gevent is installed from a released wheel. Unfortunately the "released gevent install" is the default mode for most of the users.Here is the place in pygolang that depends on
gevent._greenlet.pxd
and other gevent cython bits:https://lab.nexedi.com/kirr/pygolang/blob/874371e2/golang/runtime/_runtime_gevent.pyx#L28-30
Is it please possible to restore providing the
pxd
s to users?Thanks beforehand,
Kirill
P.S.
`pip install pygolang` output
The text was updated successfully, but these errors were encountered: