[Cython] checking for "None" in nogil function

Stefan Behnel stefan_ml at behnel.de
Mon May 7 18:52:56 CEST 2012


Dag Sverre Seljebotn, 07.05.2012 17:48:
> On 05/07/2012 03:04 PM, Stefan Behnel wrote:
>> Dag Sverre Seljebotn, 07.05.2012 13:48:
>>>>> As far as I can remember (which might be biased towards my personal
>>>>> view), the conclusion was that we left the current semantics in place,
>>>>> relying on better control flow analysis to make None-checks cheaper, and
>>>>> when those are cheap enough, make the nonecheck directive default to
>>>>> True
>>>>
>>>> At least for buffer arguments, it silently corrupts data or segfaults in
>>>> the current state of affairs, as you pointed out. Not exactly ideal.
>>>
>>> No different than writing to a field in a cdef class...
>>
>> Hmm, aren't those None checked? At least cdef method calls are AFAIR.
> 
> Not at all. That's my whole point -- currently, the rule for None in Cython
> is "it's your responsibility to never do a native operation on None".
> 
> I don't like that either, but that's just inherited from Pyrex (and many
> projects would get speed regressions etc.).
> 
> I'm not against changing that to "we safely None-check", if done nicely --
> it's just that that should be done everywhere at once.

I think that gets both of us back on the same track then. :)


> In current master (and as far back as I can remember), this code:
> 
> cdef class A:
>     cdef int field
>     cdef int method(self):
>         print self.field
> def f():
>     cdef A a = None
>     a.field = 3
>     a.method()
> 
> Turns into:
> 
>   __pyx_v_a = ((struct __pyx_obj_5test2_A *)Py_None);
>   __pyx_v_a->field = 3;
>   ((struct __pyx_vtabstruct_5test2_A *)
> __pyx_v_a->__pyx_vtab)->method(__pyx_v_a);

Guess I've just been working on the builtins optimiser too long. There,
it's obviously not allowed to inject unprotected code like this automatically.

It would be fun if we could eventually get to the point where Cython
replaces all of the code in f() with an AttributeError, as a combined
effort of control flow analysis and dead code removal. A part of that is
already there, i.e. Cython would know that 'a' "may be None" in the last
two lines and would thus generate a None check with an AttributeError if we
allowed it to do that. It wouldn't know that it's always going to be
raised, though, so the dead code removal can't strike. I guess that case is
just not important enough to implement.

BTW, I recently tried to enable None checks in a couple of places and it
broke memory views for some reason that I didn't want to investigate. The
main problems really seem to be unknown argument values and the lack of
proper exception prediction, e.g. in this case:

  def add_one_2d(int[:,:] buf):
      for x in xrange(buf.shape[0]):
          for y in xrange(buf.shape[1]):
              buf[x,y] += 1

it's statically obvious that only the first access to .shape (outside of
all loops) needs a None check and will raise an AttributeError for None, so
the check for the second loop can be eliminated as well as the None check
on indexing.


>> I think we should really get back to the habit of making code safe first
>> and fast afterwards.
> 
> Nobody has argued otherwise for some time (since the cdivision thread I
> believe), this is all about Pyrex legacy. Guess part of the story is that
> there's lots of performance-sensitive code in SAGE using cdef classes which
> was written in Pyrex before Cython was around...
> 
> In fact, the nonecheck directive was written by yours truly! And I argued
> for making it the default at the time!

I've been working on the None checks (and on removing them) repeatedly,
although I didn't remember the particular details of discussing the
nonecheck directive.

Stefan


More information about the cython-devel mailing list