Re: [Cython] memoryviews bloat?

2013-09-12 Thread Stefan Behnel
Robert Bradshaw, 10.09.2013 02:14:
>>> 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

Is there a link to this discussion?

Stefan

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


Re: [Cython] memoryviews bloat?

2013-09-12 Thread Sturla Molden

On Sep 12, 2013, at 6:33 PM, Stefan Behnel  wrote:

> Robert Bradshaw, 10.09.2013 02:14:
 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
> 
> Is there a link to this discussion?
> 

https://github.com/scikit-learn/scikit-learn/pull/2426


Sturla

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


Re: [Cython] Metaclass bug corrupts the heap

2013-09-12 Thread Stefan Behnel
Robert Bradshaw, 12.09.2013 23:02:
> Thanks for the careful analysis. See
> https://github.com/cython/cython/commit/e8b54dec4cdc94764abda4a107bc37dcd58e6f13

Might be possible to merge it with this special case:

https://github.com/cython/cython/blob/master/Cython/Compiler/Code.py#L50

and/or make it a part of BuiltinObjectType somehow.

Stefan

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


Re: [Cython] Metaclass bug corrupts the heap

2013-09-12 Thread Robert Bradshaw
Thanks for the careful analysis. See
https://github.com/cython/cython/commit/e8b54dec4cdc94764abda4a107bc37dcd58e6f13

On Tue, Sep 10, 2013 at 3:26 PM, Dénes Vadász  wrote:
> 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
>
___
cython-devel mailing list
cython-devel@python.org
https://mail.python.org/mailman/listinfo/cython-devel


[Cython] Assign from array to scalar

2013-09-12 Thread Emmanuel Gil Peyrot
Hi,

A few weeks ago I started an implementation of that simple usecase:

 cdef long a[2], x, y
 # set a[0] and a[1] to some useful values
 x, y = a

It does works, but I’m not used to the interactions between the
different parts of the code, I mainly hacked away where I could.  I’m
especially dissatisfied with the code duplication I came with (to check
the correct types and size) and the location of the checks.

I added testcases for both the array assignation and the slices of that
same assignation I plan to implement, which seems to integrate
badly with my current implementation, making the code even more
unreadable.

I tried to find a commit implementing a similar feature so I could get
some inspiration but couldn’t find one.  If someone has a hint about
that I’d be glad. :)

Btw, on a Cython.Compiler.ExprNodes.SliceIndexNode object, why does
stop_code() give an int while start_code() gives a string?  It’d be
much better to return an int in both cases, so one could interpolate
without conversion from both python or the generated C.

Preliminary patch joined.

-- 
Emmanuel Gil Peyrot
XMPP: 
OpenPGP: 24B1D609
diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py
index 2511eaf..bf04ff1 100755
--- a/Cython/Compiler/ExprNodes.py
+++ b/Cython/Compiler/ExprNodes.py
@@ -5615,6 +5615,11 @@ class SequenceNode(ExprNode):
 def generate_assignment_code(self, rhs, code):
 if self.starred_assignment:
 self.generate_starred_assignment_code(rhs, code)
+elif (isinstance(self, TupleNode) and
+  isinstance(rhs, NameNode) and
+  isinstance(rhs.entry.type, PyrexTypes.CArrayType)):
+self.generate_parallel_assignment_code2(rhs, code)
+return
 else:
 self.generate_parallel_assignment_code(rhs, code)
 
@@ -5627,6 +5632,22 @@ class SequenceNode(ExprNode):
 PyrexTypes.CFuncTypeArg("it", PyrexTypes.py_object_type, None),
 ]))
 
+def generate_parallel_assignment_code2(self, rhs, code):
+rhs_type = rhs.entry.type
+base_type = rhs_type.base_type
+size = rhs_type.size
+lhs_args = self.args
+assert len(lhs_args) == size
+for arg in lhs_args:
+if arg.entry.type is not base_type:
+raise ValueError("Wrong type for parallel assignment of '%s' 
array: %s" %
+ (base_type, arg.entry.type))
+for i, arg in enumerate(lhs_args):
+code.putln("%s = %s[%s];" % (
+arg.result(),
+rhs.result(),
+i))
+
 def generate_parallel_assignment_code(self, rhs, code):
 # Need to work around the fact that generate_evaluation_code
 # allocates the temps in a rather hacky way -- the assignment
diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py
index fe7adb3..7987cc6 100644
--- a/Cython/Compiler/Nodes.py
+++ b/Cython/Compiler/Nodes.py
@@ -4509,6 +4509,20 @@ class SingleAssignmentNode(AssignmentNode):
 self.lhs = self.lhs.analyse_target_types(env)
 self.lhs.gil_assignment_check(env)
 
+if (isinstance(self.lhs, ExprNodes.TupleNode) and
+isinstance(self.rhs, ExprNodes.NameNode)):
+rhs_type = self.rhs.entry.type
+if isinstance(rhs_type, PyrexTypes.CArrayType):
+base_type = rhs_type.base_type
+size = rhs_type.size
+lhs_args = self.lhs.args
+assert len(lhs_args) == size
+for arg in lhs_args:
+if arg.entry.type is not base_type:
+break
+else:
+return self
+
 if self.lhs.memslice_broadcast or self.rhs.memslice_broadcast:
 self.lhs.memslice_broadcast = True
 self.rhs.memslice_broadcast = True
diff --git a/tests/run/arrayassign.pyx b/tests/run/arrayassign.pyx
index 29c353e..f76ad2e 100644
--- a/tests/run/arrayassign.pyx
+++ b/tests/run/arrayassign.pyx
@@ -128,6 +128,59 @@ def test_ptr_literal_list_slice_end():
 a[:5] = [1,2,3,4,5]
 return (a[0], a[1], a[2], a[3], a[4])
 
+
+# The opposite, assignation from an array to multiple scalars.
+
+def test_array_to_scalars():
+"""
+>>> test_array_to_scalars()
+(1, 2, 3, 4, 5)
+"""
+cdef int l[5], a, b, c, d, e
+l[:] = [1,2,3,4,5]
+a, b, c, d, e = l
+return (a, b, c, d, e)
+
+def test_slice_all_to_scalars():
+"""
+>>> test_slice_all_to_scalars()
+(1, 2, 3, 4, 5)
+"""
+cdef int l[5], a, b, c, d, e
+l[:] = [1,2,3,4,5]
+a, b, c, d, e = l[:]
+return (a, b, c, d, e)
+
+def test_slice_start_to_scalars():
+"""
+>>> test_slice_start_to_scalars()
+(1, 2, 3, 4, 5)
+"""
+cdef int l[7], a, b, c, d, e
+l[:] = [6,7,1,2,3,4,5]
+a, b, c, d, e = l[2:]
+return (a, b, c, d, e)
+
+def test_slice_end_to_scalars():
+"""