Unnecessary GIL grab in nogil functions

[Thouis (Ray) Jones]

In the code below, the c_sqrt_safe() function grabs the GIL at the end of the function, even if there was no exception raised.  This seems unnecessary, unless I'm misunderstanding something about nogil functions that contain raise statements.

If this behavior is actually necessary, it would be nice for Cython to warn when a nogil function might implicitly grab the gil.

Best,

Ray Jones

Two files below:

//--------------------------------------------------
//  cython_count_gil_grabs.pyx
//--------------------------------------------------
from libc.math cimport sqrt

cdef extern from "gil_counter.h":
     int gil_access_count


cdef float c_sqrt_safe(float value) nogil:
    if value < 0:
        with gil:
            raise ValueError
    return sqrt(value)

cdef float c_sqrt(float value) nogil:
    return sqrt(value)

cpdef test_c_sqrt():
    cdef int i
    print("BEFORE SAFE {}".format(gil_access_count))
    with nogil:
        for i in range(100):
            c_sqrt_safe(i + 1)
    print("AFTER SAFE {}".format(gil_access_count))


    print("BEFORE UNSAFE {}".format(gil_access_count))
    with nogil:
        for i in range(100):
            c_sqrt(i + 1)
    print("AFTER UNSAFE {}".format(gil_access_count))

if __name__ == '__main__':
    test_c_sqrt()

// --------------------------------------------------
// gil_counter.h
// --------------------------------------------------
static int gil_access_count = 0;

PyGILState_STATE PyGILState_Ensure_Count()
{
    gil_access_count++;
    return PyGILState_Ensure();
}

#ifdef WITH_THREAD
#define PyGILState_Ensure PyGILState_Ensure_Count
#endif

[Thouis (Ray) Jones]

Sorry for the "//" weirdness, I dropped those in just before posting forgetting which language I was in.

Ray Jones

 

[Robert Bradshaw]

On Tue, Apr 14, 2015 at 6:13 AM, Thouis (Ray) Jones <tho...@gmail.com> wrote:
> In the code below, the c_sqrt_safe() function grabs the GIL at the end of
> the function, even if there was no exception raised. This seems
> unnecessary, unless I'm misunderstanding something about nogil functions
> that contain raise statements.

It's not so much about the "raise" as it is about the with-gil block.
If a function contains such a block, some cleanup is assumed to be
required at exit. In particular, consider

cdef double c_sqrt(int value) nogil:
if value % 2:
with gil:
py_var1 = object()
if value % 3:
with gil:
py_var2 = object()
return sqrt(value)

in this case, one or both py_var1 or py_var2 may need to be decref'd
at the return of the function. Conceivably one could try to be more
intelligent and only generate the gil-capturing if it's needed (both
in this case and the exception raising case).

> If this behavior is actually necessary, it would be nice for Cython to warn
> when a nogil function might implicitly grab the gil.

The "with gil" is the indication that this function will implicitly
grab the gil.

> --
>
> ---
> You received this message because you are subscribed to the Google Groups
> "cython-users" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to cython-users...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Thouis (Ray) Jones]

On Wednesday, April 15, 2015 at 12:37:07 AM UTC-4, Robert Bradshaw wrote:

Conceivably one could try to be more
intelligent and only generate the gil-capturing if it's needed (both
in this case and the exception raising case).

Maybe Cython could track whether the GIL was actually captured, using a local variable, and cleanup only if necessary.  Or the cleanup could be moved to the end of the "with gil:" block.

Ray Jones