Findings in the Cython test suite
Hi, the new exception handling functions enabled a lot more tests in Cython's test suite, with still lots and lots of failures. I created tickets for a couple of them where it's clear that the problem is in PyPy/cpyext. The test results are collected here: https://sage.math.washington.edu:8091/hudson/view/dev-scoder/job/cython-scod... Here's the general log (which I find easier to read in this case): https://sage.math.washington.edu:8091/hudson/view/dev-scoder/job/cython-scod... For C compiler errors ("CompileError" exceptions) you'll have to look in the log at the place where the test was executed, but those are getting few. The test results are also recorded in the log, at the end (which, given the amount of failures, starts at about one third through the log). Some failures are due to wrong expectations in test code, e.g. we sometimes use the fact that small integers are cached in CPython. Others fail due to different exception messages in the doctests. Both can be handled on Cython side, given that CPython's own error messages aren't exactly carved in stone either. There is one major problem that accounts for the bulk of the test failures, somehow related to frame handling. You can tell by the huge amount of long traceback sequences that run into StackOverflowErrors and equivalent RuntimeErrors. When you look closer (e.g. repeatedly searching the log for the "classkwonlyargs" test), you will notice that the traceback refers to more than one test, i.e. the next doctest execution somehow picks up the frame of a previous test and continues from it. Funny enough, the frame leaking tests that have run (and failed) before the current one appear *below* the current test in the stack trace. This makes it likely that this is due to the way exception stack frames are constructed in Cython. They are only instantiated when an exception is being propagated, and then registered using PyTraceBack_Here(). It looks similarly likely that this is a bug in cpyext as it being due to problematic code in Cython. In case it's PyPy's fault, maybe there's something like an off-by-one when cleaning up the traceback frames somewhere? Another minor thing I noticed, PyPy seems to check the object type of keyword argument names at the caller side, whereas CPython does it as part of the function argument unpacking. Since Cython also does it on the function side and therefore expects its own error messages in the tests (which mimic CPython's, including the name of the function), these tests fail now (look for "keywords must be strings" in the test log). Not sure if it's worth doing anything about this - checking the type at call time isn't really wrong, just different. However, this is one of the cases where it would be nice if PyPy simply included the function name in the error message. The same applies to the "got multiple values for keyword argument" error. Apart from the frame handling bug, the remaining problems look minor enough to say that we are really close to a point where Cython on PyPy becomes usable. Stefan
Stefan Behnel, 02.04.2012 10:35:
There is one major problem that accounts for the bulk of the test failures, somehow related to frame handling. You can tell by the huge amount of long traceback sequences that run into StackOverflowErrors and equivalent RuntimeErrors.
I found one test that, when run all by itself, triggers a RuntimeError, boldly claiming that it reached the maximum recursion depth after a couple of calls. There do not seem to be any frames or tracebacks involved up to that point. The test executes a C representation of the following code, run as a doctest: ''' class A: def append(self, x): print u"appending" return x def test_append(L): """ >>> test_append(A()) """ print L.append(1) ''' (The background is that Cython optimistically replaces calls to obj.append() by code that assumes that obj is a list, so this tests the failure of that assumption) It gives me this error: """ Traceback (most recent call last): File ".../pypy/lib-python/2.7/doctest.py", line 1254, in __run compileflags, 1) in test.globs File "<doctest append.__test__.test_append (line 11)[1]>", line 1, in <module> _ = test_append(A()) File "append.pyx", line 34, in append.test_append (append.c:930) RuntimeError: maximum recursion depth exceeded """ The same happens when I call it manually, i.e. import append; append.test_append(append.A()) The basic C code that gets executed in the test_append() function is simply PyObject* m = PyObject_GetAttrString(L, "append"); r = PyObject_CallFunctionObjArgs(m, x, NULL); And that's where the error gets raised (returning r==NULL). Specifically, it does not enter into the actual append() method but fails before that, right at the call. I put a copy of the generated C file here: http://consulting.behnel.de/append.tgz Could someone shed some light on this? Stefan
2012/4/3 Stefan Behnel <stefan_ml@behnel.de>:
The basic C code that gets executed in the test_append() function is simply
PyObject* m = PyObject_GetAttrString(L, "append"); r = PyObject_CallFunctionObjArgs(m, x, NULL);
And that's where the error gets raised (returning r==NULL). Specifically, it does not enter into the actual append() method but fails before that, right at the call.
The issue is with CyFunctionType, which looks like a subclass of PyCFunction_Type. (it's a hack: normally this type is not subclassable, booo) L.append is such a CyFunctionType. Its tp_call slot is called, but this is defined to __Pyx_PyCFunction_Call which is #defined to PyObject_Call, which itself invokes the tp_call slot... A solution would be to access the "base" tp_call, the one that CPython exposes as PyCFunction_Call. Unfortunately cpyext only defines one tp_call shared by all types, one which simply delegates to self.__call__. This means that "calling the base slot" does not work very well with cpyext. There is a solution though, which I implemented a long time ago for the tp_setattro slot. It can be easily expanded to it's a hack: normally this type is not subclassable, booo) all slots but I'm a bit scared of the explosion of code this could generate. -- Amaury Forgeot d'Arc
On Tue, Apr 3, 2012 at 10:49 PM, Amaury Forgeot d'Arc <amauryfa@gmail.com>wrote:
2012/4/3 Stefan Behnel <stefan_ml@behnel.de>:
The basic C code that gets executed in the test_append() function is simply
PyObject* m = PyObject_GetAttrString(L, "append"); r = PyObject_CallFunctionObjArgs(m, x, NULL);
And that's where the error gets raised (returning r==NULL). Specifically, it does not enter into the actual append() method but fails before that, right at the call.
The issue is with CyFunctionType, which looks like a subclass of PyCFunction_Type. (it's a hack: normally this type is not subclassable, booo)
L.append is such a CyFunctionType. Its tp_call slot is called, but this is defined to __Pyx_PyCFunction_Call which is #defined to PyObject_Call, which itself invokes the tp_call slot...
A solution would be to access the "base" tp_call, the one that CPython exposes as PyCFunction_Call. Unfortunately cpyext only defines one tp_call shared by all types, one which simply delegates to self.__call__.
This means that "calling the base slot" does not work very well with cpyext. There is a solution though, which I implemented a long time ago for the tp_setattro slot. It can be easily expanded to it's a hack: normally this type is not subclassable, booo) all slots but I'm a bit scared of the explosion of code this could generate.
-- Amaury Forgeot d'Arc _______________________________________________ pypy-dev mailing list pypy-dev@python.org http://mail.python.org/mailman/listinfo/pypy-dev
Hey I would like to point out that all the assumptions like "this type is not subclassable" or "this field is read only" might work on cpython, but the JIT will make assumptions based on that and it'll stop working or produce very occasional segfaults Cheers, fijal
2012/4/3 Maciej Fijalkowski <fijall@gmail.com>:
I would like to point out that all the assumptions like "this type is not subclassable" or "this field is read only" might work on cpython, but the JIT will make assumptions based on that and it'll stop working or produce very occasional segfaults
Fortunately pypy objects are not exposed to C code, only a copy of fields and slots; and all transfers are explicitly written in RPython by cpyext. But no need to invoke the JIT here; with cpyext there can be only one object at a given address (!) and even if we had a PyCFunction_Call, it would not receive the correct interpreter object. This could be done of course, but with another hack. -- Amaury Forgeot d'Arc
Amaury Forgeot d'Arc, 03.04.2012 22:49:
2012/4/3 Stefan Behnel:
The basic C code that gets executed in the test_append() function is simply
PyObject* m = PyObject_GetAttrString(L, "append"); r = PyObject_CallFunctionObjArgs(m, x, NULL);
And that's where the error gets raised (returning r==NULL). Specifically, it does not enter into the actual append() method but fails before that, right at the call.
The issue is with CyFunctionType, which looks like a subclass of PyCFunction_Type.
Yes. It's required to make C implemented functions compatible with Python functions. Otherwise, they look and behave rather different in CPython, especially when used as methods (e.g. when assigned to class attributes after class creation). It's also faster for many things.
(it's a hack: normally this type is not subclassable, booo)
Yep, that's a problem - works in CPython, though...
L.append is such a CyFunctionType.
Ah, right - I should have tested with a class created at the Python level as well - that works.
Its tp_call slot is called, but this is defined to __Pyx_PyCFunction_Call which is #defined to PyObject_Call, which itself invokes the tp_call slot...
Interesting. Then that's the wrong thing to do in PyPy. I guess you just put it there in your original patch because PyPy doesn't expose PyCFunction_Call() and it seemed to be the obvious replacement.
A solution would be to access the "base" tp_call, the one that CPython exposes as PyCFunction_Call. Unfortunately cpyext only defines one tp_call shared by all types, one which simply delegates to self.__call__.
Makes sense for PyPy objects.
This means that "calling the base slot" does not work very well with cpyext. There is a solution though, which I implemented a long time ago for the tp_setattro slot. It can be easily expanded to [...] all slots but I'm a bit scared of the explosion of code this could generate.
I consider it a rather special case that Cython subtypes PyCFunction_Type, so a general solution may not be necessary. Is there anything we can do on Cython side? We control the type and its tp_call slot, after all. You could also implement a fake PyCFunction_Call function specifically for this purpose. Or even just a PyPyCFunction_Call(). OTOH, I had suggested before that PyPy could eventually learn about Cython's function type and optimise for it. We could implement PEP 362 to include C type information in the annotations of the signature object, and PyPy could use that to pack and execute a direct C call to the underlying C function. By caching the signature mapping, that could bring the PyPy-to-Cython call overhead down to that of ctypes. Stefan
2012/4/4 Stefan Behnel <stefan_ml@behnel.de>:
I consider it a rather special case that Cython subtypes PyCFunction_Type, so a general solution may not be necessary. Is there anything we can do on Cython side? We control the type and its tp_call slot, after all. You could also implement a fake PyCFunction_Call function specifically for this purpose. Or even just a PyPyCFunction_Call().
Yes, this is the hack I was referring to in another thread: cpyext could implement PyCFunction_Call by not taking a w_obj as argument (this would require building an interpreter object from its address, which would not work because there is already a CyFunction pointer at the same address) but work directly with the PyCFunctionObject members (m_ml, m_self and m_module) without considering they come from a PyObject-derived structure. But this approach is not scalable - it has to be hand-written each time, and will not work at all for most kind of objects. -- Amaury Forgeot d'Arc
2012/4/4 Stefan Behnel <stefan_ml@behnel.de>:
Its tp_call slot is called, but this is defined to __Pyx_PyCFunction_Call which is #defined to PyObject_Call, which itself invokes the tp_call slot...
Interesting. Then that's the wrong thing to do in PyPy. I guess you just put it there in your original patch because PyPy doesn't expose PyCFunction_Call() and it seemed to be the obvious replacement.
I put it there to make the code compile... I did not even realize how it was used.
A solution would be to access the "base" tp_call, the one that CPython exposes as PyCFunction_Call. Unfortunately cpyext only defines one tp_call shared by all types, one which simply delegates to self.__call__.
Makes sense for PyPy objects.
But not when those objects are subclassed: self.__call__ always accesses the most derived method, whereas a tp_call slot is supposed to point to the type being defined. -- Amaury Forgeot d'Arc
Amaury Forgeot d'Arc, 04.04.2012 09:27:
2012/4/4 Stefan Behnel:
Its tp_call slot is called, but this is defined to __Pyx_PyCFunction_Call which is #defined to PyObject_Call, which itself invokes the tp_call slot...
Interesting. Then that's the wrong thing to do in PyPy. I guess you just put it there in your original patch because PyPy doesn't expose PyCFunction_Call() and it seemed to be the obvious replacement.
I put it there to make the code compile... I did not even realize how it was used.
I found that cpyext already exports everything that is required to run the original PyCFunction_Call() function from CPython, so I just copied it over and it seems to work just fine. https://github.com/scoder/cython/commit/a286151e998d9b6d0888b63370fb5d4a1d70... That means that there's nothing to do on the PyPy side for this. It also means that we have the majority of tests passing or at least basically working now - only 80 out of the more than 2000 tests are currently failing. However, there are still crashers and there are also still failures that need further investigation. Crashers are best looked up in the log of the forked test runs: https://sage.math.washington.edu:8091/hudson/job/cython-scoder-pypy-nightly-... The results of completed test runs find their way into the web interface: https://sage.math.washington.edu:8091/hudson/job/cython-scoder-pypy-nightly/ Stefan
On 04/08/2012 07:27 AM Stefan Behnel wrote:
Crashers are best looked up in the log of the forked test runs:
https://sage.math.washington.edu:8091/hudson/job/cython-scoder-pypy-nightly-...
The results of completed test runs find their way into the web interface:
https://sage.math.washington.edu:8091/hudson/job/cython-scoder-pypy-nightly/
Stefan
Firefox warns strenuously that the certificate for your https://sage.math.washington.edu:8091/hudson/job/cython-scoder-pypy-nightly/ https site might not be trustable. Do you suggest just telling firefox to make an exception? I haven't made any exceptions before, so I have some resistance ... (not implying here that I wouldn't trust you personally ;-) What do pypyers do? Certificate serial 00:9C:BE:68:16:2B:98:E4:46 expired 05/22/2010 01:47:22 (05/22/2010 08:47:22 GMT) (apparently a month after it was created). Regards, Bengt Richter PS. Don't go to any extra trouble to make the info accessible for me -- I was just curious.
Bengt Richter, 08.04.2012 18:58:
On 04/08/2012 07:27 AM Stefan Behnel wrote:
Crashers are best looked up in the log of the forked test runs:
https://sage.math.washington.edu:8091/hudson/job/cython-scoder-pypy-nightly-...
The results of completed test runs find their way into the web interface:
https://sage.math.washington.edu:8091/hudson/job/cython-scoder-pypy-nightly/
Firefox warns strenuously that the certificate for your
https://sage.math.washington.edu:8091/hudson/job/cython-scoder-pypy-nightly/ https site might not be trustable.
Do you suggest just telling firefox to make an exception? I haven't made any exceptions before, so I have some resistance ...
Get used to it. :) Seriously, what do you care about the certificate when all you want is to read the page? It's not like you're going to send any sensitive data over. (And it won't do anything with that data even if you did.) I'm personally rather annoyed by the way Firefox treats broken/untrusted/self-signed certificates. It should warn when you actually do send data over, not right on connecting. In 99% of the cases, I really don't care what server I am connecting to.
(not implying here that I wouldn't trust you personally ;-) What do pypyers do?
Certificate serial 00:9C:BE:68:16:2B:98:E4:46 expired 05/22/2010 01:47:22 (05/22/2010 08:47:22 GMT) (apparently a month after it was created).
Interesting, guess we should fix that. Thanks for noting. Anyway, the only reason it's using a certificate is to let the core developers access it through an encrypted connection. The certificate itself is rather uninteresting to "normal" users. Stefan
participants (4)
-
Amaury Forgeot d'Arc -
Bengt Richter -
Maciej Fijalkowski -
Stefan Behnel