SEGFAULT problem...
Florent Hivert
I have a segfault problem which is probably due to a bad reference counting
management... I have a cdef class which contains a attribute _list of time
int* which points to a allocated array of size self._len. I wan't to build the
corresponding list.
The following version if working:
cpdef inline list list(self):
cdef int i
cdef list L = []
for i from 0<=i<self._len:
L.append(self._list[i])
return L
I was trying to avoid the append since I know the length from the
beginning. So I wrote:
cpdef inline list list(self):
cdef int i
cdef list L = PyList_New(self._len)
for i from 0<=i<self._len:
PyList_SET_ITEM(L, i, PyInt_FromLong(self._list[i]))
return L
An I got segfault for long list. Am I doing something stupid here ?
Cheers,
Florent
mark florisson
I wouldn't say stupid, but yes. PyList_SET_ITEM steals a reference, so
you need to INCREF the result from PyInt_FromLong, because Cython sees
that you created an object, so it will DECREF it when you're done with
it (after the call to PyList_SET_ITEM). Seriously though, just use
normal Cython syntax and all your troubles will be gone (and it will
likely be just as fast or marginally slower).
Seriously though, you're passing self.list[i]
> Cheers,
>
> Florent
>
mark florisson
This last sentence shouldn't have been part of the message.
Anyway, you can also use the range() function with a typed index to
get fast iteration.
>> Cheers,
>>
>> Florent
>>
>
mark florisson
In the future it would be neat to be able to write my_int_pointer[:N]
to get a list of ints (or basically anything that can convert to a
Python object). We do this for char pointer to string conversion
already.
>>> Cheers,
>>>
>>> Florent
>>>
>>
>
Florent Hivert
Thanks for this fast answer,
> > I was trying to avoid the append since I know the length from the
> > beginning. So I wrote:
> >
> > � �cpdef inline list list(self):
> > � � � �cdef int i
> > � � � �cdef list L = PyList_New(self._len)
> > � � � �for �i from 0<=i<self._len:
> > � � � � � �PyList_SET_ITEM(L, i, PyInt_FromLong(self._list[i]))
> > � � � �return L
> >
> > An I got segfault for long list. Am I doing something stupid here ?
>
> I wouldn't say stupid, but yes. PyList_SET_ITEM steals a reference, so
> you need to INCREF the result from PyInt_FromLong, because Cython sees
> that you created an object, so it will DECREF it when you're done with
> it (after the call to PyList_SET_ITEM).
Ok. I didn't realize that Cython was adding a DECREF automatically.
> Seriously though, just use
> normal Cython syntax and all your troubles will be gone (and it will
> likely be just as fast or marginally slower).
Do you mean that
cpdef inline list list(self):
cdef int i
cdef list L = PyList_New(self._len)
for i from 0<=i<self._len:
L[i] = self._list[i]
return L
is legal even if the list is not initialized ?
Cheers,
Florent
mark florisson
No, what I mean is "don't care about the append" and just write
"return [self._list[i] for i in range(self._len)] ". Any possible
buffer resizing the list object will do will be amortized over
self._len, I very much doubt it will be a problem. If it is, then use
a view on memory, like typed memoryviews with cython.array or normal
numpy arrays.
mark florisson
(It will help to type 'i' as an int or Py_ssize_t here).
Florent Hivert
> "return [self._list[i] for i in range(self._len)] ". Any possible
> buffer resizing the list object will do will be amortized over
> self._len, I very much doubt it will be a problem. If it is, then use
> a view on memory, like typed memoryviews with cython.array or normal
> numpy arrays.
for the record:
cdef int i
cdef list L = PyList_New(self._len)
cdef object o
for i from 0<=i<self._len:
o = PyInt_FromLong(self._list[i])
Py_INCREF(o)
PyList_SET_ITEM(L, i, o)
return L
is more than 20% faster than for list of size 1000:
return [self._list[i] for i in range(self._len)]
So rezising is not completely negligible.
Thanks for your help.
Cheers,
Florent
mark florisson
I agree, it's not negligible, I just said it's amortized. I do think
numpy arrays might help out here if this is performance critical. With
the upcoming release of Cython you could do 'return <int[:self._len]>
self._list' to get a memoryview (or call numpy.asarray() on that to
turn it into a NumPy array (without copying)).
Robert Bradshaw
That is a surprising amount of overhead. I suppose it would be
possible to optimize for the case where the size of the list is
constant known ahead of time.
- Robert
Stefan Behnel
> On Mon, Nov 14, 2011 at 3:18 PM, mark florisson wrote:
Note that it's not entirely clear from the example where exactly this
difference comes from as the two implementations differ broadly. There may
be multiple factors contributing to it. Might be worth asking callgrind for
an analysis.
Also, the speed of the realloc operation (and a potentially necessary copy)
depends a lot on the OS and other platform specifics, and I didn't see
Florent mention what he's using (although mailing through Mutt would hint
at some version of Linux or Unix).
> I suppose it would be
> possible to optimize for the case where the size of the list is
> constant known ahead of time.
Yes, CPython actually does that for list comprehensions. Builtin iterators
like range() that have a constant length provide a size hint that is used
to create the list.
Stefan
Stefan Behnel
> In the future it would be neat to be able to write my_int_pointer[:N]
> to get a list of ints (or basically anything that can convert to a
> Python object). We do this for char pointer to string conversion
> already.
Yes, I agree that this would be nice. I think the intention is clear enough
from the syntax, and converting it into a list (as opposed to a tuple, for
example) should target the most common use case.
Note that this already works for looping, so the original example could be
written as
return [ i for i in self._list[:self.len] ]
Stefan
Florent Hivert
> >>I agree, it's not negligible, I just said it's amortized. I do think
> >>numpy arrays might help out here if this is performance critical. With
> >>the upcoming release of Cython you could do 'return<int[:self._len]>
> >>self._list' to get a memoryview (or call numpy.asarray() on that to
> >>turn it into a NumPy array (without copying)).
> >
> >That is a surprising amount of overhead.
>
> Note that it's not entirely clear from the example where exactly
> this difference comes from as the two implementations differ
> broadly. There may be multiple factors contributing to it. Might be
> worth asking callgrind for an analysis.
>
> Also, the speed of the realloc operation (and a potentially
> necessary copy) depends a lot on the OS and other platform
> specifics, and I didn't see Florent mention what he's using
> (although mailing through Mutt would hint at some version of Linux
> or Unix).
I'm indeed using Linux on a 64 bits box (OpenSuSE 11.3) with
popcorn-~ $ gcc --version
gcc (SUSE Linux) 4.5.0 20100604 [gcc-4_5-branch revision 160292]
Cheers,
Florent
Stefan Behnel
Ok, thanks. I did a little testing on my side and it's pretty obvious now
that the difference is only due to the usage of PyList_Append() in the list
comprehension code. Everything else can be written in Python syntax without
any performance penalty.
The difference on my side is more like 30%, BTW, also 64bit Linux. Totally
worth optimising.
Stefan