Re: [Cython] memoryviews bloat?

[Sturla Molden]

Den 10. sep. 2013 kl. 02:14 skrev Robert Bradshaw :

>>> Wow. The first version of this PR used Cython memoryviews, which added a 
>>> whopping 14 kLOC of generated C to the repo. Switched back to bare pointers 
>>> to keep the compile times within bounds.
>> It would be interesting to hear the Cython team's point of view on that. 
>> @sturlamolden @robertwb @markflorisson88 @dagss
> 
> The priority has always been to produce the most optimized runtime
> code, with compile time being way down on the list of priorities, if
> even thought about at all. 


I thought perhaps the problem was the number of pyx files they had to compile. 
I.e. that the utility code is included in multiple C files, since they 
complained about 14 kLOC extra. But to that they answered:


" larsmans commented
a day ago
The problem with typed memoryviews is simply that they're different from what 
we were doing. We'll have to change a lot of code and habits, being careful not 
to lose performance due to memoryview overhead."


So I am not sure what the problem really was. Bloat or just an unfamiliar API?

There was also a strange comment about tail-call optimization being prevented 
in cdef functions that are not declared nogil – due to the refnanny. I am not 
sure what to make of that.


Sturla










___
cython-devel mailing list
cython-devel@python.org
https://mail.python.org/mailman/listinfo/cython-devel

[Cython] Metaclass bug corrupts the heap

[Dénes Vadász]

Hello,

according to valgrind the following Cython fragment causes a heap 
corruption (with Python 2.7.5 and Cython 0.19.1 on Ubuntu):


   cdef class MetaClass(type):
cdef int i

   class MyClass(object):
__metaclass__ = MetaClass



Please find below the result of a many hour investigation (originally 
triggered by random crashes and heap corruptions of a bigger Cython 
program that uses metaclasses).


MetaClass is compiled to a C structure that extends PyTypeObject (a cc. 
200 byte structure on a 32-bit architecture):


   struct __pyx_obj_4meta_MetaClass {
  PyTypeObject __pyx_base;
  int i;
   };


Instances of PyTypeObject are supposed to be allocated statically when 
initializing extension modules, because the structure does not support 
(among others) garbage collection. However, when MyClass is created, an 
instance of struct __pyx_obj_4meta_MetaClass (cc. 200 + 4 bytes) is 
dynamically allocated by the Python memory management machinery. The 
machinery then tries to initialize the allocated memory. The problem is 
that it expects that the first member of struct 
__pyx_obj_4meta_MetaClass is of type PyHeapTypeObject (a cc. 500 byte 
structure) and after a type cast it writes to the tail members of the 
assumed PyHeapTypeObject, i.e. way beyond the allocated cc. 200 + 4 
bytes, corrupting the object heap. This corruption is nicely reported by 
valgrind.


The proposed fix is to make __pyx_obj_4meta_MetaClass extend 
PyHeapTypeObject:


   struct __pyx_obj_4meta_MetaClass {
  PyHeapTypeObject __pyx_base;
  int i;
   };


This can be achieved by adding the below 2 lines to Compiler/Builtin.py:

   382a383,384
> elif name == 'type':
> objstruct_cname = 'PyHeapTypeObject'


So that the init_builtin_types function becomes:

   def init_builtin_types():
global builtin_types
for name, cname, methods in builtin_types_table:
utility = builtin_utility_code.get(name)
if name == 'frozenset':
objstruct_cname = 'PySetObject'
elif name == 'bool':
objstruct_cname = None
   *   elif name == 'type':*
   *   objstruct_cname = 'PyHeapTypeObject'*
else:
objstruct_cname = 'Py%sObject' % name.capitalize()
the_type = builtin_scope.declare_builtin_type(name, cname,
   utility, objstruct_cname)
builtin_types[name] = the_type
for method in methods:
method.declare_in_type(the_type)


After patching my Cython installation with the above, valgrind stopped 
to complain and there were no more crashes.


Please consider adding this fix in the next release of Cython.

Regards

Dénes Vadász
___
cython-devel mailing list
cython-devel@python.org
https://mail.python.org/mailman/listinfo/cython-devel