[Cython] GIL handling C code

mark florisson markflorisson88 at gmail.com
Tue Feb 28 22:31:30 CET 2012


On 28 February 2012 21:22, Stefan Behnel <stefan_ml at behnel.de> wrote:
> mark florisson, 28.02.2012 22:09:
>> On 28 February 2012 21:08, mark florisson wrote:
>>> On 28 February 2012 20:19, Stefan Behnel 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.
>>>
>>> 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();
>>> L2:
>>>    /* normal code */
>>>    return __pyx_r;
>>> }
>>
>> And there should be a release just before the return of course :)
>
> Yes, I think that's much better than acquiring the GIL in some node in the
> syntax tree and then releasing it in a completely different place at
> function exit.
>
> (IIRC, the C compiler won't allow the definition of the GIL state variable
> to occur at the end in this case because there'd be code paths that use it
> that start at a label after the declaration, so it would still have to be
> declared at the beginning. Fine with me.)
>
>
>>> If the entire body is a 'with gil' block, it'd probably be easiest to
>>> transform the nogil function into a 'with gil' function.
>
> Right.
>
>
>>> 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.
>
> In any case, jumping through labels is the normal way we do these things in
> Cython, so this is worth cleaning up. And if we do all of this in the main
> function code generation, it's much easier to figure out on what code paths
> (i.e. at what labels) the GIL is really required for cleanup.

A cleanup along with any optimizations would be great, +1 from me.

> Stefan
> _______________________________________________
> cython-devel mailing list
> cython-devel at python.org
> http://mail.python.org/mailman/listinfo/cython-devel


More information about the cython-devel mailing list