[Cython] GIL handling C code

[mark florisson]

On 28 February 2012 20:19, Stefan Behnel <stefan_ml at behnel.de> wrote:
> Stefan Behnel, 28.02.2012 20:58:
>> mark florisson, 28.02.2012 16:35:
>>> Basically, the cleanup code only needs a matching release because the
>>> corresponding acquire is in EnsureGILNode, which wraps the function
>>> body in case of a nogil function with a 'with gil' block. Any changes
>>> to the conditions in FuncDefNode will have to be reflected by the code
>>> that does that wrapping. Changing the refnanny macro for the cleanup
>>> code will not avail anything, as the GIL is already ensured.
>>
>> Regarding the "with gil" code, ISTM that the "finally" code in the with_gil
>> test is being duplicated. I noticed this when I moved the refnanny's GIL
>> state into a block local variable and that broke the C code. Basically, the
>> with-gil block had declared the variable in its own block, but was then
>> trying to access that variable in a second finally clause, further down and
>> outside of the with-gil block.
>
> Hmm, guess I got confused again...
>
> So, the code is this:
>
> """
>       /*finally:*/ {
>          int __pyx_why;
>          __pyx_why = 0; goto __pyx_L8;
>          __pyx_L7: __pyx_why = 4; goto __pyx_L8;
>          __pyx_L8:;
>          #ifdef WITH_THREAD
>          PyGILState_Release(__pyx_gilstate_save);
>          #endif
>          switch (__pyx_why) {
>            case 4: goto __pyx_L4;
>          }
>        }
>    }
>  }
>  /*finally:*/ {
>    int __pyx_why;
>    __pyx_why = 0; goto __pyx_L5;
>    __pyx_L4: __pyx_why = 4; goto __pyx_L5;
>    __pyx_L5:;
>    #ifdef WITH_THREAD
>    __pyx_gilstate_save = PyGILState_Ensure();
>    #endif
>    switch (__pyx_why) {
>      case 4: goto __pyx_L1_error;
>    }
>  }
>
>  goto __pyx_L0;
>  __pyx_L1_error:;
>  __Pyx_XDECREF(__pyx_t_1);
>  __Pyx_XDECREF(__pyx_t_2);
>  __Pyx_WriteUnraisable("with_gil.void_nogil_ignore_exception",
> __pyx_clineno, __pyx_lineno, __pyx_filename);
>  __pyx_L0:;
>  #ifdef WITH_THREAD
>  PyGILState_Release(__pyx_gilstate_save);
>  #endif
> }
> """
>
> The first "finally" block is inside of the with-gil block, whereas the
> second is outside. In the second, the GIL is reacquired, and I guess that's
> for cleaning up temps in the error case, right? I see the problem that it
> can't be acquired after jumping to the exit labels (error/return) in the
> current code layout, but wouldn't it make sense to generate this code
> instead (starting at the second finally clause):

This code could certainly be optimized, I just never got around to
implementing it.

> """
>  /*finally:*/ {
>    int __pyx_why;
>    __pyx_why = 0; goto __pyx_L5;
>    __pyx_L4: __pyx_why = 4; goto __pyx_L5;
>    __pyx_L5:;
>    switch (__pyx_why) {
>      case 4: goto __pyx_L1_error;
>    }
>  }
>
>  goto __pyx_L0;
>  __pyx_L1_error:;
>  #ifdef WITH_THREAD
>  __pyx_gilstate_save = PyGILState_Ensure();
>  #endif
>  __Pyx_XDECREF(__pyx_t_1);
>  __Pyx_XDECREF(__pyx_t_2);
>  __Pyx_WriteUnraisable("with_gil.void_nogil_ignore_exception",
> __pyx_clineno, __pyx_lineno, __pyx_filename);
>  #ifdef WITH_THREAD
>  PyGILState_Release(__pyx_gilstate_save);
>  #endif
>  return;   /* <- error return here! */
>  __pyx_L0:;
> }
> """
>
> Meaning: split the return code paths and let them independently acquire the
> GIL if needed? That would at least relieve the outer finally from having to
> care about the GIL and having to rely on "someone" to declare the GIL
> variable and eventually release it.

(Sorry, dinner). In this case, that would be valid. Normally though,
the exception case should fall through to the normal case, so I think
what you'd want is

    ...
    goto L0;
L1:
    PyGILState_Ensure();
    /* error code */
    __pyx_r = ...;
    goto L2;
L0:
    PyGILState_Ensure();
goto L2:
    /* normal code */
    return __pyx_r;
}

If the entire body is a 'with gil' block, it'd probably be easiest to
transform the nogil function into a 'with gil' function. If you want
to optimize for "paths going from with gil blocks directly into
function cleanup code" (i.e., no intermediate nogil try/finally or
nested with gil blocks), then you could use two more labels in the
code above that bypass the acquire.

> Or, alternatively, add an additional exit path to the finally block that
> jumps to a label that acquires the GIL before going on to the original
> label. Something along those lines...
>
> Stefan
> _______________________________________________
> cython-devel mailing list
> cython-devel at python.org
> http://mail.python.org/mailman/listinfo/cython-devel