Get some exception safety, using Cython's built-in exception translation #32
Conversation
|
I'm conflicted about this one - do you have an idea which cython versions have / do not have this problem? |
|
All of them, I think. I got in touch with the Cython guys and it has to do with the ambiguity of Also, the pointers are really necessary since Cython has the habit of generating faulty code in the face of references, but you knew that. |
|
Ok, that explains it then. Can you add a link to that ML discussion in your commit message and quickly check it doesn't regress in speed (although as you said providing gcc etc do the right thing with the indexing template, it should not). Perhaps it is worth adding a small performance test case to the test suite? |
|
>>> import pcl
>>> p = pcl.PointCloud()
>>> %timeit p.from_array(a)
100000 loops, best of 3: 17.5 µs per loopUsed to be 23.4 µs per loop according to ef538e7. I'll amend the commit message and push one more commit to make |
Uses a hack to get around limitations in Cython, see pcl/indexing.hpp. See https://groups.google.com/forum/#!topic/cython-users/AAo7MQFLZyw for explanation. Also, we no longer need to use the at member when bounds checking is not needed: >>> import pcl >>> p = pcl.PointCloud() >>> %timeit p.from_array(a) 100000 loops, best of 3: 17.5 µs per loop This used to be 23.4 µs per loop.
Used to trigger a C assert/abort in PCL 1.7.1.
This is the segfault reported in strawlab#28. __cinit__ unconditionally calls its base class version, so two objects got allocated and one of them was leaked. Fixed by moving the actual allocation to __init__. Similarly, __dealloc__ would call its base class version and a "double delete" would follow. Fixed by removing the child class's __dealloc__, and set self.me to NULL explicitly in OctreePointCloud.__dealloc__ for added safety.
|
Pushed a new version of the commit, as well as two fixes to |
Get some exception safety, using Cython's built-in exception translation
|
Great, thanks a lot! |
Get some exception safety, using Cython's built-in exception translation
2 participants
Uses a pretty ugly hack to get around limitations in Cython, see
pcl/indexing.hpp.OTOH, we no longer need to use the
atmember when bounds checking is not needed, which should give some speedup. I must confess that I didn't measure it.